Implement separate version table for journals

This ensures inserts to the journal versions are atomtic, so we can
avoid locking the journals table.

https://community.openproject.com/wp/30594

app/models/changeset.rb

@@ -196,13 +196,21 @@ class Changeset < ActiveRecord::Base
   # i.e. a work_package that belong to the repository project, a subproject or a parent project
   def find_referenced_work_package_by_id(id)
     return nil if id.blank?
+
     work_package = WorkPackage.includes(:project).find_by(id: id.to_i)
-    if work_package
-      unless work_package.project && (project == work_package.project || project.is_ancestor_of?(work_package.project) || project.is_descendant_of?(work_package.project))
-        work_package = nil
-      end
+
+    # Check that the work package is either in the same,
+    # a parent or child project of the given changeset
+    if in_ancestor_chain?(work_package, project)
+      work_package
     end
-    work_package
+  end
+
+  def in_ancestor_chain?(work_package, project)
+    work_package&.project &&
+      (project == work_package.project ||
+        project.is_ancestor_of?(work_package.project) ||
+        project.is_descendant_of?(work_package.project))
   end
 
   def fix_work_package(work_package)
@@ -227,6 +235,7 @@ class Changeset < ActiveRecord::Base
     unless work_package.save(validate: false)
       logger.warn("Work package ##{work_package.id} could not be saved by changeset #{id}: #{work_package.errors.full_messages}") if logger
     end
+
     work_package
   end
 

app/models/journal.rb

@@ -54,31 +54,6 @@ class Journal < ActiveRecord::Base
   # logs like the history on issue#show
   scope :changing, -> { where(['version > 1']) }
 
-  # Ensure that no INSERT/UPDATE/DELETE statements as well as other code inside :with_write_lock
-  # is run concurrently to the code inside this block, by using database locking.
-  # Note for PostgreSQL: If this is called from inside a transaction, the lock will last until the
-  #   end of that transaction.
-  # Note for MySQL: THis method does not currently change anything (no locking at all)
-  def self.with_write_lock(journable)
-    lock_name =
-    if OpenProject::Database.mysql?
-      # MySQL only supports a single lock
-      "journals.write_lock"
-    else
-      "journal.#{journable.class}.#{journable.id}"
-    end
-
-    result = Journal.with_advisory_lock_result(lock_name, timeout_seconds: 60) do
-      yield
-    end
-
-    unless result.lock_was_acquired?
-      raise "Failed to acquire write lock to journable #{journable.class} #{journable.id}"
-    end
-
-    result.result
-  end
-
   def changed_data=(changed_attributes)
     attributes = changed_attributes
 

app/models/journal_manager.rb

@@ -91,13 +91,17 @@ class JournalManager
     end
 
     def journal_class_name(type)
-      "#{base_class(type).name}Journal"
+      "#{base_class_name(type)}Journal"
     end
 
     def base_class(type)
       type.base_class
     end
 
+    def base_class_name(type)
+      base_class(type).name
+    end
+
     def create_association_data(journable, journal)
       create_attachment_data journable, journal if journable.respond_to? :attachments
       create_custom_field_data journable, journal if journable.respond_to? :custom_values
@@ -225,30 +229,75 @@ class JournalManager
   end
 
   def self.add_journal!(journable, user = User.current, notes = '')
-    if journalized? journable
-      # Obtain a table lock to ensure consistent version numbers
-      Journal.with_write_lock(journable) do
+    return unless journalized?(journable)
 
-        # Maximum version might be nil, so use to_i here.
-        version = journable.journals.maximum(:version).to_i + 1
+    # Ensure a version exists for this journable type
+    # since no version is changed here, in case of concurrency, one
+    # of the calls is allowed to fail
+    journable_type = base_class_name(journable.class)
+    ::JournalVersion.find_or_create_by(journable_type: journable_type, journable_id: journable.id)
 
-        journal_attributes = { journable_id: journable.id,
-                               journable_type: journal_class_name(journable.class),
-                               version: version,
-                               activity_type: journable.send(:activity_type),
-                               details: journable_details(journable) }
+    version = get_next_journal_version(journable_type, journable)
 
-        journal = create_journal journable, journal_attributes, user, notes
+    Rails.logger.debug "Inserting new journal for #{journable_type} ##{journable.id} @ #{version}"
 
-        # FIXME: this is required for the association to be correctly saved...
-        journable.journals.select(&:new_record?)
+    journal_attributes = { journable_id: journable.id,
+                           journable_type: journal_class_name(journable.class),
+                           version: version,
+                           activity_type: journable.send(:activity_type),
+                           details: journable_details(journable) }
 
