Allow model associations to define a separate dataset?

[kenaniah]

Hi Jeremy,

I'd like to see if you would consider splitting out model datasets into two variants -- one used by the model itself, and one for when the model is associated against. Rationale as follows:

When dealing with multi-tenant architectures, it is common to have a tenant_id field (or something semantically similar) defined as part of the primary key for each tenant-specific table. To increase tenant isolation safety, one may be tempted to do the following:

module Sequel
  module Plugins
    module TenantSafety
      # Ensures that the model's dataset is scoped to the current tenant
      def dataset
        @dataset.where Sequel[@dataset.first_source_alias][:tenant_id] => Thread.current[:tenant]&.pk
      end
    end
  end
end

While this works well in the general case, it doesn't quite work well for model associations (as of v5.59.0), because the overridden dataset now contains instance-specific state that really isn't applicable when associating. The above example on its own would lead to instance-specific datasets being cached within the association reflections (but marking these as instance-specific would present its own problems, such as disabling eager loading, etc.).

One simple workaround for this is to patch the association plugins to use a separate dataset altogether:

class Sequel::Model
  # This is a new method that will be used specifically within the association plugins
  def self.associating_dataset
    @dataset || raise(Error, "No dataset associated with #{self}")
  end
end

class Sequel::Model::Associations::AssociationReflection
  def _associated_dataset
    associated_class.associating_dataset # patch #dataset -> #associating_dataset
  end
end

class Sequel::Plugins::ManyThroughMany::ManyThroughManyAssociationReflection
  def _associated_dataset
    ds = associated_class.associating_dataset # patch adds #associating_dataset
    if separate_query_per_table?
      ds = ds.dataset
    else
      (reverse_edges + [final_reverse_edge]).each do |t|
        h = {:qualify=>:deep}
        if t[:alias] != t[:table]
          h[:table_alias] = t[:alias]
        end
        ds = ds.join(t[:table], Array(t[:left]).zip(Array(t[:right])), h)
      end
    end
    ds
  end
end

With both in place, ensuring tenant isolation becomes a simple matter of adding the plugin to the relevant models.

Now nobody likes monkey patches, and I think it would be wonderful to see this implemented in Sequel's association plugins directly.

Is this something you would be open to, or is there another approach that you believe would be more suitable?

Thanks!

[jeremyevans]

If you have an existing model instance for a specific tenant, I assume all associated objects are also related to that tenant. The main thing you want is that the initial query to get a specific model instance only looks for instances for the current tenant. That's best done via an approach such asModelClass.for_current_tenant.whatever or TenantModelClass.whatever. I'm guessing you want to do ModelClass.whatever instead, but that approach leads you to the problems you are reporting here.

I don't think it's a good idea to complicate the internals so that associations use different datasets than the model classes. If you don't want to switch to one of the approaches I outlined above, you should probably continue to use the approach you are currently using. Thankfully, it appears to be fairly compact.