From 52feadde1728bfd73ce4d65747072636b68e2214 Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Tue, 9 Aug 2022 21:22:03 +0900
Subject: [PATCH 01/11] [WIP] Support renaming viewmodel classes in migrations

---
 lib/view_model/active_record/controller.rb    | 11 ++-
 lib/view_model/migratable_view.rb             | 42 +++++++-
 lib/view_model/migration.rb                   | 20 ++++
 .../migration/no_such_version_error.rb        | 25 +++++
 lib/view_model/migrator.rb                    | 99 ++++++++++++-------
 lib/view_model/registry.rb                    | 65 ++++++++++--
 lib/view_model/test_helpers/arvm_builder.rb   |  3 +
 test/helpers/arvm_test_utilities.rb           |  1 +
 test/helpers/viewmodel_spec_helpers.rb        |  5 +-
 .../active_record/migration_test.rb           | 78 +++++++++++++++
 test/unit/view_model/record_test.rb           |  2 +-
 11 files changed, 296 insertions(+), 55 deletions(-)
 create mode 100644 lib/view_model/migration/no_such_version_error.rb

diff --git a/lib/view_model/active_record/controller.rb b/lib/view_model/active_record/controller.rb
index 3c597306..b9ca10c1 100644
--- a/lib/view_model/active_record/controller.rb
+++ b/lib/view_model/active_record/controller.rb
@@ -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
diff --git a/lib/view_model/migratable_view.rb b/lib/view_model/migratable_view.rb
index 71e1493c..95c8bf96 100644
--- a/lib/view_model/migratable_view.rb
+++ b/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
+      @previous_name_cache = 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_previous_names! if @previous_name_cache.nil?
+        @previous_name_cache
+      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)
@@ -61,10 +76,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
+        @previous_name_cache = nil
+        @realized_paths = false
       end
     end
 
@@ -99,5 +124,16 @@ def realize_paths!
 
       @realized_paths = true
     end
+
+    def cache_previous_names!
+      name = self.view_name
+      @previous_name_cache =
+        self.schema_version.downto(1).to_h do |version|
+          if @previous_names.has_key?(version)
+            name = @previous_names[version]
+          end
+          [version, name]
+        end
+    end
   end
 end
diff --git a/lib/view_model/migration.rb b/lib/view_model/migration.rb
index 2c03d20f..40f2b6fa 100644
--- a/lib/view_model/migration.rb
+++ b/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)')
diff --git a/lib/view_model/migration/no_such_version_error.rb b/lib/view_model/migration/no_such_version_error.rb
new file mode 100644
index 00000000..9a84df3a
--- /dev/null
+++ b/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
diff --git a/lib/view_model/migrator.rb b/lib/view_model/migrator.rb
index 8554afda..f6153ca3 100644
--- a/lib/view_model/migrator.rb
+++ b/lib/view_model/migrator.rb
@@ -5,16 +5,16 @@ 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|
+          client_versions.each do |required_vm_class, client_version|
             name = required_vm_class.view_name
             if deep_schema_version.has_key?(name)
-              deep_schema_version[name] = required_version
+              deep_schema_version[name] = client_version
             end
           end
         end
@@ -23,16 +23,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 +81,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,26 +118,29 @@ 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
@@ -128,26 +148,31 @@ def migrate_viewmodel!(view_name, source_version, view_hash, references)
   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
diff --git a/lib/view_model/registry.rb b/lib/view_model/registry.rb
index 6456974f..c24ce31a 100644
--- a/lib/view_model/registry.rb
+++ b/lib/view_model/registry.rb
@@ -14,20 +14,33 @@ class << self
   def initialize
     @lock = Monitor.new
     @viewmodel_classes_by_name = {}
+    @viewmodel_classes_by_name_and_version = {}
     @deferred_viewmodel_classes = []
   end
 