-        journal.save!
-        journal
-      end
+    journal = create_journal journable, journal_attributes, user, notes
+
+    # FIXME: this is required for the association to be correctly saved...
+    journable.journals.select(&:new_record?)
+
+    journal.save!
+    journal
+  end
+
+  def self.get_next_journal_version(journable_type, journable)
+    # Increment the version according to our DB
+    if OpenProject::Database.postgresql?
+      increment_version!(journable_type, journable.id)
+    else
+      lock_and_increment_version!(journable_type, journable.id)
     end
   end
 
+  ##
+  # In case of MySQL, we cannot return the value just inserted with RETURNING,
+  # but have to insert and select separately, which is not atomic
+  def self.lock_and_increment_version!(journable_type, journable_id)
+    result = Journal.with_advisory_lock_result('journals.write_lock', timeout_seconds: 30) do
+      entry = ::JournalVersion.find_by!(journable_type: journable_type, journable_id: journable_id)
+      entry.increment!(:version)
+
+      entry.version
+    end
+
+    unless result.lock_was_acquired?
+      raise "Failed to acquire write lock to journable #{journable_type} ##{journable_id}"
+    end
+
+    result.result
+  end
+
+  def self.increment_version!(journable_type, journable_id)
+    sql = <<~SQL
+      UPDATE #{JournalVersion.table_name}
+      SET version = version + 1
+      WHERE journable_type = :journable_type AND :journable_id = journable_id
+      RETURNING version
+    SQL
+
+    sanitized = ::OpenProject::SqlSanitization.sanitize(sql, journable_type: journable_type, journable_id: journable_id)
+    ::JournalVersion
+      .connection
+      .execute(sanitized)
+      .first['version']
+  end
+
   def self.create_journal(journable, journal_attributes, user = User.current,  notes = '')
     type = base_class(journable.class)
     extended_journal_attributes = journal_attributes

app/models/journal_version.rb

@@ -0,0 +1,31 @@
+#-- encoding: UTF-8
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+class JournalVersion < ActiveRecord::Base
+end

app/services/changesets/log_time_service.rb

@@ -67,7 +67,8 @@ module Changesets
 
     def log_error(service_result)
       unless service_result.success?
-        logger&.warn("TimeEntry could not be created by changeset #{id}: #{service_result.errors.full_messages}")
+        errors = service_result.errors.full_messages.join(", ")
+        Rails.logger.warn("TimeEntry could not be created by changeset #{changeset.id}: #{errors}")
       end
     end
 

db/migrate/20190724093332_add_journal_versions_table.rb

@@ -0,0 +1,28 @@
+require_relative './migration_utils/utils'
+
+class AddJournalVersionsTable < ActiveRecord::Migration[5.2]
+  include ::Migration::Utils
+
+  def up
+    create_table :journal_versions do |t|
+      t.string :journable_type
+      t.integer :journable_id
+      t.integer :version, default: 0
+      t.index %i[journable_type journable_id version],
+              name: 'unique_journal_version',
+              unique: true
+    end
+
+    ActiveRecord::Base.connection.execute <<-SQL
+          INSERT INTO journal_versions (journable_type, journable_id, version)
+          (SELECT
+            journable_type, journable_id, MAX(version)
+          FROM journals
+          GROUP BY journable_type, journable_id);
+    SQL
+  end
+
+  def down
+    drop_table :journal_versions
+  end
+end

db/migrate/tables/journals.rb

@@ -45,7 +45,8 @@ class Tables::Journals < Tables::Base
       t.index :journable_type
       t.index :user_id
       t.index :activity_type
-      t.index %i[journable_type journable_id version], unique: true
+      t.index %i[journable_type journable_id version],
+              unique: true
     end
   end
 end

lib/plugins/acts_as_journalized/lib/redmine/acts/journalized/creation.rb

@@ -156,6 +156,8 @@ module Redmine::Acts::Journalized
           new_journal.user_id = user.id
         end
 
+        # Ensure journal version exists
+        ::JournalVersion.find_or_create_by(journable_type: self.class.name, journable_id: id, version: 1)
         JournalManager.recreate_initial_journal self.class, new_journal, changed_data
 
         # Backdate journal

lib/plugins/acts_as_journalized/lib/redmine/acts/journalized/save_hooks.rb

@@ -56,11 +56,18 @@ module Redmine::Acts::Journalized
 
       base.class_eval do
         after_save :save_journals
+        after_destroy :remove_journal_version
 
         attr_accessor :journal_notes, :journal_user, :extra_journal_attributes
       end
     end
 
+    def remove_journal_version
+      ::JournalVersion
+        .where(journable_type: self.class.name, journable_id: id)
+        .delete_all
+    end
+
     def save_journals
       @journal_user ||= User.current
       @journal_notes ||= ''

spec/models/changeset_spec.rb

@@ -34,10 +34,10 @@ describe Changeset, type: :model do
   with_virtual_subversion_repository do
     let(:changeset) {
       FactoryBot.build(:changeset,
-                        repository: repository,
-                        revision: '1',
-                        committer: email,
-                        comments: 'Initial commit')
+                       repository: repository,
+                       revision: '1',
+                       committer: email,
+                       comments: 'Initial commit')
     }
   end
 
