Migration validations

lib/iknow_view_models/version.rb

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

lib/view_model/migratable_view.rb

@@ -10,79 +10,145 @@ module ViewModel::MigratableView
   extend ActiveSupport::Concern
 
   class_methods do
-    def inherited(base)
+    def inherited(subclass)
       super
-      base.initialize_as_migratable_view
+      subclass.initialize_as_migratable_view
     end
 
     def initialize_as_migratable_view
-      @migrations_lock   = Monitor.new
-      @migration_classes = {}
-      @migration_paths   = {}
-      @realized_migration_paths = true
+      @_migrations = MigrationSet.new(self)
     end
 
-    def migration_path(from:, to:)
-      @migrations_lock.synchronize do
-        realize_paths! unless @realized_migration_paths
+    delegate :migration_path, :migration_class, to: :@_migrations
 
-        migrations = @migration_paths.fetch([from, to]) do
-          raise ViewModel::Migration::NoPathError.new(self, from, to)
-        end
+    private
 
-        migrations
-      end
+    # Helper for defining migrations exhaustively: migrations are defined in the
+    # block, and then validated for completeness on its return.
+    def migrations(&block)
+      @_migrations.build!(&block)
+    end
+  end
+
+  class MigrationSet
+    def initialize(viewmodel)
+      @viewmodel = viewmodel
+      @migration_classes = {}
+      @migration_paths = {}
+      @mentioned_versions = Set.new
     end
 
-    protected
+    def build!(&block)
+      instance_exec(&block)
+      realize_paths!
+    end
+
+    def migration_path(from:, to:)
+      @migration_paths.fetch([from, to]) do
+        raise ViewModel::Migration::NoPathError.new(@viewmodel, from, to)
+      end
+    end
 
     def migration_class(from, to)
       @migration_classes.fetch([from, to]) do
-        raise ViewModel::Migration::NoPathError.new(self, from, to)
+        raise ViewModel::Migration::NoPathError.new(@viewmodel, from, to)
       end
     end
 
     private
 
-    # Define a migration on this viewmodel
     def migrates(from:, to:, inherit: nil, at: nil, &block)
-      @migrations_lock.synchronize do
-        migration_superclass =
-          if inherit
-            raise ArgumentError.new('Must provide inherit version') unless at
-
-            inherit.migration_class(at - 1, at)
-          else
-            ViewModel::Migration
-          end
+      migration_superclass =
+        if inherit
+          raise ArgumentError.new('Must provide inherit version') unless at
+
+          inherit.migration_class(at - 1, at)
+        else
+          ViewModel::Migration
+        end
 
-        builder = ViewModel::Migration::Builder.new(migration_superclass)
-        builder.instance_exec(&block)
+      builder = ViewModel::Migration::Builder.new(migration_superclass)
+      builder.instance_exec(&block)
 
-        migration_class = builder.build!
+      migration_class = builder.build!
 
-        const_set(:"Migration_#{from}_To_#{to}", migration_class)
-        @migration_classes[[from, to]] = migration_class
+      @viewmodel.const_set(:"Migration_#{from}_To_#{to}", migration_class)
+      @migration_classes[[from, to]] = migration_class
+      @mentioned_versions << from
+    end
 
-        @realized_migration_paths = false
+    # Migration helper for common migration actions
+    #
+    # @param adding_fields list of fields added
+    # @param removing_fields map of removed field to default value
+    # @param renaming_fields
+    #   map of name in the `from` version to the name in the `to` version
+    # @param [Number] from from version
+    # @param [Number] to to version
+    def migrates_by(adding_fields: [], removing_fields: {}, renaming_fields: {}, from:, to:)
+      adding_fields   = adding_fields.map(&:to_s)
+      removing_fields = removing_fields.map { |(k, v)| [k.to_s, v.freeze] }
+      renaming_fields = renaming_fields.map { |(k, v)| [k.to_s, v.to_s] }
+
+      migrates from: from, to: to do
+        down do |view, refs|
+          # Hide newly created fields
+          adding_fields.each { |f| view.delete(f) }
+
+          # Add dummy values for removed fields
+          removing_fields.each do |from_name, default_value|
+            view[from_name] =
+              if default_value.is_a?(Proc)
+                default_value.call(view, refs)
+              else
+                default_value
+              end
+          end
+
+          renaming_fields.each do |from_name, to_name|
+            view[from_name] = view.delete(to_name)
+          end
+        end
+        up do |view, _refs|
+          # Silently drop updates to removed fields
+          removing_fields.each_key do |from_name|
+            view.delete(from_name)
+          end
+
+          renaming_fields.each do |from_name, to_name|
+            view[to_name] = view.delete(from_name) if view.has_key?(from_name)
+          end
+        end
       end
     end
 
