Merge pull request #72 from mlibrary/repo-dry

DRY GrantRepo and add missing soft delete support on for_collection_class
mlibrary · Feb 28, 2024 · 90742cb · 90742cb
2 parents 82b83c1 + dfb074c
commit 90742cb
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 52 deletions.
diff --git a/lauth/app/repositories/grant_repo.rb b/lauth/app/repositories/grant_repo.rb
@@ -10,40 +10,9 @@ def find(id)
 
       def for_collection_class(username:, client_ip:, collection_class:)
         smallest_network = smallest_network_for_ip(client_ip)
-
-        ds = grants
-          .dataset
-          .where(grants[:dlpsDeleted].is("f"))
-          .join(collections.name.dataset, uniqueIdentifier: :coll, dlpsDeleted: "f")
-          .left_join(users.name.dataset, userid: grants[:userid], dlpsDeleted: "f")
-          .left_join(institutions.name.dataset, uniqueIdentifier: grants[:inst], dlpsDeleted: "f")
-          .left_join(institution_memberships.name.dataset, inst: grants[:inst], dlpsDeleted: "f")
-          .left_join(groups.name.dataset, uniqueIdentifier: grants[:user_grp], dlpsDeleted: "f")
-          .left_join(group_memberships.name.dataset, user_grp: grants[:user_grp], dlpsDeleted: "f")
-          .left_join(Sequel.as(smallest_network, :smallest), inst: grants[:inst])
+        ds = base_grants_for(username: username, network: smallest_network)
+          .join(collections.name.dataset, uniqueIdentifier: grants[:coll], dlpsDeleted: "f")
           .where(collections[:dlpsClass] => collection_class)
-          .where(
-            Sequel.|(
-              Sequel.&(
-                Sequel.~(users[:userid] => nil),
-                {users[:userid] => username}
-              ),
-              Sequel.&(
-                Sequel.~(institutions[:uniqueIdentifier] => nil),
-                Sequel.~(institution_memberships[:userid] => nil),
-                {institution_memberships[:userid] => username}
-              ),
-              Sequel.&(
-                Sequel.~(groups[:uniqueIdentifier] => nil),
-                Sequel.~(group_memberships[:userid] => nil),
-                {group_memberships[:userid] => username}
-              ),
-              Sequel.&(
-                Sequel.~(Sequel[:smallest][:inst] => nil),
-                {Sequel[:smallest][:dlpsAccessSwitch] => "allow"}
-              )
-            )
-          )
 
         rel = grants.class.new(ds)
         rel.to_a
@@ -53,7 +22,28 @@ def for(username:, collection:, client_ip: nil)
         return [] unless collection&.dlpsDeleted == "f"
 
         smallest_network = smallest_network_for_ip(client_ip)
-        ds = grants
+        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
+      end
+
+      private
+
+      def smallest_network_for_ip(client_ip)
+        ip = client_ip ? IPAddr.new(client_ip).to_i : nil
+        networks
+          .dataset
+          .where(dlpsDeleted: "f")
+          .where { dlpsAddressStart <= ip }
+          .where { dlpsAddressEnd >= ip }
+          .select_append(Sequel.as(Sequel.expr { dlpsAddressEnd - dlpsAddressStart }, :block_size))
+          .order(Sequel.asc(:block_size)).limit(1)
+      end
+
+      def base_grants_for(username:, network:)
+        grants
           .dataset
           .where(grants[:dlpsDeleted].is("f"))
           .left_join(users.name.dataset, userid: grants[:userid], dlpsDeleted: "f")
@@ -63,8 +53,7 @@ def for(username:, collection:, client_ip: nil)
           .left_join(groups.name.dataset, uniqueIdentifier: grants[:user_grp], dlpsDeleted: "f")
           .left_join(group_memberships.name.dataset, user_grp: :uniqueIdentifier, dlpsDeleted: "f")
           .left_join(Sequel.as(users.name.dataset, :group_users), userid: :userid, dlpsDeleted: "f")
-          .left_join(Sequel.as(smallest_network, :smallest), inst: institutions[:uniqueIdentifier])
-          .where(grants[:coll] => collection.uniqueIdentifier)
+          .left_join(Sequel.as(network, :smallest), inst: institutions[:uniqueIdentifier])
           .where(
             Sequel.|(
               Sequel.&(
@@ -87,22 +76,6 @@ def for(username:, collection:, client_ip: nil)
               )
             )
           )
-
-        rel = grants.class.new(ds)
-        rel.combine(:user, institutions: {institution_memberships: :users}).to_a
-      end
-
-      private
-
-      def smallest_network_for_ip(client_ip)
-        ip = client_ip ? IPAddr.new(client_ip).to_i : nil
-        networks
-          .dataset
-          .where(dlpsDeleted: "f")
-          .where { dlpsAddressStart <= ip }
-          .where { dlpsAddressEnd >= ip }
-          .select_append(Sequel.as(Sequel.expr { dlpsAddressEnd - dlpsAddressStart }, :block_size))
-          .order(Sequel.asc(:block_size)).limit(1)
       end
     end
   end

diff --git a/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb b/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb
@@ -35,6 +35,14 @@
     it "finds no grants" do
       expect(grants).to be_empty
     end
+
+    it "#for_collection_class finds no grants" do
+      expect(repo.for_collection_class(
+        username: "lauth-group-member",
+        client_ip: "10.99.3.4",
+        collection_class: "someclass"
+      )).to be_empty
+    end
   end
 
   context "when group is soft deleted" do
@@ -59,6 +67,14 @@
     it "finds no grants" do
       expect(grants).to be_empty
     end
+
+    it "#for_collection_class finds no grants" do
+      expect(repo.for_collection_class(
+        username: "lauth-group-member",
+        client_ip: "10.99.3.4",
+        collection_class: "someclass"
+      )).to be_empty
+    end
   end
 
   context "when membership is soft deleted" do
@@ -83,5 +99,13 @@
     it "finds no grants" do
       expect(grants).to be_empty
     end
+
+    it "#for_collection_class finds no grants" do
+      expect(repo.for_collection_class(
+        username: "lauth-group-member",
+        client_ip: "10.99.3.4",
+        collection_class: "someclass"
+      )).to be_empty
+    end
   end
 end
diff --git a/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb b/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb
@@ -35,6 +35,14 @@
     it "finds no grants" do
       expect(grants).to be_empty
     end
+
+    it "#for_collection_class finds no grants" do
+      expect(repo.for_collection_class(
+        username: "lauth-inst-member",
+        client_ip: "10.99.3.4",
+        collection_class: "someclass"
+      )).to be_empty
+    end
   end
 
   context "when institution is soft deleted" do
@@ -59,6 +67,14 @@
     it "finds no grants" do
       expect(grants).to be_empty
     end
+
+    it "#for_collection_class finds no grants" do
+      expect(repo.for_collection_class(
+        username: "lauth-inst-member",
+        client_ip: "10.2.3.4",
+        collection_class: "someclass"
+      )).to be_empty
+    end
   end
 
   context "when membership is soft deleted" do
@@ -83,5 +99,13 @@
     it "finds no grants" do
       expect(grants).to be_empty
     end
+
+    it "#for_collection_class finds no grants" do
+      expect(repo.for_collection_class(
+        username: "lauth-inst-member",
+        client_ip: "10.99.3.4",
+        collection_class: "someclass"
+      )).to be_empty
+    end
   end
 end