@@ -57,9 +57,318 @@ describe Changeset, type: :model do
     end
   end
 
+  describe 'empty comment' do
+    it 'should comments empty' do
+      changeset.comments = ''
+      expect(changeset.save).to eq true
+      expect(changeset.comments).to eq ''
+
+      if changeset.comments.respond_to?(:force_encoding)
+        assert_equal 'UTF-8', changeset.comments.encoding.to_s
+      end
+    end
+
+    it 'should comments nil' do
+      changeset.comments = nil
+      expect(changeset.save).to eq true
+      expect(changeset.comments).to eq ''
+
+      if changeset.comments.respond_to?(:force_encoding)
+        assert_equal 'UTF-8', changeset.comments.encoding.to_s
+      end
+    end
+  end
+
+  describe 'stripping commit' do
+    let(:comment) { 'This is a looooooooooooooong comment' + (' ' * 80 + "\n") * 5 }
+    with_virtual_subversion_repository do
+      let(:changeset) {
+        FactoryBot.build(:changeset,
+                         repository: repository,
+                         revision: '1',
+                         committer: email,
+                         comments: comment)
+      }
+    end
+
+    it 'should for changeset comments strip' do
+      expect(changeset.save).to eq true
+      expect(comment).to_not eq changeset.comments
+      expect(changeset.comments).to eq 'This is a looooooooooooooong comment'
+    end
+  end
+
+  describe 'mapping' do
+    let!(:user) { FactoryBot.create :user, login: 'jsmith', mail: 'jsmith@somenet.foo' }
+    let!(:repository) { FactoryBot.create(:repository_subversion) }
+
+    it 'should manual user mapping' do
+      c = Changeset.create! repository: repository,
+                            committer: 'foo',
+                            committed_on: Time.now,
+                            revision: 100,
+                            comments: 'Committed by foo.'
+
+      expect(c.user).to be_nil
+      repository.committer_ids = { 'foo' => user.id }
+      expect(c.reload.user).to eq user
+
+      # committer is now mapped
+      c = Changeset.create! repository: repository,
+                            committer: 'foo',
+                            committed_on: Time.now,
+                            revision: 101,
+                            comments: 'Another commit by foo.'
+      expect(c.user).to eq user
+    end
+
+    it 'should auto user mapping by username' do
+      c = Changeset.create! repository: repository,
+                            committer: 'jsmith',
+                            committed_on: Time.now,
+                            revision: 100,
+                            comments: 'Committed by john.'
+      expect(c.user).to eq user
+    end
+
+    it 'should auto user mapping by email' do
+      c = Changeset.create! repository: repository,
+                            committer: 'john <jsmith@somenet.foo>',
+                            committed_on: Time.now,
+                            revision: 100,
+                            comments: 'Committed by john.'
+
+      expect(c.user).to eq user
+    end
+  end
+
+  describe '#scan_comment_for_work_package_ids',
+           with_settings: {
+             commit_fix_done_ratio: '90',
+             commit_ref_keywords: 'refs , references, IssueID',
+             commit_fix_keywords: 'fixes , closes',
+             default_language: 'en'
+           } do
+    let!(:user) { FactoryBot.create :admin, login: 'dlopper' }
+    let!(:open_status) { FactoryBot.create :status }
+    let!(:closed_status) { FactoryBot.create :closed_status }
+
+    let!(:other_work_package) { FactoryBot.create :work_package, status: open_status }
+    let(:comments) { "Some fix made, fixes ##{work_package.id} and fixes ##{other_work_package.id}" }
+
+    with_virtual_subversion_repository do
+      let!(:work_package) { FactoryBot.create :work_package, project: repository.project, status: open_status }
+      let(:changeset) {
+        FactoryBot.create(:changeset,
+                          repository: repository,
+                          revision: '123',
+                          committer: user.login,
+                          comments: comments)
+      }
+    end
+
+    before do
+      # choosing a status to apply to fix issues
+      allow(Setting).to receive(:commit_fix_status_id).and_return closed_status.id
+    end
+
+    describe 'with any matching', with_settings: { commit_ref_keywords: '*' } do
+      describe 'reference with brackets' do
+        let(:comments) { "[##{work_package.id}] Worked on this work_package" }
+
+        it 'should reference' do
+          changeset.scan_comment_for_work_package_ids
+          work_package.reload
+
+          expect(work_package.changesets).to eq [changeset]
+        end
+      end
+
+      describe 'reference at line start' do
+        let(:comments) { "##{work_package.id} Worked on this work_package" }
+
+        it 'should reference' do
+          changeset.scan_comment_for_work_package_ids
+          work_package.reload
+
+          expect(work_package.changesets).to eq [changeset]
+        end
+      end
+    end
+
+    describe 'non matching ref' do
+      let(:comments) { "Some fix ignores ##{work_package.id}" }
+      it 'should reference the work package id' do
+        changeset.scan_comment_for_work_package_ids
+        work_package.reload
+
+        expect(work_package.changesets).to eq []
+      end
+    end
+
+    describe 'with timelogs' do
+      let!(:activity) { FactoryBot.create :activity, is_default: true }
+      before do
+        repository.project.enabled_module_names += ['time_tracking']
+        repository.project.save!
+      end
+
+      it 'should ref keywords any with timelog' do
+        allow(Setting).to receive(:commit_ref_keywords).and_return '*'
+        allow(Setting).to receive(:commit_logtime_enabled?).and_return true
+
+        {
+          '2' => 2.0,
+          '2h' => 2.0,
+          '2hours' => 2.0,
+          '15m' => 0.25,
+          '15min' => 0.25,
+          '3h15' => 3.25,
+          '3h15m' => 3.25,
+          '3h15min' => 3.25,
+          '3:15' => 3.25,
+          '3.25' => 3.25,
+          '3.25h' => 3.25,
+          '3,25' => 3.25,
+          '3,25h' => 3.25
+        }.each do |syntax, expected_hours|
+          c = Changeset.new repository: repository,
+                            committed_on: 24.hours.ago,
+                            comments: "Worked on this work_package ##{work_package.id} @#{syntax}",
+                            revision: '520',
+                            user: user
+
+          expect { c.scan_comment_for_work_package_ids }
+            .to change { TimeEntry.count }.by(1)
+
+          expect(c.work_package_ids).to eq [work_package.id]
+
+          time = TimeEntry.order(Arel.sql('id DESC')).first
+          assert_equal work_package.id, time.work_package_id
+          assert_equal work_package.project_id, time.project_id
+          assert_equal user.id, time.user_id
+
+          expect(time.hours).to eq expected_hours
+          expect(time.spent_on).to eq Date.yesterday
+
+          expect(time.activity.is_default).to eq true
+          expect(time.comments).to include 'r520'
+        end
+      end
+
+      context 'with a second work package' do
+        let!(:work_package2) { FactoryBot.create :work_package, project: repository.project, status: open_status }
+        it 'should ref keywords closing with timelog' do
+          allow(Setting).to receive(:commit_fix_status_id).and_return closed_status.id
+          allow(Setting).to receive(:commit_ref_keywords).and_return '*'
+          allow(Setting).to receive(:commit_fix_keywords).and_return 'fixes , closes'
+          allow(Setting).to receive(:commit_logtime_enabled?).and_return true
+
+          c = Changeset.new repository: repository,
+                            committed_on: Time.now,
+                            comments: "This is a comment. Fixes ##{work_package.id} @4.5, ##{work_package2.id} @1",
+                            revision: '520',
+                            user: user
+
+          expect { c.scan_comment_for_work_package_ids }
+            .to change { TimeEntry.count }.by(2)
+
+          expect(c.work_package_ids).to match_array [work_package.id, work_package2.id]
+
+          work_package.reload
+          work_package2.reload
+          expect(work_package).to be_closed
+          expect(work_package2).to be_closed
+
+          times = TimeEntry.order(Arel.sql('id desc')).limit(2)
+          expect(times.map(&:work_package_id)).to match_array [work_package.id, work_package2.id]
+        end
+      end
+    end
+
+    it 'should reference the work package id' do
+      # make sure work package 1 is not already closed
+      expect(work_package.status.is_closed?).to eq false
+
+      changeset.scan_comment_for_work_package_ids
+      work_package.reload
+
+      expect(work_package.changesets).to eq [changeset]
+
+      expect(work_package.status).to eq closed_status
+      expect(work_package.done_ratio).to eq 90
+
+      # issue change
+      journal = work_package.journals.last
+
+      expect(journal.user).to eq user
+      expect(journal.notes).to eq 'Applied in changeset r123.'
+
+      # Expect other work package to be unchanged
+      # due to other project
+      other_work_package.reload
+      expect(other_work_package.changesets).to eq []
+    end
+
+    describe 'with work package in parent project' do
+      let(:parent) { FactoryBot.create :project }
+      let!(:work_package) { FactoryBot.create :work_package, project: parent, status: open_status }
+
+      before do
+        repository.project.parent = parent
+        repository.project.save!
+      end
+
+      it 'can reference it' do
+        # make sure work package 1 is not already closed
+        expect(work_package.status.is_closed?).to eq false
+
+        changeset.scan_comment_for_work_package_ids
+        work_package.reload
+
+        expect(work_package.changesets).to eq [changeset]
+
+        # Expect other work package to be unchanged
+        # due to other project
+        other_work_package.reload
+        expect(other_work_package.changesets).to eq []
+      end
+    end
+
+    describe 'with work package in sub project' do
+      let(:sub) { FactoryBot.create :project }
+      let!(:work_package) { FactoryBot.create :work_package, project: sub, status: open_status }
+
+      before do
+        sub.parent = repository.project
+        sub.save!
+
+        repository.project.reload
+        sub.reload
+      end
+
+      it 'can reference it' do
+        # make sure work package 1 is not already closed
+        expect(work_package.status.is_closed?).to eq false
+
+        changeset.scan_comment_for_work_package_ids
+        work_package.reload
+
+        expect(work_package.changesets).to eq [changeset]
+
+        # Expect other work package to be unchanged
+        # due to other project
+        other_work_package.reload
+        expect(other_work_package.changesets).to eq []
+      end
+    end
+  end
+
   describe 'assign_openproject user' do
     describe 'w/o user' do
