Merge pull request #135 from gate-sso/optimize-cache-usage

Optimize cache usage (part 1)

app/models/group.rb

@@ -22,8 +22,6 @@ class Group < ApplicationRecord
 
   GID_CONSTANT = 9000
 
-
-
   def burst_host_cache
     if host_machines.count > 0
       host_machines.each do |host|
@@ -75,12 +73,6 @@ def self.get_all_response
     return response
   end
 
-  def self.response_array group_response
-    response_array = []
-    response_array <<  group_response
-    response_array
-  end
-
   def self.get_gid_response gid
     group = Group.where(gid: gid).first
     return [] if group.blank?
@@ -95,6 +87,15 @@ def member? user
     users.exists? user.id
   end
 
+  def self.generate_group_response(name, gid, members)
+    {
+      gr_name: name,
+      gr_passwd: 'x',
+      gr_gid: 'gid',
+      gr_mem: members,
+    }
+  end
+
   def group_response
     return Group.group_nss_response name
   end
@@ -106,11 +107,8 @@ def self.group_nss_response name
     if group_response.blank?
       group = Group.find_by(name: name)
       if group.present?
-        response_hash = {}
-        response_hash[:gr_name] = group.name
-        response_hash[:gr_passwd] = "x"
-        response_hash[:gr_gid] = group.gid
-        response_hash[:gr_mem] = group.users.collect { |u| u.user_login_id}
+        members = group.users.collect { |u| u.user_login_id }
+        response_hash = Group.generate_group_response(group.name, group.gid, members)
         REDIS_CACHE.set("#{GROUP_NSS_RESPONSE}:#{group.name}", response_hash.to_json)
         REDIS_CACHE.expire("#{GROUP_NSS_RESPONSE}:#{group.name}", REDIS_KEY_EXPIRY)
         group_response = response_hash
@@ -120,22 +118,32 @@ def self.group_nss_response name
   end
 
   def self.get_sysadmins_and_groups sysadmins, default_admins = true
-    groups, sysadmins_login_ids = Group.get_groups_for_host sysadmins
+    sysadmins_login_ids = User.
+      select(:user_login_id).
+      where("id IN (?)", sysadmins).
+      collect(&:user_login_id)
+    groups = Group.
+      select(%Q(
+        id,
+        name,
+        gid,
+        (
+          SELECT user_login_id
+          FROM users
+          INNER JOIN group_associations
+            ON users.id = group_associations.user_id
+          WHERE group_associations.group_id = groups.id
+        ) AS members
+      )).
+      where("name IN (?)", sysadmins_login_ids).
+      map{ |group|
+        members = (group.members.is_a? Array) ? group.members : [group.members]
+        Group.generate_group_response(group.name, group.gid, members)
+      }
     groups << Group.get_default_sysadmin_group_for_host(sysadmins_login_ids, default_admins)
     groups.to_json
   end
 
-  def self.get_groups_for_host sysadmins
-    groups = []
-    sysadmins_login_ids = []
-    sysadmins.each do |sysadmin|
-      user = User.from_cache(sysadmin)
-      sysadmins_login_ids << user["user_login_id"]
-      groups << Group.group_nss_response(user["user_login_id"])
-    end
-    [groups, sysadmins_login_ids]
-  end
-
   def get_user_ids
     user_ids = REDIS_CACHE.get("#{GROUP_UID_RESPONSE}:#{name}")
     user_ids = JSON.parse(user_ids) if user_ids.present?
@@ -149,7 +157,6 @@ def get_user_ids
   end
 
   def self.get_default_sysadmin_group_for_host sysadmins_login_ids, default_admins = true
-    sysadmin_group = {}
     sysadmins = sysadmins_login_ids
 
     if default_admins
@@ -161,10 +168,7 @@ def self.get_default_sysadmin_group_for_host sysadmins_login_ids, default_admins
     end
     group_id = group.blank? ? 8999 : group.id
 
-    sysadmin_group[:gr_gid] = group_id
-    sysadmin_group[:gr_mem] = sysadmins.uniq
-    sysadmin_group[:gr_name] = "sysadmins"
-    sysadmin_group[:gr_passwd] = "x"
+    sysadmin_group = Group.generate_group_response("sysadmins", group_id, sysadmins.uniq)
     return sysadmin_group
   end
 

