From 4d3f00d178f102db8d668735430449fa8f9a8494 Mon Sep 17 00:00:00 2001 From: Noah Botimer Date: Fri, 28 Jun 2024 18:10:51 -0400 Subject: [PATCH] Remove pre-fetching for grant associations When finding grants, we were loading associations for some general convenience. However, we do not inspect the grants in detail for our current use cases -- we supply very specific join/where conditions and check for their existence as the implementation of the authorization rules. This preloading would be useful for building an administrative interface, for example, but is unncessary now. The reason to remove it is to avoid complexity and a performance problem introduced by fetching the institution memberships: we were loading all of them for any matched institution and doing an in-memory join on the object graph. In production, that can be a giant list, consuming excessive time and memory. This problem can be solved by "adjusting the combine", applying appropriate pieces of the filter conditions the association nodes, but it is not worth doing now. We would be doing extra work and extra queries to throw the data away. --- lauth/app/repositories/grant_repo.rb | 6 ++---- lauth/spec/repositories/grant_repo_spec.rb | 9 --------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/lauth/app/repositories/grant_repo.rb b/lauth/app/repositories/grant_repo.rb index 782a951..da25931 100644 --- a/lauth/app/repositories/grant_repo.rb +++ b/lauth/app/repositories/grant_repo.rb @@ -14,8 +14,7 @@ def for_collection_class(username:, client_ip:, collection_class:) .join(collections.name.dataset, uniqueIdentifier: grants[:coll], dlpsDeleted: "f") .where(collections[:dlpsClass] => collection_class) - rel = grants.class.new(ds) - rel.to_a + grants.class.new(ds).to_a end def for(username:, collection:, client_ip: nil) @@ -25,8 +24,7 @@ def for(username:, collection:, client_ip: nil) ds = base_grants_for(username: username, network: smallest_network) .where(grants[:coll] => collection.uniqueIdentifier) - rel = grants.class.new(ds) - rel.combine(:user, institutions: {institution_memberships: :users}).to_a + grants.class.new(ds).to_a end private diff --git a/lauth/spec/repositories/grant_repo_spec.rb b/lauth/spec/repositories/grant_repo_spec.rb index 65eabb2..06cd7a1 100644 --- a/lauth/spec/repositories/grant_repo_spec.rb +++ b/lauth/spec/repositories/grant_repo_spec.rb @@ -59,15 +59,6 @@ expect(grants).to eq [] end - - # TODO: extract this - describe "grant association loading" do - subject(:found_grant) { repo.for(username: "lauth-allowed", collection: collection).first } - - it "loads user" do - expect(found_grant.user.userid).to eq grant.user.userid - end - end end context "with a member of an authorized institution" do