+    # Define a simple migration for added optional fields, with a down-migration
+    # removing them and an empty up-migration.
+    def migrates_adding_fields(*fields, from:, to:)
+      migrates_by(adding_fields: fields, from: from, to: to)
+    end
+
+    def migrates_renaming_fields(fields, from:, to:)
+      migrates_by(renaming_fields: fields, from: from, to: to)
+    end
+
+    def no_migration_from!(version)
+      @mentioned_versions << version
+    end
+
     # Internal: find and record possible paths to the current schema version.
     def realize_paths!
-      @migration_paths.clear
-
       graph = RGL::DirectedAdjacencyGraph.new
 
       # Add a vertex for the current version, in case no edges reach it
-      graph.add_vertex(self.schema_version)
+      graph.add_vertex(@viewmodel.schema_version)
 
       # Add edges backwards, as we care about paths from the latest version
       @migration_classes.each_key do |from, to|
         graph.add_edge(to, from)
       end
 
-      paths = graph.dijkstra_shortest_paths(Hash.new { 1 }, self.schema_version)
+      paths = graph.dijkstra_shortest_paths(Hash.new { 1 }, @viewmodel.schema_version)
 
       paths.each do |target_version, path|
         next if path.nil? || path.length == 1
@@ -92,12 +158,19 @@ def realize_paths!
           @migration_classes.fetch([from, to])
         end
 
-        key = [target_version, schema_version]
+        key = [target_version, @viewmodel.schema_version]
 
         @migration_paths[key] = path_migration_classes.map(&:new)
       end
 
-      @realized_paths = true
+      # Ensure that all versions up to schema_version are either specified in a
+      # migration, or declared as `no_migration!`. This does not imply that
+      # every version is reachable, but merely that every version is mentioned.
+      (1 ... @viewmodel.schema_version).each do |target_version|
+        unless @mentioned_versions.include?(target_version)
+          raise ViewModel::Migration::MigrationsIncompleteError.new(@viewmodel, target_version)
+        end
+      end
     end
   end
 end

lib/view_model/migration.rb

@@ -4,6 +4,7 @@ class ViewModel::Migration
   require 'view_model/migration/no_path_error'
   require 'view_model/migration/one_way_error'
   require 'view_model/migration/unspecified_version_error'
+  require 'view_model/migration/migrations_incomplete_error'
 
   def up(view, _references)
     raise ViewModel::Migration::OneWayError.new(view[ViewModel::TYPE_ATTRIBUTE], :up)

lib/view_model/migration/migrations_incomplete_error.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class ViewModel::Migration::MigrationsIncompleteError < ViewModel::AbstractError
+  attr_reader :vm_name, :version
+
+  status 400
+  code 'Migration.MigrationsIncomplete'
+
+  def initialize(viewmodel, version)
+    @vm_name = viewmodel.view_name
+    @version = version
+    super()
+  end
+
+  def detail
+    "Viewmodel '#{vm_name}' neither defines a migration reaching client version #{version} nor explicitly excludes it"
+  end
+
+  def meta
+    {
+      viewmodel: vm_name,
+      version: version,
+    }
+  end
+end

test/helpers/controller_test_helpers.rb

@@ -98,15 +98,17 @@ def build_controller_test_models(externalize: [])
         association  :category, external: true
         association :tags, through: :parent_tags, external: true
 