app/models/host_machine.rb

@@ -8,7 +8,6 @@ class HostMachine < ApplicationRecord
 
   before_create :set_lower_case_name
   before_create :set_host_access_key
-  before_save :remove_host_cache
 
   def set_host_access_key
     self.access_key = ROTP::Base32.random_base32
@@ -27,24 +26,12 @@ def self.get_group_response name
     response
   end
 
-  def remove_host_cache
-    REDIS_CACHE.del("#{HOST_UID_PREFIX}:#{name}")
-  end
-
   def sysadmins
-    host_users = REDIS_CACHE.get("#{HOST_UID_PREFIX}:#{name}")
-    host_users = JSON.parse(host_users) if host_users.present?
-    users = []
-    if host_users.blank?
-      users = GroupAssociation.
-        joins(:user).
-        where("group_id IN (?)", groups.collect(&:id)).
-        collect(&:user_id)
-      host_users = users.uniq
-      REDIS_CACHE.set("#{HOST_UID_PREFIX}:#{name}", host_users.to_json)
-      REDIS_CACHE.expire("#{HOST_UID_PREFIX}:#{name}", REDIS_KEY_EXPIRY)
-    end
-    host_users
+    users = GroupAssociation.
+      joins(:user).
+      where("group_id IN (?)", groups.collect(&:id)).
+      collect(&:user_id)
+    users.uniq
   end
 
   def add_host_group(name)

app/models/user.rb

@@ -21,10 +21,6 @@ class User < ApplicationRecord
   HOME_DIR = '/home'.freeze
   USER_SHELL = '/bin/bash'.freeze
 
