Support defining multiple stores with the same store_attribute

[AlphonseSantoro]

We are currently using StoreExt in our projects which is now archived and not maintained anymore. Upon migrating one of our projects I discovered one case that is not supported here. I also came across #36 which also touches the same issue.

In some of our models we have defined a store with fields specific for that model and also included a store that is common among other models. F.ex

module AddressStore
  extend ActiveSupport::Concern

  included do
    typed_store :data do |s|
      s.string :address_city
      s.string :address_zip
    end
  end
end

class User
 include AddressStore

  typed_store :data do |s|
    s.string :phone_number
  end
end

User.store_accessors # => #<Set: {"phone_number"}>

The example above results in a redefinition of the store constant and the class variable store_accessors contain only the last defined store. You can still access the all the fields via the accessors, but you lose the typed store for the first defined store.

This PR solves this by:

• Only defining the constant when necessary. When a parent class has an existing store defined the sub class defines it including the fields from the parent class (Any duplicated fields is overridden by the sub class). When the class itself has a store defined, the stores are combined (Any duplicated fields is overridden by the last defined store).
• Due to CVE-2022-32224 serialization fails due to the types used. I've decided to set this in the tests only and not set this as a default to not surprise anyone using this gem. It's then up to the appliccation to add the permitted classes.

[AlphonseSantoro]

@byroot This is ready for review

[byroot]

I'm in vacation this week. I'll try to have a look this week end.

[casperisfine]

Ok, so I had a quick look at this, and I don't think it's typed-store job to work around such a problem.

I think we should improve this in Active Record, either have a store "inheritance" behavior or either explicitly break when such case happens.

[AlphonseSantoro]

Just to clarify, the rails store works as intended and store attributes are inherited properly there. The issue here is that when you define more than one store the previous one is discarded. Methods defined by the previous store is still accessible, but you then lose all information on the stored type including which attributes are defined.

I would expect the method typed_store to behave the same way as the rails store method:

# From https://api.rubyonrails.org/classes/ActiveRecord/Store.html
class User
  store :settings, accessors: [ :two_factor_auth ]
  store :settings, accessors: [ :login_retry ]
end
User.stored_attributes[:settings] # => [:two_factor_auth, :login_retry]

But if you do the same with typed_store:

class User
  typed_store :settings, accessors: [ :two_factor_auth ]
  typed_store :settings, accessors: [ :login_retry ]
end
# Typed store:
User.store_accessors # => #<Set: {"login_retry"}>
# Rails:
User.stored_attributes[:settings] # => [:two_factor_auth, :login_retry]

[casperisfine]

Ah I see, my bad.

I'm ok with fixing this, but your PR is really big and a bit hard to grasp.

I think first we should fix the test suite that is failing because of the YAML changes in Rails. For a quick fix we should add all the necessary types to the allow list in the test app. It's best done as a standalone PR, once it's done it will be easier to review.

However I super swamped this week (and probably the couple next weeks as well) so I'm not sure I'll have a whole lot of time to look at this.

[AlphonseSantoro]

I think first we should fix the test suite that is failing because of the YAML changes in Rails.

I'll see if I can find time to fix the CI. I briefly checked it out last night and it seems there are more issues than just the YAML changes.

[casperisfine]

I fixed CI this morning.