Fix memory consuming user membership checks in Spaces model

- instead of loading all users into an array in memory and searching
them one by one, we now use db queries

- improves checks for space managers, developers, auditors, and org
users

[#155275098]

Signed-off-by: Tim Downey <tdowney@pivotal.io>

app/models/runtime/organization.rb

@@ -228,6 +228,10 @@ def isolation_segment_guids
       isolation_segment_models.map(&:guid)
     end
 
+    def has_user?(user)
+      user.present? && users_dataset.where(user_id: user.id).present?
+    end
+
     private
 
     def validate_default_isolation_segment

app/models/runtime/space.rb

@@ -150,16 +150,15 @@ def having_developers(*users)
     end
 
     def has_developer?(user)
-      developers.include?(user)
+      user.present? && developers_dataset.where(user_id: user.id).present?
     end
 
     def has_member?(user)
-      members = developers | managers | auditors
-      members.include?(user)
+      has_developer?(user) || has_manager?(user) || has_auditor?(user)
     end
 
     def in_organization?(user)
-      organization && organization.users.include?(user)
+      organization && organization.has_user?(user)
     end
 
     def validate
@@ -238,6 +237,14 @@ def in_suspended_org?
 
     private
 
+    def has_manager?(user)
+      user.present? && managers_dataset.where(user_id: user.id).present?
+    end
+
+    def has_auditor?(user)
+      user.present? && auditors_dataset.where(user_id: user.id).present?
+    end
+
     def memory_remaining
       memory_used = started_app_memory + running_task_memory
       space_quota_definition.memory_limit - memory_used

spec/unit/models/runtime/organization_spec.rb

@@ -895,5 +895,28 @@ module VCAP::CloudController
         end
       end
     end
+
+    describe '#has_user?' do
+      subject(:org) { Organization.make }
+      let(:user) { User.make }
+      let(:second_user) { User.make }
+
+      before do
+        org.add_user(second_user)
+      end
+
+      it 'returns true if the given user is a user within the org' do
+        org.add_user(user)
+        expect(org.has_user?(user)).to be true
+      end
+
+      it 'returns false if the given user is not a user within the org' do
+        expect(org.has_user?(user)).to be false
+      end
+
+      it 'returns false if the given user is nil' do
+        expect(org.has_user?(nil)).to be false
+      end
+    end
   end
 end

spec/unit/models/runtime/space_spec.rb

@@ -687,42 +687,74 @@ module VCAP::CloudController
     describe '#has_developer?' do
       subject(:space) { Space.make }
       let(:user) { User.make }
+      let(:other_developer) { User.make }
+
+      before do
+        space.organization.add_user(user)
+        space.organization.add_user(other_developer)
+        space.add_developer(other_developer)
+      end
 
       it 'returns true if the given user is a space developer' do
-        space.organization.add_user user
-        space.add_developer user
-        expect(space.has_developer?(user)).to be_truthy
+        space.add_developer(user)
+        expect(space.has_developer?(user)).to be true
       end
 
       it 'returns false if the given user is not a space developer' do
-        expect(space.has_developer?(user)).to be_falsey
+        expect(space.has_developer?(user)).to be false
+      end
+
+      it 'returns false if the given user is nil' do
+        expect(space.has_developer?(nil)).to be false
       end
     end
 
     describe '#has_member?' do
       subject(:space) { Space.make }
       let(:user) { User.make }
+      let(:other_user) { User.make }
+
+      before do
+        space.organization.add_user(user)
+        space.organization.add_user(other_user)
+        space.add_developer(other_user)
+      end
 
       it 'returns true if the given user is a space developer' do
-        space.organization.add_user user
-        space.add_developer user
-        expect(space.has_member?(user)).to be_truthy
+        space.add_developer(user)
+        expect(space.has_member?(user)).to be true
       end
 
       it 'returns true if the given user is a space auditor' do
-        space.organization.add_user user
-        space.add_auditor user
-        expect(space.has_member?(user)).to be_truthy
+        space.add_auditor(user)
+        expect(space.has_member?(user)).to be true
       end
 
       it 'returns true if the given user is a space manager' do
-        space.organization.add_user user
-        space.add_manager user
-        expect(space.has_member?(user)).to be_truthy
+        space.add_manager(user)
+        expect(space.has_member?(user)).to be true
       end
 
       it 'returns false if the given user is not a manager, auditor, or developer' do
-        expect(space.has_member?(user)).to be_falsey
+        expect(space.has_member?(user)).to be false
+      end
+
+      it 'returns false if the given user is nil' do
+        expect(space.has_member?(nil)).to be false
+      end
+    end
+
+    describe '#in_organization?' do
+      subject(:space) { Space.make }
+      let(:user) { User.make }
+
+      it "returns true if the given user is in the space's organization" do
+        space.organization.add_user(user)
+        expect(space.in_organization?(user)).to be true
+      end
+
+      it "returns false if the given user is not in the space's organization" do
+        expect(space.in_organization?(user)).to be false
       end
     end
   end