-
-
-  before_save :remove_user_cache
-
   def self.add_user(first_name, last_name, user_role, domain)
     user = User.new(first_name: first_name, last_name: last_name, user_role: user_role)
     user.assign_attributes(
@@ -38,22 +34,6 @@ def self.add_user(first_name, last_name, user_role, domain)
     user
   end
 
-  def remove_user_cache
-    REDIS_CACHE.del("#{USER_CACHE_PREFIX}:#{id}") if id.present?
-  end
-
-  def self.from_cache id
-    user = REDIS_CACHE.get("#{USER_CACHE_PREFIX}:#{id}")
-    user = JSON.parse(user) if user.present?
-    if user.blank?
-      user = User.find(id).to_json
-      REDIS_CACHE.set("#{USER_CACHE_PREFIX}:#{id}", user)
-      REDIS_CACHE.expire("#{USER_CACHE_PREFIX}:#{id}", REDIS_KEY_EXPIRY)
-      user = JSON.parse(user)
-    end
-    return user
-  end
-
   def generate_login_id
     email.split('@').first
   end

config/initializers/redis.rb

@@ -7,9 +7,6 @@
               end
 RATE_LIMIT = REDIS_CONFIG['limit']
 
-
-GROUP_NAME_RESPONSE = "GROUP_NAME:"
-GROUP_GID_RESPONSE = "GROUP_GID:"
 GROUP_ALL_RESPONSE = "GROUP_ALL"
 GROUP_RESPONSE = "G"
 GROUP_NSS_RESPONSE = "UG"
@@ -18,14 +15,7 @@
 SHADOW_NAME_RESPONSE = "SHADOW_NAME:"
 SHADOW_ALL_RESPONSE = "SHADOW_ALL"
 
-PASSWD_NAME_RESPONSE = "PASSWD_NAME:"
-PASSWD_UID_RESPONSE = "PASSWD_UID:"
 PASSWD_ALL_RESPONSE = "PASSWD_ALL"
 PASSWD_RESPONSE = "P"
 
-HOST_GROUP_RESPONSE = "HG:"
-HOST_UID_PREFIX = "HOST_UID"
-
-USER_CACHE_PREFIX = "USER_CACHE"
-
 REDIS_KEY_EXPIRY = 7 * 60

lib/tasks/users.rake

@@ -49,9 +49,6 @@ namespace :users do
       user.save!
     end
 
-    REDIS_CACHE.del(GROUP_NAME_RESPONSE + group.name)
-    REDIS_CACHE.del(GROUP_GID_RESPONSE + group.gid.to_s)
-
     @response = Group.get_all_response.to_json
     REDIS_CACHE.set(GROUP_ALL_RESPONSE, @response)
     REDIS_CACHE.expire(GROUP_ALL_RESPONSE, REDIS_KEY_EXPIRY)
@@ -76,10 +73,6 @@ namespace :users do
       user.save!
     end
 
-
-    REDIS_CACHE.del(GROUP_NAME_RESPONSE + group.name)
-    REDIS_CACHE.del(GROUP_GID_RESPONSE + group.gid.to_s)
-
     @response = Group.get_all_response.to_json
     REDIS_CACHE.set(GROUP_ALL_RESPONSE, @response)
     REDIS_CACHE.expire(GROUP_ALL_RESPONSE, REDIS_KEY_EXPIRY)

spec/controllers/nss_controller_spec.rb

@@ -38,7 +38,6 @@
   end
 
   it "should not create groups and admins again for the same host name" do
-
     create(:access_token, token: access_token)
     json = { token: access_token, name: 'random_host', group_name: 'duplicate_group', format: :json }
     post 'add_host', params: json
@@ -64,7 +63,6 @@
     host = HostMachine.find_by(name: "random_host_01")
     expect(access_key).to eq host.access_key
 
-
     group = create(:group)
     user = create(:user)
     group.host_machines << host
@@ -86,7 +84,6 @@
     expect(body.count).to eq 3
     expect(body[2]["gr_mem"].count).to eq 2
 
-
     get "group", params: { token: access_key, format: json }
     body = JSON.parse(response.body)
 
@@ -97,11 +94,9 @@
 
     expect(Group.find_by(name: "random_group_01")).not_to eq nil
     expect(Group.find_by(name: "random_host_01_host_group")).not_to eq nil
-
   end
 
   it "should return members of sysadmins if no other group exists" do
-
     access_token = create(:access_token)
     group = create(:group, name: "sysadmins")
     user = create(:user)
@@ -112,7 +107,6 @@
     expect(host.name).to eq "random_host_01"
     host.groups << group
 
-
     get "group", params: { token: host.access_key, format: :json }
     body = JSON.parse(response.body)
 
@@ -122,12 +116,9 @@
 
     expect(body[0]["gr_mem"][0]).to eq user.user_login_id
     expect(body[1]["gr_mem"][0]).to eq user.user_login_id
-
-
   end
 
   it "should not return sysadmins if inheritance is off" do
-
     access_token = create(:access_token)
     group = create(:group, name: "sysadmins")
     group_2 = create(:group, name: "random")
@@ -141,7 +132,6 @@
     expect(host.name).to eq "random_host_01"
     host.groups << group_2
 
-
     get "group", params: { token: host.access_key, format: :json }
     body = JSON.parse(response.body)
 
@@ -151,12 +141,9 @@
 
     expect(body[0]["gr_mem"][0]).to eq user.user_login_id
     expect(body[1]["gr_mem"][0]).to eq user.user_login_id
-
-
   end
 
   it "should return all the users for the host" do
-
     access_token = create(:access_token)
     group = create(:group, name: "sysadmins")
     user = create(:user)
@@ -171,7 +158,6 @@
     group.reload
     user.reload
 
-
     get "passwd", params: { token: host.access_key, format: :json }
     body = JSON.parse(response.body)
     expect(body.count).to eq 1
@@ -192,18 +178,11 @@
       group.burst_host_cache
     end
 
-    cache_count_bfr = REDIS_CACHE.keys("UG*").count
-
+    cache_count_bfr = REDIS_CACHE.keys("#{GROUP_RESPONSE}*").count
     get "group", params: { token: host_machine.access_key, format: :json }
-    cache_count_aft = REDIS_CACHE.keys("UG*").count
+    expect(REDIS_CACHE.keys("#{GROUP_RESPONSE}*").count).to eq cache_count_bfr + 1
 
-    expect(cache_count_aft).to eq cache_count_bfr + 1
     group.burst_host_cache
-
-    cache_count_aft = REDIS_CACHE.keys("UG*").count
-    expect(cache_count_aft).to eq cache_count_bfr + 1
-
+    expect(REDIS_CACHE.keys("#{GROUP_RESPONSE}*").count).to eq cache_count_bfr
   end
-
-
 end