Project

General

Profile

Actions

Defect #31116

closed

Database migrations don't run correctly for plugins when specifying the `VERSION` env variable

Added by Anonymous about 5 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Plugin API
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

tl;dr: When comparing versions, the current_version is set to the core's latest migration instead of the plugin's latest.

Steps to reproduce:

1. Install any plugin with migrations, for example:

cd redmine/plugins
git clone https://github.com/ixti/redmine_tags.git

bundle install
rake redmine:plugins:migrate NAME=redmine_tags

2. Undo the last migration of that plugin:

rake redmine:plugins:migrate NAME=redmine_tags VERSION=20151209153801

3. Migrate to the latest version again, but specifying the version this time:

rake redmine:plugins:migrate NAME=redmine_tags VERSION=20180122193833

Expected behaviour: the 20180122193833 migration is executed.
Actual behaviour: the 20180122193833 is not executed.

This is because redmine core's latest migration is 20180923091603 which is greather that the plugin's 20180122193833.

The attached fix defines a `migrate` method for the `Redmine::Plugin::MigrationContext` class that makes sure to pass along the current plugin's latest migration. Using the default migrate method defined in activerecord executes a `current_version` that looks for the most current migration in the list of all migrations (core and plugins).

Note: I have not run any tests with this patch.


Files

Actions #1

Updated by Anonymous about 5 years ago

Test run available here: https://gitlab.com/sdwolfz/redmine/-/jobs/187237412

Although... I am not sure how to add a test that checks for this exact error.

Actions #2

Updated by Go MAEDA about 5 years ago

  • Category set to Plugin API
  • Target version set to Candidate for next major release
Actions #3

Updated by Martijn Vernooij almost 5 years ago

I noticed the same issue (with 4.0.2 though) and 'fixed' it another way.

Actions #4

Updated by crypto gopher over 1 year ago

The problem stems from fact, that Redmine::Plugin::MigrationContext does not override current_version() method. So whenever ActiveRecord::MigrationContext.migrate() is executed, it calls ActiveRecord::MigrationContext.current_version() which does not differentiate between Redmine's migrations and plugin's migrations - yielding erroneous result.

The most straightforward change is to override ActiveRecord::MigrationContext.migrate() by using Redmine::Plugin::Migrator.current_version() which already provides required functionality:

class MigrationContext < ActiveRecord::MigrationContext
  ...

  def current_version
    Migrator.current_version
  end
end

Actions #5

Updated by Go MAEDA over 1 year ago

  • Status changed from New to Confirmed
Actions #6

Updated by Go MAEDA over 1 year ago

crypto gopher wrote:

The most straightforward change is to override ActiveRecord::MigrationContext.migrate() by using Redmine::Plugin::Migrator.current_version() which already provides required functionality:

Thank you for posting the patch.

I wrote a test for your patch as follows. Do you think the following code tests the right way?

diff --git a/test/unit/lib/redmine/plugin_test.rb b/test/unit/lib/redmine/plugin_test.rb
index a5a1b2aa3..24c06240c 100644
--- a/test/unit/lib/redmine/plugin_test.rb
+++ b/test/unit/lib/redmine/plugin_test.rb
@@ -217,4 +217,17 @@ class Redmine::PluginTest < ActiveSupport::TestCase

     assert Redmine::Plugin.migrate('foo_plugin')
   end
+
+  def test_migration_context_should_override_current_version
+    plugin = @klass.register :foo_plugin do
+      name 'Foo plugin'
+      version '0.0.1'
+    end
+    migration_dir = File.join(@klass.directory, 'db', 'migrate')
+
+    Redmine::Plugin::Migrator.current_plugin = plugin
+    context = Redmine::Plugin::MigrationContext.new(migration_dir, ::ActiveRecord::Base.connection.schema_migration)
+    # current_version should be zero because Foo plugin has no migration
+    assert_equal 0, context.current_version
+  end
 end
Actions #7

Updated by Go MAEDA over 1 year ago

  • Target version changed from Candidate for next major release to 5.1.0

Setting the target version to 5.1.0.

Actions #8

Updated by Go MAEDA over 1 year ago

  • Status changed from Confirmed to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch. Thank you for your contribution.

Actions

Also available in: Atom PDF