Skip to content

Prevent preloading `last_deployment`/`last_visible_deployment`/`upcoming_deployment` with a legacy approach

Problem

Originally, pointed out by @bala.kumar in !83558 (comment 895662565).

We have Preloaders::Environments::DeploymentPreloader to preload last_deployment/last_visible_deployment correctly and efficiently, however, technically we can still use the legacy preload approach such as resource.preload({last_deployment: deployment_associations}), which could read data wrongly and inefficiently. We should prevent us from using the legacy approach.

Context discussion thread

!83558 (comment 895662565)

Possible solution:

  1. Introduce argument to the lambda scope of association so direct preloading is not possible as mentioned in https://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html

Note: Joining, eager loading and preloading of these associations is not possible. These operations happen before instance creation and the scope will be called with a nil argument.

  # Introducing association argument to prevent direct preloading.
  # Consider using Preloaders::Environments::DeploymentPreloader.new.execute_with_union instead.
  has_one :last_deployment, -> (environments) { success.ordered }, class_name: 'Deployment', inverse_of: :environment
  1. Or even better consider moving last_deployment to a model class method instead of an association.

Proposal

  1. Introduce argument to the lambda scope of association so direct preloading is not possible.
  2. Moving the associations to class method results in ActiveRecord::AssociationNotFoundError: Association named 'last_deployment' was not found on Environment; and to avoid it we will have to do further refactor to existing code areas. If required we can create a low priority issue to capture this work in the future and it can be worked on once the serializer fragility epic is closed as some of these associations may not be required then.

POC

!97963 (diffs)

Edited by Bala Kumar