Improve Indexed behaviour with model subclasses (#580)

Improved the `Indexed` mixin with a new `create_validate_and_add!` convenience method and improved behaviour for subclassed models.

Author: danielpgross

dev/lib/product_taxonomy/models/mixins/indexed.rb

@@ -3,6 +3,14 @@
 module ProductTaxonomy
   # Mixin providing indexing for a model, using hashes to support fast uniqueness checks and lookups by field value.
   module Indexed
+    class << self
+      # Keep track of which model class used "extend Indexed" so that the model can be subclassed while still storing
+      # model objects in a shared index on the superclass.
+      def extended(indexed_model_class)
+        indexed_model_class.instance_variable_set(:@is_indexed, true)
+      end
+    end
+
     # Validator that checks for uniqueness of a field value across all models in a given index.
     class UniquenessValidator < ActiveModel::EachValidator
       def validate_each(record, attribute, value)
@@ -21,6 +29,18 @@ def add(model)
       end
     end
 
+    # Convenience method to create a model, validate it, and add it to the index. If the model is not valid, raise an
+    # error.
+    #
+    # @param args [Array] The arguments to pass to the model's constructor.
+    # @return [Object] The created model.
+    def create_validate_and_add!(...)
+      model = new(...)
+      model.validate!
+      add(model)
+      model
+    end
+
     # Check if a field value is a duplicate of an existing model. A model is considered to be a duplicate if it was
     # added after the first model with that value.
     #
@@ -71,6 +91,18 @@ def size
       all.size
     end
 
+    # Get the hash of hash of models, indexed by fields marked unique.
+    #
+    # @return [Hash] The hash of hash of models indexed by the fields marked unique.
+    def hashed_models
+      # If we're a subclass of the model class that originally extended Indexed, use the parent class' hashed_models.
+      return superclass.hashed_models unless @is_indexed
+
+      @hashed_models ||= unique_fields.each_with_object({}) do |field, hash|
+        hash[field] = {}
+      end
+    end
+
     private
 
     def unique_fields
@@ -80,11 +112,5 @@ def unique_fields
         end
       end.keys
     end
-
-    def hashed_models
-      @hashed_models ||= unique_fields.each_with_object({}) do |field, hash|
-        hash[field] = {}
-      end
-    end
   end
 end

dev/test/models/mixins/indexed_test.rb

@@ -23,6 +23,10 @@ def initialize(id:)
       end
     end
 
+    # Subclass of Model to test inheritance behavior with hashed_models
+    class SubModel < Model
+    end
+
     setup do
       @model = Model.new(id: 1)
       Model.add(@model)
@@ -93,5 +97,42 @@ def initialize(id:)
     test "all returns all models in the index" do
       assert_equal [@model], Model.all
     end
+
+    test "self.extended sets @is_indexed to true on the extended class" do
+      assert_equal true, Model.instance_variable_get(:@is_indexed)
+    end
+
+    test "create_validate_and_add! creates, validates, and adds a model to the index" do
+      Model.reset
+      model = Model.create_validate_and_add!(id: 2)
+
+      assert_equal 2, model.id
+      assert_equal 1, Model.size
+      assert_equal model, Model.find_by(id: 2)
+    end
+
+    test "create_validate_and_add! raises an error if the model is not valid" do
+      assert_raises(ActiveModel::ValidationError) do
+        Model.create_validate_and_add!(id: 1)
+      end
+    end
+
+    test "subclass shares hashed_models with parent class" do
+      parent_model = Model.new(id: 2)
+      Model.add(parent_model)
+
+      sub_model = SubModel.new(id: 3)
+      SubModel.add(sub_model)
+
+      assert_equal 3, Model.size
+      assert_equal 3, SubModel.size
+
+      assert_equal parent_model, Model.find_by(id: 2)
+      assert_equal sub_model, Model.find_by(id: 3)
+      assert_equal parent_model, SubModel.find_by(id: 2)
+      assert_equal sub_model, SubModel.find_by(id: 3)
+
+      assert_same Model.hashed_models, SubModel.hashed_models
+    end
   end
 end