-      before do changeset.save! end
+      before do
+        changeset.save!
+      end
 
       it_behaves_like 'valid changeset' do
         let(:journal_user) { User.anonymous }
@@ -69,7 +378,9 @@ describe Changeset, type: :model do
     describe 'with user is committer' do
       let!(:committer) { FactoryBot.create(:user, login: email) }
 
-      before do changeset.save! end
+      before do
+        changeset.save!
+      end
 
       it_behaves_like 'valid changeset' do
         let(:journal_user) { committer }

spec/models/journal_version_spec.rb

@@ -0,0 +1,73 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+describe JournalVersion, type: :model do
+  let!(:work_package) do
+    wp = FactoryBot.build(:work_package)
+    wp.journal_notes = 'foobar!'
+
+    wp.save!
+    wp
+  end
+
+  subject { ::JournalVersion.find_by!(journable_type: 'WorkPackage', id: work_package.id) }
+
+  before do
+    work_package
+    subject
+  end
+
+  it 'is created when the work package is created' do
+    expect(subject.version).to eq 1
+  end
+
+  it 'is incremented when the work package is journaled' do
+    work_package.subject = 'Foobar!'
+    work_package.journal_notes = 'My comment'
+    work_package.save!
+
+    work_package.reload
+
+    expect(work_package.journals.count).to eq 2
+    expect(work_package.journals.first.version).to eq 1
+    expect(work_package.journals.last.version).to eq 2
+
+    subject.reload
+    expect(subject.version).to eq 2
+  end
+
+  it 'is removed when the work package is removed' do
+    expect(subject).to be_present
+
+    work_package.destroy!
+
+    expect { subject.reload }.to raise_error ActiveRecord::RecordNotFound
+  end
+end