-  def for_view_name(name)
+  def for_view_name(name, version: nil)
     raise ViewModel::DeserializationError::InvalidSyntax.new('ViewModel name cannot be nil') if name.nil?
 
     @lock.synchronize do
       # Resolve names for any deferred viewmodel classes
       resolve_deferred_classes
 
-      viewmodel_class = @viewmodel_classes_by_name[name]
-
-      if viewmodel_class.nil? || !(viewmodel_class < ViewModel)
-        raise ViewModel::DeserializationError::UnknownView.new(name)
+      viewmodel_class =
+        if version
+          versions_for_name = @viewmodel_classes_by_name_and_version.fetch(name) do
+            raise ViewModel::DeserializationError::UnknownView.new(name)
+          end
+          versions_for_name.fetch(version) do
+            raise ViewModel::Migration::NoSuchVersionError.new(@viewmodel_classes_by_name, version)
+          end
+        else
+          @viewmodel_classes_by_name.fetch(name) do
+            raise ViewModel::DeserializationError::UnknownView.new(name)
+          end
+        end
+
+      unless viewmodel_class < ViewModel
+        raise RuntimeError.new("Internal registry error, registered '#{name}' is not a viewmodel: #{viewmodel_class.inspect}")
       end
 
       viewmodel_class
@@ -65,7 +78,12 @@ def clear_removed_classes!
     @lock.synchronize do
       resolve_deferred_classes
       @viewmodel_classes_by_name.delete_if do |_name, klass|
-        !Kernel.const_defined?(klass.name)
+        !Kernel.const_defined?(klass.name) || Kernel.const_get(klass.name) != klass
+      end
+      @viewmodel_classes_by_name_and_version.each_value do |versions|
+        versions.delete_if do |_version, klass|
+          !Kernel.const_defined?(klass.name) || Kernel.const_get(klass.name) != klass
+        end
       end
     end
   end
@@ -75,10 +93,37 @@ def clear_removed_classes!
   def resolve_deferred_classes
     until @deferred_viewmodel_classes.empty?
       vm, view_name = @deferred_viewmodel_classes.pop
-
-      if vm.should_register?
-        view_name = vm.view_name if view_name == DEFERRED_NAME
-        @viewmodel_classes_by_name[view_name] = vm
+      next unless vm.should_register?
+
+      view_name = vm.view_name if view_name == DEFERRED_NAME
+      @viewmodel_classes_by_name[view_name] = vm
+
+      # Migratable views record their previous names. Other views have only the
+      # current version.
+      versioned_view_names =
+        if vm.respond_to?(:versioned_view_names)
+          vm.versioned_view_names
+        else
+          { vm.schema_version => vm.view_name }
+        end
+
+      versioned_view_names.each do |previous_version, previous_name|
+        versions_for_name = (@viewmodel_classes_by_name_and_version[previous_name] ||= {})
+
+        if (prev_vm = versions_for_name[previous_version]) && prev_vm != vm
+          # By recording the name/version pairs in this global registry, we're
+          # promising that a given (name, version) is always resolvable to a
+          # single viewmodel. This results in the constraint that there can
+          # never be two viewmodels that had the same name at the same version:
+          # if renaming a viewmodel would cause such a conflict, it is necessary
+          # to 'jump' schema versions at the same time as renaming to ensure
+          # there is no ambiguity.
+          raise RuntimeError.new(
+                  'No two viewmodel classes may have the same name at the same version: '\
+                  "#{vm.name} and #{prev_vm.name} named #{previous_name} at #{previous_version}")
+        end
+
+        versions_for_name[previous_version] = vm
       end
     end
   end
diff --git a/lib/view_model/test_helpers/arvm_builder.rb b/lib/view_model/test_helpers/arvm_builder.rb
index e027400c..7a8df098 100644
--- a/lib/view_model/test_helpers/arvm_builder.rb
+++ b/lib/view_model/test_helpers/arvm_builder.rb
@@ -70,6 +70,9 @@ def teardown
     if ActiveSupport::VERSION::MAJOR < 7
       ActiveSupport::Dependencies::Reference.clear!
     end
