Preserve current tenant through active job

[tvongaza]

Add a new ActiveJobExtensions which hooks into the serialization and deserialization of an ActiveJob to preserve the current tenant through ActiveJob.

We are using this with GoodJob, though as it is hooking into ActiveJob I'd imagine it should work with any job processor which is ActiveJob compatible.

[drale2k]

@excid3 any opinion on this? Looking to switch to GoodJob, would be cool to have this merged

[tvongaza]

@excid3 any opinion on this? Looking to switch to GoodJob, would be cool to have this merged

@drale2k I outlined how to patch your rails install to make use of this here if you want to use it in the meantime:

#321 (comment)

[excid3]

I like this. I think we'll want to undo the PR that automatically integrates with Sidekiq since this would double include the tenant in the params which doesn't need to happen.

It'll still be useful for any jobs directly integrated with Sidekiq, so we will keep it around but this will be a more seamless experience for Rails.

[mintuhouse]

What is the recommended way to disable this?
(We have custom active job extension which does this and few other things)

[excid3]

Not at the moment. Just remove that portion from your extension?

[joshforbes]

This seems to be causing an interesting issue for me. I'm using SolidQueue and have an ActiveJob that has a retry configured:

retry_on ActiveRecord::RecordNotFound

This particular job is triggered at the start of a new trial which means that, rarely, the job runs before the "Organization" has been committed to the database. It just so happens that the Organization is also the tenant.

The first thing I'm doing in my job is to query for the Organization:

organization = Organization.find(organization_id)

So I would expect those rare cases to raise a RecordNotFound and the job to retry a few seconds later. However, it seems that it's raising prior to the job being performed when the GlobalID locater is called. Because of this order of events, the ActiveJob retry is not being engaged.

I recognize that this exact situation is uncommon. Retrying on a RecordNotFound when the record that isn't found is the same as the tenant. I can find a workaround, possible enqueuing this one specific job inside of a without_tenant or taking steps to ensure that the job is never performed before the record is committed.

I moved over to ActsAsTenant later in the life of this application so none of my jobs are making use of the current_tenant serialization. It would be convenient to be able to turn it off.

Thanks for the gem. 🙏

[tvongaza]

@joshforbes Interesting race condition, and as you've mentioned you have work arounds.

The problem seems to stem from GlobalID::Locator.locate returning nil if it fails to find the tenant. Maybe we should raise an exception here if the job's current_tenant value is set, but the tenant can't be found. We are expecting a tenant to be set in this case, so throwing an exception seems reasonable.

GlobalId's docs for reference: https://github.com/rails/globalid

Should be a reasonably easy change to the following if you want to create a PR:
https://github.com/ErwinM/acts_as_tenant/blob/master/lib/acts_as_tenant/active_job_extensions.rb#L9

Be sure to add a test case:
https://github.com/ErwinM/acts_as_tenant/blob/master/spec/jobs/active_job_extensions_spec.rb