spec/models/repository/subversion_spec.rb

@@ -46,6 +46,11 @@ describe Repository::Subversion, type: :model do
     it 'does not allow creating a repository' do
       expect { instance.save! }.to raise_error ActiveRecord::RecordInvalid
     end
+
+    it 'returns an error when trying to save' do
+      expect(instance.save).to eq false
+      expect(instance.errors[:type]).to include I18n.translate('activerecord.errors.models.repository.not_available')
+    end
   end
 
   describe 'default Subversion' do

spec_legacy/functional/wiki_controller_spec.rb

@@ -176,6 +176,7 @@ describe WikiController, type: :controller do
   # because running whole suite is fine, but running only this test
   # results in failure
   it 'should update stale page should not raise an error' do
+    ::JournalVersion.create!(journable_type: 'WikiContent', journable_id: 2, version: 1)
     journal = FactoryBot.create :wiki_content_journal,
                                  journable_id: 2,
                                  version: 1,

spec_legacy/unit/changeset_spec.rb

@@ -1,279 +0,0 @@
-#-- encoding: UTF-8
-#-- copyright
-# OpenProject is a project management system.
-# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License version 3.
-#
-# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
-# Copyright (C) 2006-2017 Jean-Philippe Lang
-# Copyright (C) 2010-2013 the ChiliProject Team
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-#
-# See docs/COPYRIGHT.rdoc for more details.
-#++
-require_relative '../legacy_spec_helper'
-
-describe Changeset, type: :model do
-  fixtures :all
-
-  context 'with notified events', with_settings: { notified_events: %w(work_package_updated) } do
-    it 'should ref keywords any' do
-      WorkPackage.all.each(&:recreate_initial_journal!)
-
-      Setting.commit_fix_status_id = Status.where(['is_closed = ?', true]).first.id
-      Setting.commit_fix_done_ratio = '90'
-      Setting.commit_ref_keywords = '*'
-      Setting.commit_fix_keywords = 'fixes , closes'
-
-      c = Changeset.new(repository: Project.find(1).repository,
-                        committed_on: Time.now,
-                        comments: 'New commit (#2). Fixes #1')
-      c.scan_comment_for_work_package_ids
-
-      assert_equal [1, 2], c.work_package_ids.sort
-      fixed = WorkPackage.find(1)
-      assert fixed.closed?
-      assert_equal 90, fixed.done_ratio
-      assert_equal 2, ActionMailer::Base.deliveries.size
-    end
-
-    it 'should ref keywords' do
-      Setting.commit_ref_keywords = 'refs'
-      Setting.commit_fix_keywords = ''
-
-      c = Changeset.new(repository: Project.find(1).repository,
-                        committed_on: Time.now,
-                        comments: 'Ignores #2. Refs #1')
-      c.scan_comment_for_work_package_ids
-
-      assert_equal [1], c.work_package_ids.sort
-    end
-
-    it 'should ref keywords any only' do
-      Setting.commit_ref_keywords = '*'
-      Setting.commit_fix_keywords = ''
-
-      c = Changeset.new(repository: Project.find(1).repository,
-                        committed_on: Time.now,
-                        comments: 'Ignores #2. Refs #1')
-      c.scan_comment_for_work_package_ids
-
-      assert_equal [1, 2], c.work_package_ids.sort
-    end
-
-    it 'should ref keywords any with timelog' do
-      Setting.commit_ref_keywords = '*'
-      Setting.commit_logtime_enabled = '1'
-
-      {
-        '2' => 2.0,
-        '2h' => 2.0,
-        '2hours' => 2.0,
-        '15m' => 0.25,
-        '15min' => 0.25,
-        '3h15' => 3.25,
-        '3h15m' => 3.25,
-        '3h15min' => 3.25,
-        '3:15' => 3.25,
-        '3.25' => 3.25,
-        '3.25h' => 3.25,
-        '3,25' => 3.25,
-        '3,25h' => 3.25
-      }.each do |syntax, expected_hours|
-        c = Changeset.new(repository: Project.find(1).repository,
-                          committed_on: 24.hours.ago,
-                          comments: "Worked on this work_package #1 @#{syntax}",
-                          revision: '520',
-                          user: User.find(2))
-        assert_difference 'TimeEntry.count' do
-          c.scan_comment_for_work_package_ids
-        end
-        assert_equal [1], c.work_package_ids.sort
-
-        time = TimeEntry.order(Arel.sql('id DESC')).first
-        assert_equal 1, time.work_package_id
-        assert_equal 1, time.project_id
-        assert_equal 2, time.user_id
-        assert_equal expected_hours,
-                     time.hours,
-                     "@#{syntax} should be logged as #{expected_hours} hours but was #{time.hours}"
-        assert_equal Date.yesterday, time.spent_on
-        assert time.activity.is_default?
-        assert time.comments.include?('r520'),
-               "r520 was expected in time_entry comments: #{time.comments}"
-      end
-    end
-
-    it 'should ref keywords closing with timelog' do
-      Setting.commit_fix_status_id = Status.where(['is_closed = ?', true]).first.id
-      Setting.commit_ref_keywords = '*'
-      Setting.commit_fix_keywords = 'fixes , closes'
-      Setting.commit_logtime_enabled = '1'
-
-      c = Changeset.new(repository: Project.find(1).repository,
-                        committed_on: Time.now,
-                        comments: 'This is a comment. Fixes #1 @4.5, #2 @1',
-                        user: User.find(2))
-      assert_difference 'TimeEntry.count', 2 do
-        c.scan_comment_for_work_package_ids
-      end
-
-      assert_equal [1, 2], c.work_package_ids.sort
-      assert WorkPackage.find(1).closed?
-      assert WorkPackage.find(2).closed?
-
-      times = TimeEntry.order(Arel.sql('id desc')).limit(2)
-      assert_equal [1, 2], times.map(&:work_package_id).sort
-    end
-
-    it 'should ref keywords any line start' do
-      Setting.commit_ref_keywords = '*'
-
-      c = Changeset.new(repository: Project.find(1).repository,
-                        committed_on: Time.now,
-                        comments: '#1 is the reason of this commit')
-      c.scan_comment_for_work_package_ids
-
-      assert_equal [1], c.work_package_ids.sort
-    end
-
-    it 'should ref keywords allow brackets around a work package number' do
-      Setting.commit_ref_keywords = '*'
-
-      c = Changeset.new(repository: Project.find(1).repository,
-                        committed_on: Time.now,
-                        comments: '[#1] Worked on this work_package')
-      c.scan_comment_for_work_package_ids
-
-      assert_equal [1], c.work_package_ids.sort
-    end
-
-    it 'should ref keywords allow brackets around multiple work package numbers' do
-      Setting.commit_ref_keywords = '*'
-
-      c = Changeset.new(repository: Project.find(1).repository,
-                        committed_on: Time.now,
-                        comments: '[#1 #2, #3] Worked on these')
-      c.scan_comment_for_work_package_ids
-
-      assert_equal [1, 2, 3], c.work_package_ids.sort
-    end
-
-    it 'should commit referencing a subproject work package' do
-      c = Changeset.new(repository: Project.find(1).repository,
-                        committed_on: Time.now,
-                        comments: 'refs #5, a subproject work_package')
-      c.scan_comment_for_work_package_ids
-
-      assert_equal [5], c.work_package_ids.sort
-      assert c.work_packages.first.project != c.project
-    end
-
-    it 'should commit referencing a parent project work package' do
-      # repository of child project
-      r = Repository::Subversion.create!(
-        project: Project.find(3),
-        scm_type: 'existing',
-        url:      'svn://localhost/test')
-
-      c = Changeset.new(repository: r,
-                        committed_on: Time.now,
-                        comments: 'refs #2, an work_package of a parent project')
-      c.scan_comment_for_work_package_ids
-
-      assert_equal [2], c.work_package_ids.sort
-      assert c.work_packages.first.project != c.project
-    end
-
-    it 'should text tag revision' do
-      c = Changeset.new(revision: '520')
-      assert_equal 'r520', c.text_tag
-    end
-
-    it 'should text tag hash' do
-      c = Changeset.new(
-        scmid:    '7234cb2750b63f47bff735edc50a1c0a433c2518',
-        revision: '7234cb2750b63f47bff735edc50a1c0a433c2518')
-      assert_equal 'commit:7234cb2750b63f47bff735edc50a1c0a433c2518', c.text_tag
-    end
-
-    it 'should text tag hash all number' do
-      c = Changeset.new(scmid: '0123456789', revision: '0123456789')
-      assert_equal 'commit:0123456789', c.text_tag
-    end
-
-    it 'should previous' do
-      changeset = Changeset.find_by(revision: '3')
-      assert_equal Changeset.find_by(revision: '2'), changeset.previous
-    end
-
-    it 'should previous nil' do
-      changeset = Changeset.find_by(revision: '1')
-      assert_nil changeset.previous
-    end
-
-    it 'should next' do
-      changeset = Changeset.find_by(revision: '2')
-      assert_equal Changeset.find_by(revision: '3'), changeset.next
-    end
-
-    it 'should next nil' do
-      changeset = Changeset.find_by(revision: '10')
-      assert_nil changeset.next
-    end
-  end
-
-  context 'enabled scm', with_settings: { enabled_scm: ['subversion'] } do
-    it 'should comments empty' do
-      r = FactoryBot.create(:repository_subversion)
-
-      assert r
-      c = Changeset.new(repository: r,
-                        committed_on: Time.now,
-                        revision: '123',
-                        scmid: '12345',
-                        comments: '')
-      assert(c.save)
-      assert_equal '', c.comments
-      if c.comments.respond_to?(:force_encoding)
-        assert_equal 'UTF-8', c.comments.encoding.to_s
-      end
-    end
-
-    it 'should comments nil' do
-      r = FactoryBot.create(:repository_subversion)
-      assert r
-
-      c = Changeset.new(repository: r,
-                        committed_on: Time.now,
-                        revision: '123',
-                        scmid: '12345',
-                        comments: nil)
-      assert(c.save)
-      assert_equal '', c.comments
-      if c.comments.respond_to?(:force_encoding)
-        assert_equal 'UTF-8', c.comments.encoding.to_s
-      end
-    end
-
-    it 'should identifier' do
-      c = Changeset.find_by(revision: '1')
-      assert_equal c.revision, c.identifier
-    end
-  end
-end