+
+    # Ensure the registry no longer refers to us
+    ViewModel::Registry.clear_removed_classes!
   end
 
   private
diff --git a/test/helpers/arvm_test_utilities.rb b/test/helpers/arvm_test_utilities.rb
index 5ad3b7d4..9157d0e5 100644
--- a/test/helpers/arvm_test_utilities.rb
+++ b/test/helpers/arvm_test_utilities.rb
@@ -33,6 +33,7 @@ def initialize(*)
   def after_all
     @viewmodels.each(&:teardown)
     @viewmodels.clear
+    ViewModel::Registry.clear_removed_classes!
     super
   end
 
diff --git a/test/helpers/viewmodel_spec_helpers.rb b/test/helpers/viewmodel_spec_helpers.rb
index d78269ef..ba37c2de 100644
--- a/test/helpers/viewmodel_spec_helpers.rb
+++ b/test/helpers/viewmodel_spec_helpers.rb
@@ -18,6 +18,7 @@ module Base
         @builders.each do |b|
           b.teardown
         end
+        ViewModel::Registry.clear_removed_classes!
       end
     end
 
@@ -143,8 +144,8 @@ def subject_association
 
   module ParentAndChildMigrations
     extend ActiveSupport::Concern
-  
-  def model_attributes
+
+    def model_attributes
       super.merge(
         schema: ->(t) { t.integer :new_field, default: 1, null: false },
         viewmodel: ->(_v) {
diff --git a/test/unit/view_model/active_record/migration_test.rb b/test/unit/view_model/active_record/migration_test.rb
index 801474ef..0f24f758 100644
--- a/test/unit/view_model/active_record/migration_test.rb
+++ b/test/unit/view_model/active_record/migration_test.rb
@@ -273,6 +273,84 @@ def new_model
     end
   end
 
+  describe 'renaming views' do
+    include ViewModelSpecHelpers::Single
+
+    def model_attributes
+      super.merge(
+        viewmodel: ->(_v) {
+          self.schema_version = 2
+          migrates from: 1, to: 2 do
+            down { |view, _refs| view['old_field'] = 'present' }
+            up   { |view, _refs| view.delete('old_field') }
+            renamed_from :OldName
+          end
+        },
+      )
+    end
+
+    def new_model
+      model_class.new(name: 'm1')
+    end
+
+    let(:migration_versions) { { viewmodel_class => 1 } }
+
+    let(:v1_serialization_data) do
+      {
+        ViewModel::TYPE_ATTRIBUTE => 'OldName',
+        ViewModel::VERSION_ATTRIBUTE => 1,
+        ViewModel::ID_ATTRIBUTE => viewmodel.id,
+        'name' => viewmodel.name,
+        'old_field' => 'present',
+      }
+    end
+
+    let(:v1_serialization_references) { {} }
+
+    let(:v1_serialization) do
+      {
+        'data' => v1_serialization_data,
+        'references' => v1_serialization_references,
+      }
+    end
+
+    describe 'down' do
+      let(:migrator) { down_migrator }
+      let(:migration_versions) { { viewmodel_class => 1 } }
+
+      let(:subject) { current_serialization.deep_dup }
+
+      let(:expected_result) do
+        v1_serialization.deep_merge({ 'data' => { ViewModel::MIGRATED_ATTRIBUTE => true } })
+      end
+
+      it 'migrates' do
+        migrate!
+        assert_equal(expected_result, subject)
+      end
+    end
+
+    describe 'up' do
+      let(:migrator) { up_migrator }
+      let(:subject) { v1_serialization.deep_dup }
+
+      let(:expected_result) do
+        current_serialization.deep_merge(
+          {
+            'data' => {
+              ViewModel::MIGRATED_ATTRIBUTE => true,
+            },
+          },
+        )
+      end
+
+      it 'migrates' do
+        migrate!
+        assert_equal(expected_result, subject)
+      end
+    end
+  end
+
   describe 'garbage collection' do
     include ViewModelSpecHelpers::ParentAndSharedBelongsToChild
 
diff --git a/test/unit/view_model/record_test.rb b/test/unit/view_model/record_test.rb
index a6019acd..f7afb434 100644
--- a/test/unit/view_model/record_test.rb
+++ b/test/unit/view_model/record_test.rb
@@ -476,7 +476,7 @@ def teardown
         if ActiveSupport::VERSION::MAJOR < 7
           ActiveSupport::Dependencies::Reference.clear!
         end
-
+        ViewModel::Registry.clear_removed_classes!
         super
       end
 

From 29edfc35c6c8a5bbc6be24e8c190d03b2f920425 Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Wed, 10 Aug 2022 14:28:05 +0900
Subject: [PATCH 02/11] Only record previous names for known schema versions

This means we can swap names by skipping version ranges
---
 lib/view_model/migratable_view.rb | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/lib/view_model/migratable_view.rb b/lib/view_model/migratable_view.rb
index 95c8bf96..c797e38d 100644
--- a/lib/view_model/migratable_view.rb
+++ b/lib/view_model/migratable_view.rb
@@ -45,7 +45,7 @@ def versioned_view_names
 
     def view_name_at_version(version)
       versioned_view_names.fetch(version) do
-       raise ViewModel::Migration::NoSuchVersionError.new(self, version)
+        raise ViewModel::Migration::NoSuchVersionError.new(self, version)
       end
     end
 
@@ -57,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
@@ -128,7 +139,7 @@ def realize_paths!
     def cache_previous_names!
       name = self.view_name
       @previous_name_cache =
-        self.schema_version.downto(1).to_h do |version|
+        known_schema_versions.reverse_each.to_h do |version|
           if @previous_names.has_key?(version)
             name = @previous_names[version]
           end

From f6dc7830e0eae125120564ab93fa253c8c3b29cc Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Wed, 10 Aug 2022 14:45:24 +0900
Subject: [PATCH 03/11] Exercise viewmodel renaming in controller tests

---
 test/helpers/controller_test_helpers.rb               |  2 ++
 test/unit/view_model/active_record/controller_test.rb | 11 ++++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/test/helpers/controller_test_helpers.rb b/test/helpers/controller_test_helpers.rb
index 6ad9d992..7513d7b0 100644
--- a/test/helpers/controller_test_helpers.rb
+++ b/test/helpers/controller_test_helpers.rb
@@ -108,6 +108,8 @@ def build_controller_test_models(externalize: [])
           down do |view, _refs|
             view['old_name'] = view.delete('name')
           end
+
+          renamed_from :PreviousParent
         end
       end
     end
diff --git a/test/unit/view_model/active_record/controller_test.rb b/test/unit/view_model/active_record/controller_test.rb
index fdcc7b34..669b5101 100644
--- a/test/unit/view_model/active_record/controller_test.rb
+++ b/test/unit/view_model/active_record/controller_test.rb
@@ -46,15 +46,18 @@ def test_show
   end
 
   def test_migrated_show
+    assert_equal(ParentView.view_name_at_version(1), 'PreviousParent')
+
     parentcontroller = ParentController.new(
       params: { id: @parent.id },
-      headers: { 'X-ViewModel-Versions' => { ParentView.view_name => 1 }.to_json })
+      headers: { 'X-ViewModel-Versions' => { 'PreviousParent' => 1 }.to_json })
 
     parentcontroller.invoke(:show)
 
     expected_view = @parent_view.to_hash
                       .except('name')
                       .merge('old_name' => @parent.name,
+                             ViewModel::TYPE_ATTRIBUTE => 'PreviousParent',
                              ViewModel::VERSION_ATTRIBUTE => 1,
                              ViewModel::MIGRATED_ATTRIBUTE => true)
 
@@ -118,13 +121,15 @@ def test_create
   end
 
   def test_migrated_create
+    assert_equal(ParentView.view_name_at_version(1), 'PreviousParent')
+
     data = {
-      '_type'    => 'Parent',
+      '_type'    => 'PreviousParent',
       '_version' => 1,
       'old_name' => 'p2',
     }
 
-    parentcontroller = ParentController.new(params: { data: data, versions: { ParentView.view_name => 1 } })
+    parentcontroller = ParentController.new(params: { data: data, versions: { 'PreviousParent' => 1 } })
     parentcontroller.invoke(:create)
 
     assert_equal(200, parentcontroller.status)

From 7804cd396617d914a72c56f6fd0dddb758e9dd5a Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Wed, 10 Aug 2022 14:45:46 +0900
Subject: [PATCH 04/11] Bump gem version

---
 lib/iknow_view_models/version.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/iknow_view_models/version.rb b/lib/iknow_view_models/version.rb
index b44ae3f5..6128b914 100644
--- a/lib/iknow_view_models/version.rb
+++ b/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

From 658cc40f5ee0a0a9189d984adc2bdfbf66d0266b Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Wed, 10 Aug 2022 17:13:56 +0900
Subject: [PATCH 05/11] Make the gemspec independent of cwd

---
 iknow_view_models.gemspec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/iknow_view_models.gemspec b/iknow_view_models.gemspec
index 81bfb0f8..21605295 100644
--- a/iknow_view_models.gemspec
+++ b/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']

From c6899f30c0e6210ca413bbbc1ec432e8e70bee3f Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Thu, 18 Aug 2022 11:34:12 +0900
Subject: [PATCH 06/11] squash: review formatting nitpick

Co-authored-by: Andrew Childs <andrew.childs@bibo.com.ph>
---
 test/unit/view_model/active_record/migration_test.rb | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/test/unit/view_model/active_record/migration_test.rb b/test/unit/view_model/active_record/migration_test.rb
index 0f24f758..ecef95b7 100644
--- a/test/unit/view_model/active_record/migration_test.rb
+++ b/test/unit/view_model/active_record/migration_test.rb
@@ -335,13 +335,7 @@ def new_model
       let(:subject) { v1_serialization.deep_dup }
 
       let(:expected_result) do
-        current_serialization.deep_merge(
-          {
-            'data' => {
-              ViewModel::MIGRATED_ATTRIBUTE => true,
-            },
-          },
-        )
+        current_serialization.deep_merge({ 'data' => { ViewModel::MIGRATED_ATTRIBUTE => true } })
       end
 
       it 'migrates' do

From 209a3e765655d50157f38be34dcbde4aeb2fbc28 Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Thu, 18 Aug 2022 11:43:59 +0900
Subject: [PATCH 07/11] squash: naming

---
 lib/view_model/migrator.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/view_model/migrator.rb b/lib/view_model/migrator.rb
index f6153ca3..7d1e4e1e 100644
--- a/lib/view_model/migrator.rb
+++ b/lib/view_model/migrator.rb
@@ -11,8 +11,8 @@ def migrated_deep_schema_version(viewmodel_class, client_versions, include_refer
         if client_versions.present?
           deep_schema_version = deep_schema_version.dup
 
-          client_versions.each do |required_vm_class, client_version|
-            name = required_vm_class.view_name
+          client_versions.each do |vm_class, client_version|
+            name = vm_class.view_name
             if deep_schema_version.has_key?(name)
               deep_schema_version[name] = client_version
             end

From 698e0f2c59d122207fb9ec703b79ece53afcd680 Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Thu, 18 Aug 2022 12:12:22 +0900
Subject: [PATCH 08/11] fixup: use old view name when computing migrated deep
 schema version

---
 lib/view_model/migrator.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/view_model/migrator.rb b/lib/view_model/migrator.rb
index 7d1e4e1e..3cdb1775 100644
--- a/lib/view_model/migrator.rb
+++ b/lib/view_model/migrator.rb
@@ -13,8 +13,10 @@ def migrated_deep_schema_version(viewmodel_class, client_versions, include_refer
 
           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] = client_version
+              deep_schema_version.delete(name)
+              deep_schema_version[name_at_client_version] = client_version
             end
           end
         end

From e2b4a5fd73d98fa4e6ea215cc84c1e8779e3dbed Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Thu, 18 Aug 2022 12:12:49 +0900
Subject: [PATCH 09/11] Use both new and old schema versions when calculating
 migrated etag

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.
---
 lib/view_model/active_record/controller.rb | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/view_model/active_record/controller.rb b/lib/view_model/active_record/controller.rb
index b9ca10c1..8f22b940 100644
--- a/lib/view_model/active_record/controller.rb
+++ b/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
@@ -141,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

From ce7cb6ec94b1282854e4a881257ecabb7c9e7a35 Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Thu, 18 Aug 2022 13:03:25 +0900
Subject: [PATCH 10/11] fixup: guard against two VMs with the same current name
 as well

---
 lib/view_model/registry.rb | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/view_model/registry.rb b/lib/view_model/registry.rb
index c24ce31a..6bcf653f 100644
--- a/lib/view_model/registry.rb
+++ b/lib/view_model/registry.rb
@@ -96,6 +96,13 @@ def resolve_deferred_classes
       next unless vm.should_register?
 
       view_name = vm.view_name if view_name == DEFERRED_NAME
+
+      if (prev_vm = @viewmodel_classes_by_name[view_name]) && prev_vm != vm
+        raise RuntimeError.new(
+                'No two viewmodel classes may have the same name: ' \
+                "#{vm.name} and #{prev_vm.name} named #{view_name}")
+      end
+
       @viewmodel_classes_by_name[view_name] = vm
 
       # Migratable views record their previous names. Other views have only the
@@ -119,7 +126,7 @@ def resolve_deferred_classes
           # to 'jump' schema versions at the same time as renaming to ensure
           # there is no ambiguity.
           raise RuntimeError.new(
-                  'No two viewmodel classes may have the same name at the same version: '\
+                  'No two viewmodel classes may have the same name at the same version: ' \
                   "#{vm.name} and #{prev_vm.name} named #{previous_name} at #{previous_version}")
         end
 

From 76d74b3598ba6540bd28a187ac856ead06eb9500 Mon Sep 17 00:00:00 2001
From: Chris Andreae <chris@bibo.com.ph>
Date: Thu, 18 Aug 2022 13:10:43 +0900
Subject: [PATCH 11/11] fixup: renaming suggested in review

---
 lib/view_model/migratable_view.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/view_model/migratable_view.rb b/lib/view_model/migratable_view.rb
index c797e38d..ad246988 100644
--- a/lib/view_model/migratable_view.rb
+++ b/lib/view_model/migratable_view.rb
@@ -21,7 +21,7 @@ def initialize_as_migratable_view
       @migration_paths   = {}
       @previous_names    = {}
       @realized_paths    = true
-      @previous_name_cache = nil
+      @versioned_view_names = nil
     end
 
     def migration_path(from:, to:)
@@ -38,8 +38,8 @@ def migration_path(from:, to:)
 
     def versioned_view_names
       @migrations_lock.synchronize do
-        cache_previous_names! if @previous_name_cache.nil?
-        @previous_name_cache
+        cache_versioned_view_names! if @versioned_view_names.nil?
+        @versioned_view_names
       end
     end
 
@@ -99,7 +99,7 @@ def migrates(from:, to:, inherit: nil, at: nil, &block)
         const_set(:"Migration_#{from}_To_#{to}", migration_class)
         @migration_classes[[from, to]] = migration_class
 
-        @previous_name_cache = nil
+        @versioned_view_names = nil
         @realized_paths = false
       end
     end
@@ -136,9 +136,9 @@ def realize_paths!
       @realized_paths = true
     end
 
-    def cache_previous_names!
+    def cache_versioned_view_names!
       name = self.view_name
-      @previous_name_cache =
+      @versioned_view_names =
         known_schema_versions.reverse_each.to_h do |version|
           if @previous_names.has_key?(version)
             name = @previous_names[version]