[WIP] Support renaming viewmodel classes in migrations

iknow_view_models.gemspec

@@ -14,7 +14,7 @@ Gem::Specification.new do |spec|
   spec.homepage      = 'https://github.com/iknow/cerego_view_models'
   spec.license       = 'MIT'
 
-  spec.files         = `git ls-files -z`.split("\x0")
+  spec.files         = `cd #{__dir__} && git ls-files -z`.split("\x0")
   spec.executables   = spec.files.grep(%r{^bin/}) { |f| File.basename(f) }
   spec.test_files    = spec.files.grep(%r{^(test|spec|features)/})
   spec.require_paths = ['lib']

lib/iknow_view_models/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module IknowViewModels
-  VERSION = '3.6.5'
+  VERSION = '3.6.6'
 end

lib/view_model/active_record/controller.rb

@@ -64,7 +64,7 @@ def destroy(serialize_context: new_serialize_context, deserialize_context: new_d
   end
 
   included do
-    etag { migrated_deep_schema_version }
+    etag { migrated_deep_schema_version_key }
   end
 
   def parse_viewmodel_updates
@@ -120,10 +120,17 @@ def migration_versions
         migration_versions = {}
 
         versions.each do |view_name, required_version|
-          viewmodel_class = ViewModel::Registry.for_view_name(view_name)
+          viewmodel_class = ViewModel::Registry.for_view_name(view_name, version: required_version)
 
           if viewmodel_class.schema_version != required_version
-            migration_versions[viewmodel_class] = required_version
+            if migration_versions.has_key?(viewmodel_class) && migration_versions[viewmodel_class] != required_version
+              raise ViewModel::Error.new(
+                      status: 400,
+                      code: 'ViewModel.InconsistentMigration',
+                      detail: "Viewmodel #{viewmodel_class.view_name} specified twice with different versions (as '#{view_name}')")
+            else
+              migration_versions[viewmodel_class] = required_version
+            end
           end
         rescue ViewModel::DeserializationError::UnknownView
           # Ignore requests to migrate types that no longer exist
@@ -134,7 +141,15 @@ def migration_versions
       end
   end
 
-  def migrated_deep_schema_version
-    ViewModel::Migrator.migrated_deep_schema_version(viewmodel_class, migration_versions, include_referenced: true)
+  # To identify a migrated schema version for caching purposes, we need to use
+  # both the current and target schema versions. Otherwise if we were to make
+  # future migrations that would affect the result when migrating to older views
+  # (e.g. by discarding and reconstructing data), the cache would not be
+  # invalidated and cached results would be different from computed ones.
+  def migrated_deep_schema_version_key
+    {
+      from: viewmodel_class.deep_schema_version(include_referenced: true),
+      to: ViewModel::Migrator.migrated_deep_schema_version(viewmodel_class, migration_versions, include_referenced: true),
+    }
   end
 end

lib/view_model/migratable_view.rb

@@ -19,12 +19,14 @@ def initialize_as_migratable_view
       @migrations_lock   = Monitor.new
       @migration_classes = {}
       @migration_paths   = {}
-      @realized_migration_paths = true
+      @previous_names    = {}
+      @realized_paths    = true
+      @versioned_view_names = nil
     end
 
     def migration_path(from:, to:)
       @migrations_lock.synchronize do
-        realize_paths! unless @realized_migration_paths
+        realize_paths! unless @realized_paths
 
         migrations = @migration_paths.fetch([from, to]) do
           raise ViewModel::Migration::NoPathError.new(self, from, to)
@@ -34,6 +36,19 @@ def migration_path(from:, to:)
       end
     end
 
+    def versioned_view_names
+      @migrations_lock.synchronize do
+        cache_versioned_view_names! if @versioned_view_names.nil?
+        @versioned_view_names
+      end
+    end
+
+    def view_name_at_version(version)
+      versioned_view_names.fetch(version) do
+        raise ViewModel::Migration::NoSuchVersionError.new(self, version)
+      end
+    end
+
     protected
 
     def migration_class(from, to)
@@ -42,6 +57,17 @@ def migration_class(from, to)
       end
     end
 
+    def known_schema_versions
+      @migrations_lock.synchronize do
+        realize_paths! unless @realized_paths
+        versions = Set.new([schema_version])
+        @migration_paths.each_key do |from, to|
+          versions << from << to
+        end
+        versions.to_a.sort
+      end
+    end
+
     private
 
     # Define a migration on this viewmodel
@@ -61,10 +87,20 @@ def migrates(from:, to:, inherit: nil, at: nil, &block)
 
         migration_class = builder.build!
 
+        if migration_class.renamed?
+          old_name = migration_class.renamed_from
+          if @previous_names.has_key?(from)
+            raise ArgumentError.new("Inconsistent previous naming for version #{from}") if @previous_names[from] != old_name
+          else
+            @previous_names[from] = old_name
+          end
+        end
+
         const_set(:"Migration_#{from}_To_#{to}", migration_class)
         @migration_classes[[from, to]] = migration_class
 
-        @realized_migration_paths = false
+        @versioned_view_names = nil
+        @realized_paths = false
       end
     end
 
@@ -99,5 +135,16 @@ def realize_paths!
 
       @realized_paths = true
     end
+
+    def cache_versioned_view_names!
+      name = self.view_name
+      @versioned_view_names =
+        known_schema_versions.reverse_each.to_h do |version|
+          if @previous_names.has_key?(version)
+            name = @previous_names[version]
+          end
+          [version, name]
+        end
+    end
   end
 end

lib/view_model/migration.rb

@@ -13,18 +13,34 @@ def down(view, _references)
     raise ViewModel::Migration::OneWayError.new(view[ViewModel::TYPE_ATTRIBUTE], :down)
   end
 
+  def self.renamed_from
+    nil
+  end
+
+  def self.renamed?
+    renamed_from.present?
+  end
+
+  delegate :renamed_from, :renamed?, to: :class
+
   # Tiny DSL for defining migration classes
   class Builder
     def initialize(superclass = ViewModel::Migration)
       @superclass = superclass
       @up_block = nil
       @down_block = nil
+      @renamed_from = nil
     end
 
     def build!
       migration = Class.new(@superclass)
       migration.define_method(:up, &@up_block) if @up_block
       migration.define_method(:down, &@down_block) if @down_block
+
+      # unconditionally define renamed_from: unlike up and down blocks, we do
+      # not want to inherit previous view names.
+      renamed_from = @renamed_from
+      migration.define_singleton_method(:renamed_from) { renamed_from }
       migration
     end
 
@@ -40,6 +56,10 @@ def down(&block)
       @down_block = block
     end
 
+    def renamed_from(name)
+      @renamed_from = name.to_s
+    end
+
     def check_signature!(block)
       unless block.arity == 2
         raise RuntimeError.new('Illegal signature for migration method, must be (view, references)')

lib/view_model/migration/no_such_version_error.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class ViewModel::Migration::NoSuchVersionError < ViewModel::AbstractError
+  attr_reader :vm_name, :version
+
+  status 400
+  code 'Migration.NoSuchVersionError'
+
+  def initialize(viewmodel, version)
+    @vm_name = viewmodel.view_name
+    @version = version
+    super()
+  end
+
+  def detail
+    "No version found for #{vm_name} at version #{version}"
+  end
+
+  def meta
+    {
+      viewmodel: vm_name,
+      version: version,
+    }
+  end
+end

lib/view_model/migrator.rb

@@ -5,16 +5,18 @@ class Migrator
     EXCLUDE_FROM_MIGRATION = '_exclude_from_migration'
 
     class << self
-      def migrated_deep_schema_version(viewmodel_class, required_versions, include_referenced: true)
+      def migrated_deep_schema_version(viewmodel_class, client_versions, include_referenced: true)
         deep_schema_version = viewmodel_class.deep_schema_version(include_referenced: include_referenced)
 
-        if required_versions.present?
+        if client_versions.present?
           deep_schema_version = deep_schema_version.dup
 
-          required_versions.each do |required_vm_class, required_version|
-            name = required_vm_class.view_name
+          client_versions.each do |vm_class, client_version|
+            name = vm_class.view_name
+            name_at_client_version = vm_class.view_name_at_version(client_version)
             if deep_schema_version.has_key?(name)
-              deep_schema_version[name] = required_version
+              deep_schema_version.delete(name)
+              deep_schema_version[name_at_client_version] = client_version
             end
           end
         end
@@ -23,16 +25,27 @@ def migrated_deep_schema_version(viewmodel_class, required_versions, include_ref
       end
     end
 
-    def initialize(required_versions)
-      @paths = required_versions.each_with_object({}) do |(viewmodel_class, required_version), h|
-        if required_version != viewmodel_class.schema_version
-          path = viewmodel_class.migration_path(from: required_version, to: viewmodel_class.schema_version)
-          h[viewmodel_class.view_name] = path
-        end
+    MigrationDetail = Value.new(:viewmodel_class, :path, :client_name, :client_version) do
+      def current_name
+        viewmodel_class.view_name
+      end
+
+      def current_version
+        viewmodel_class.schema_version
       end
+    end
 
-      @versions = required_versions.each_with_object({}) do |(viewmodel_class, required_version), h|
-        h[viewmodel_class.view_name] = [required_version, viewmodel_class.schema_version]
+    def initialize(client_versions)
+      @migrations = client_versions.each_with_object({}) do |(viewmodel_class, client_version), h|
+        next if client_version == viewmodel_class.schema_version
+
+        path = viewmodel_class.migration_path(from: client_version, to: viewmodel_class.schema_version)
+        client_name = viewmodel_class.view_name_at_version(client_version)
+        detail = MigrationDetail.new(viewmodel_class, path, client_name, client_version)
+
+        # Index by the name we expect to see in the tree to be migrated (client
+        # name for up, current name for down)
+        h[source_name(detail)] = detail
       end
     end
 
@@ -70,6 +83,12 @@ def migrate_tree!(node, references:)
     def migrate_viewmodel!(_view_name, _version, _view_hash, _references)
       raise RuntimeError.new('abstract method')
     end
+
+    # What name is expected for a given view in the to-be-migrated source tree.
+    # Varies between up and down migration.
+    def source_name(_migration_detail)
+      raise RuntimeError.new('abstract method')
+    end
   end
 
   class UpMigrator < Migrator
@@ -101,53 +120,61 @@ def migrate_functional_update!(node, references:)
       end
     end
 
-    def migrate_viewmodel!(view_name, source_version, view_hash, references)
-      path = @paths[view_name]
-      return false unless path
+    def migrate_viewmodel!(source_name, source_version, view_hash, references)
+      migration = @migrations[source_name]
+      return false unless migration
 
       # We assume that an unspecified source version is the same as the required
-      # version.
-      required_version, current_version = @versions[view_name]
-
-      unless source_version.nil? || source_version == required_version
-        raise ViewModel::Migration::UnspecifiedVersionError.new(view_name, source_version)
+      # client version.
+      unless source_version.nil? || source_version == migration.client_version
+        raise ViewModel::Migration::UnspecifiedVersionError.new(source_name, source_version)
       end
 
-      path.each do |migration|
-        migration.up(view_hash, references)
+      migration.path.each do |step|
+        step.up(view_hash, references)
       end
 
-      view_hash[ViewModel::VERSION_ATTRIBUTE] = current_version
+      view_hash[ViewModel::TYPE_ATTRIBUTE]    = migration.current_name
+      view_hash[ViewModel::VERSION_ATTRIBUTE] = migration.current_version
 
       true
     end
+
+    def source_name(migration_detail)
+      migration_detail.client_name
+    end
   end
 
   # down migrations find a reverse path from the current schema version to the
   # specific version requested by the client.
   class DownMigrator < Migrator
     private
 
-    def migrate_viewmodel!(view_name, source_version, view_hash, references)
-      path = @paths[view_name]
-      return false unless path
-
-      # In a serialized output, the source version should always be the present
-      # and the current version, unless already modified by a parent migration
-      required_version, current_version = @versions[view_name]
-      return false if source_version == required_version
-
-      unless source_version == current_version
-        raise ViewModel::Migration::UnspecifiedVersionError.new(view_name, source_version)
+    def migrate_viewmodel!(source_name, source_version, view_hash, references)
+      migration = @migrations[source_name]
+      return false unless migration
+
+      # In a serialized output, the source version should always be present and
+      # the current version, unless already modified by a parent migration (in
+      # which case there's nothing to be done).
+      if source_version == migration.client_version
+        return false
+      elsif source_version != migration.current_version
+        raise ViewModel::Migration::UnspecifiedVersionError.new(source_name, source_version)
       end
 
-      path.reverse_each do |migration|
-        migration.down(view_hash, references)
+      migration.path.reverse_each do |step|
+        step.down(view_hash, references)
       end
 
-      view_hash[ViewModel::VERSION_ATTRIBUTE] = required_version
+      view_hash[ViewModel::TYPE_ATTRIBUTE]    = migration.client_name
+      view_hash[ViewModel::VERSION_ATTRIBUTE] = migration.client_version
 
       true
     end
+
+    def source_name(migration_detail)
+      migration_detail.current_name
+    end
   end
 end