spec_legacy/unit/repository_spec.rb

@@ -1,168 +0,0 @@
-#-- encoding: UTF-8
-
-#-- copyright
-# OpenProject is a project management system.
-# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License version 3.
-#
-# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
-# Copyright (C) 2006-2017 Jean-Philippe Lang
-# Copyright (C) 2010-2013 the ChiliProject Team
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-#
-# See docs/COPYRIGHT.rdoc for more details.
-#++
-require_relative '../legacy_spec_helper'
-
-describe Repository, type: :model do
-  fixtures :all
-
-  before do
-    @repository = Project.find(1).repository
-    Setting.enabled_scm = %w(subversion)
-  end
-
-  it 'should create' do
-    repository = Repository::Subversion.new(project: Project.find(3), scm_type: 'existing')
-    assert !repository.save
-
-    repository.url = 'svn://localhost'
-    assert repository.save
-    repository.reload
-
-    project = Project.find(3)
-    assert_equal repository, project.repository
-  end
-
-  it 'should destroy' do
-    changesets = Changeset.where('repository_id = 10').size
-    changes = Change.includes(:changeset)
-      .where("#{Changeset.table_name}.repository_id = 10")
-      .references(:changesets)
-      .size
-    assert_difference 'Changeset.count', -changesets do
-      assert_difference 'Change.count', -changes do
-        Repository.find(10).destroy
-      end
-    end
-  end
-
-  it 'should not create with disabled scm' do
-    Setting.enabled_scm = ['Git'] # disable Subversion
-    repository = Repository::Subversion.new(project: Project.find(3), scm_type: 'existing', url: 'svn://localhost')
-    assert !repository.save
-    assert_includes repository.errors[:type], I18n.translate('activerecord.errors.models.repository.not_available')
-    # re-enable Subversion for following tests
-    Setting.delete_all
-  end
-
-  it 'should scan changesets for work package ids' do
-    WorkPackage.all.each(&:recreate_initial_journal!)
-
-    Setting.default_language = 'en'
-    Setting.notified_events = ['work_package_added', 'work_package_updated']
-
-    # choosing a status to apply to fix issues
-    Setting.commit_fix_status_id = Status.where(['is_closed = ?', true]).first.id
-    Setting.commit_fix_done_ratio = '90'
-    Setting.commit_ref_keywords = 'refs , references, IssueID'
-    Setting.commit_fix_keywords = 'fixes , closes'
-    Setting.default_language = 'en'
-
-    # make sure work package 1 is not already closed
-    fixed_work_package = WorkPackage.find(1)
-    assert !fixed_work_package.status.is_closed?
-    old_status = fixed_work_package.status
-
-    Repository.scan_changesets_for_work_package_ids
-    assert_equal [101, 102], WorkPackage.find(3).changeset_ids
-
-    # fixed issues
-    fixed_work_package.reload
-    assert fixed_work_package.status.is_closed?
-    assert_equal 90, fixed_work_package.done_ratio
-    assert_equal [101], fixed_work_package.changeset_ids
-
-    # issue change
-    journal = fixed_work_package.journals.last
-    assert_equal User.find_by_login('dlopper'), journal.user
-    assert_equal 'Applied in changeset r2.', journal.notes
-
-    # 2 email notifications to 5 users
-    assert_equal 5, ActionMailer::Base.deliveries.size
-    mail = ActionMailer::Base.deliveries.first
-    assert_kind_of Mail::Message, mail
-    assert mail.subject.starts_with?("#{fixed_work_package.project.name} - #{fixed_work_package.status.name} #{fixed_work_package.type.name} ##{fixed_work_package.id}")
-    assert mail.body.encoded.include?(
-      "<strong>Status</strong> changed from <i title=\"#{old_status}\">#{old_status}</i> <br/><strong>to</strong> <i title=\"#{fixed_work_package.status}\">#{fixed_work_package.status}</i>"
-    )
-
-    # ignoring commits referencing an issue of another project
-    assert_equal [], WorkPackage.find(4).changesets
-  end
-
-  it 'should for changeset comments strip' do
-    repository = Repository::Subversion.create(
-      project: Project.find(4),
-      scm_type: 'existing',
-      url: 'svn://:login:password@host:/path/to/the/repository'
-    )
-    comment = 'This is a looooooooooooooong comment' + (' ' * 80 + "\n") * 5
-    changeset = Changeset.new(
-      comments: comment, commit_date: Time.now, revision: 0, scmid: 'f39b7922fb3c',
-      committer: 'foo <foo@example.com>', committed_on: Time.now, repository: repository)
-    assert(changeset.save)
-    refute_equal(comment, changeset.comments)
-    assert_equal('This is a looooooooooooooong comment', changeset.comments)
-  end
-
-  it 'should for urls strip' do
-    repository = Repository::Subversion.create(
-      project: Project.find(4),
-      url: ' svn://:login:password@host:/path/to/the/repository',
-      scm_type: 'existing',
-      log_encoding: 'UTF-8')
-    repository.root_url = 'foo  ' # can't mass-assign this attr
-    assert repository.save
-    repository.reload
-    assert_equal 'svn://:login:password@host:/path/to/the/repository', repository.url
-    assert_equal 'foo', repository.root_url
-  end
-
-  it 'should manual user mapping' do
-    assert_no_difference "Changeset.where('user_id <> 2').count" do
-      c = Changeset.create!(repository: @repository, committer: 'foo', committed_on: Time.now, revision: 100, comments: 'Committed by foo.')
-      assert_nil c.user
-      @repository.committer_ids = { 'foo' => '2' }
-      assert_equal User.find(2), c.reload.user
-      # committer is now mapped
-      c = Changeset.create!(repository: @repository, committer: 'foo', committed_on: Time.now, revision: 101, comments: 'Another commit by foo.')
-      assert_equal User.find(2), c.user
-    end
-  end
-
-  it 'should auto user mapping by username' do
-    c = Changeset.create!(repository: @repository, committer: 'jsmith', committed_on: Time.now, revision: 100, comments: 'Committed by john.')
-    assert_equal User.find(2), c.user
-  end
-
-  it 'should auto user mapping by email' do
-    c = Changeset.create!(repository: @repository, committer: 'john <jsmith@somenet.foo>', committed_on: Time.now, revision: 100, comments: 'Committed by john.')
-    assert_equal User.find(2), c.user
-  end
-end