-        migrates from: 1, to: 2 do
-          up do |view, _refs|
-            if view.has_key?('old_name')
-              view['name'] = view.delete('old_name')
+        migrations do
+          migrates from: 1, to: 2 do
+            up do |view, _refs|
+              if view.has_key?('old_name')
+                view['name'] = view.delete('old_name')
+              end
             end
-          end
 
-          down do |view, _refs|
-            view['old_name'] = view.delete('name')
+            down do |view, _refs|
+              view['old_name'] = view.delete('name')
+            end
           end
         end
       end

test/helpers/viewmodel_spec_helpers.rb

@@ -152,36 +152,38 @@ def model_attributes
 
           attribute :new_field
 
-          # add: old_field (one-way)
-          migrates from: 1, to: 2 do
-            down do |view, _refs|
-              view.delete('old_field')
+          migrations do
+            # add: old_field (one-way)
+            migrates from: 1, to: 2 do
+              down do |view, _refs|
+                view.delete('old_field')
+              end
             end
-          end
 
-          # rename: old_field -> mid_field
-          migrates from: 2, to: 3 do
-            up do |view, _refs|
-              if view.has_key?('old_field')
-                view['mid_field'] = view.delete('old_field') + 1
+            # rename: old_field -> mid_field
+            migrates from: 2, to: 3 do
+              up do |view, _refs|
+                if view.has_key?('old_field')
+                  view['mid_field'] = view.delete('old_field') + 1
+                end
               end
-            end
 
-            down do |view, _refs|
-              view['old_field'] = view.delete('mid_field') - 1
+              down do |view, _refs|
+                view['old_field'] = view.delete('mid_field') - 1
+              end
             end
-          end
 
-          # rename: mid_field -> new_field
-          migrates from: 3, to: 4 do
-            up do |view, _refs|
-              if view.has_key?('mid_field')
-                view['new_field'] = view.delete('mid_field') + 1
+            # rename: mid_field -> new_field
+            migrates from: 3, to: 4 do
+              up do |view, _refs|
+                if view.has_key?('mid_field')
+                  view['new_field'] = view.delete('mid_field') + 1
+                end
               end
-            end
 
-            down do |view, _refs|
-              view['mid_field'] = view.delete('new_field') - 1
+              down do |view, _refs|
+                view['mid_field'] = view.delete('new_field') - 1
+              end
             end
           end
         })
@@ -192,14 +194,18 @@ def child_attributes
         viewmodel: ->(_v) {
           self.schema_version = 3
 
-          # delete: former_field
-          migrates from: 2, to: 3 do
-            up do |view, _refs|
-              view.delete('former_field')
-            end
+          migrations do
+            no_migration_from! 1
+
+            # delete: former_field
+            migrates from: 2, to: 3 do
+              up do |view, _refs|
+                view.delete('former_field')
+              end
 
-            down do |view, _refs|
-              view['former_field'] = 'reconstructed'
+              down do |view, _refs|
+                view['former_field'] = 'reconstructed'
+              end
             end
           end
         })
@@ -222,9 +228,11 @@ def migration_bearing_viewmodel_class
           viewmodel: ->(v) {
             root!
             self.schema_version = 2
-            migrates from: 1, to: 2 do
-              down do |view, _refs|
-                view['inherited_base'] = 'present'
+            migrations do
+              migrates from: 1, to: 2 do
+                down do |view, _refs|
+                  view['inherited_base'] = 'present'
+                end
               end
             end
           }))
@@ -240,15 +248,17 @@ def model_attributes
 
           attribute :new_field
 
-          migrates from: 1, to: 2, inherit: migration_bearing_viewmodel_class, at: 2 do
-            down do |view, refs|
-              super(view, refs)
-              view.delete('new_field')
-            end
+          migrations do
+            migrates from: 1, to: 2, inherit: migration_bearing_viewmodel_class, at: 2 do
+              down do |view, refs|
+                super(view, refs)
+                view.delete('new_field')
+              end
 
-            up do |view, refs|
-              view.delete('inherited_base')
-              view['new_field'] = 100
+              up do |view, _refs|
+                view.delete('inherited_base')
+                view['new_field'] = 100
+              end
             end
           end
         })