Merge pull request #128 from gate-sso/optimize-nss-passwd

Optimize nss/passwd API, avoid N+1 query

app/controllers/nss_controller.rb

@@ -39,15 +39,15 @@ def add_host
   end
 
   def group
-    @response = REDIS_CACHE.get( "G:" + params[:token])
+    @response = REDIS_CACHE.get("#{GROUP_RESPONSE}:#{params[:token]}")
     @response = JSON.parse(@response) if @response.present?
     if @response.blank?
       host_machine = HostMachine.find_by(access_key: params[:token])
       sysadmins = host_machine.sysadmins if host_machine.present?
       if sysadmins.present? && sysadmins.count > 0
         @response = Group.get_sysadmins_and_groups sysadmins, host_machine.default_admins
-        REDIS_CACHE.set( "G:" + params[:token], @response.to_json)
-        REDIS_CACHE.expire( "G:" + params[:token], REDIS_KEY_EXPIRY)
+        REDIS_CACHE.set("#{GROUP_RESPONSE}:#{params[:token]}", @response.to_json)
+        REDIS_CACHE.expire("#{GROUP_RESPONSE}:#{params[:token]}", REDIS_KEY_EXPIRY)
       end
     end
 
@@ -56,17 +56,15 @@ def group
 
 
   def passwd
-    @response = REDIS_CACHE.get( "P:" + params[:token])
+    @response = REDIS_CACHE.get("#{PASSWD_RESPONSE}:#{params[:token]}")
     @response = JSON.parse(@response) if @response.present?
     if @response.blank?
-
       host_machine = HostMachine.find_by(access_key: params[:token])
       sysadmins = host_machine.sysadmins if host_machine.present?
-
       if sysadmins.present? && sysadmins.count > 0
         @response = User.get_sysadmins sysadmins
-        REDIS_CACHE.set( "P:" + params[:token], @response.to_json)
-        REDIS_CACHE.expire( "P:" + params[:token], REDIS_KEY_EXPIRY)
+        REDIS_CACHE.set("#{PASSWD_RESPONSE}:#{params[:token]}", @response.to_json)
+        REDIS_CACHE.expire("#{PASSWD_RESPONSE}:#{params[:token]}", REDIS_KEY_EXPIRY)
       end
     end
     render json: @response

app/models/group.rb

@@ -28,8 +28,8 @@ def burst_host_cache
     if host_machines.count > 0
       host_machines.each do |host|
         if host.access_key.present?
-          REDIS_CACHE.del ("G:" + host.access_key)
-          REDIS_CACHE.del ("P:" + host.access_key)
+          REDIS_CACHE.del ("#{GROUP_RESPONSE}:#{host.access_key}")
+          REDIS_CACHE.del ("#{PASSWD_RESPONSE}:#{host.access_key}")
           Rails.logger.info "hello #{host.name} #{host.access_key}"
         end
       end
@@ -100,7 +100,7 @@ def group_response
   end
 
   def self.group_nss_response name
-    group_response = REDIS_CACHE.get( "UG:" + name)
+    group_response = REDIS_CACHE.get("#{GROUP_NAME_RESPONSE}:#{name}")
     group_response = JSON.parse(group_response) if group_response.present?
 
     if group_response.blank?
@@ -111,8 +111,8 @@ def self.group_nss_response name
         response_hash[:gr_passwd] = "x"
         response_hash[:gr_gid] = group.gid
         response_hash[:gr_mem] = group.users.collect { |u| u.user_login_id}
-        REDIS_CACHE.set( "UG:" + group.name, response_hash.to_json)
-        REDIS_CACHE.expire( "UG:" + group.name, REDIS_KEY_EXPIRY)
+        REDIS_CACHE.set("#{GROUP_NAME_RESPONSE}:#{group.name}", response_hash.to_json)
+        REDIS_CACHE.expire("#{GROUP_NAME_RESPONSE}:#{group.name}", REDIS_KEY_EXPIRY)
         group_response = response_hash
       end
     end
@@ -137,13 +137,13 @@ def self.get_groups_for_host sysadmins
   end
 
   def get_user_ids
-    user_ids = REDIS_CACHE.get( "G_UID:" + name)
+    user_ids = REDIS_CACHE.get("#{GROUP_UID_RESPONSE}:#{name}")
     user_ids = JSON.parse(user_ids) if user_ids.present?
 
     if user_ids.blank?
       user_ids = users.collect {|u| u.user_login_id}
-      REDIS_CACHE.set( "G_UID:" + name, user_ids.to_json)
-      REDIS_CACHE.expire( "G_UID:" + name, REDIS_KEY_EXPIRY)
+      REDIS_CACHE.set("#{GROUP_UID_RESPONSE}:#{name}", user_ids.to_json)
+      REDIS_CACHE.expire("#{GROUP_UID_RESPONSE}:#{name}", REDIS_KEY_EXPIRY)
     end
     return user_ids
   end
@@ -154,7 +154,7 @@ def self.get_default_sysadmin_group_for_host sysadmins_login_ids, default_admins
 
     if default_admins
       group = Group.find_by(name: "sysadmins")
-       
+
       if group.present?
         sysadmins = sysadmins + group.get_user_ids
       end

app/models/host_machine.rb

@@ -28,11 +28,11 @@ def self.get_group_response name
   end
 
   def remove_host_cache
-    REDIS_CACHE.del("HOST_UID:" + name)
+    REDIS_CACHE.del("#{HOST_UID_PREFIX}:#{name}")
   end
 
   def sysadmins
-    host_users = REDIS_CACHE.get("HOST_UID:" + name)
+    host_users = REDIS_CACHE.get("#{HOST_UID_PREFIX}:#{name}")
     host_users = JSON.parse(host_users) if host_users.present?
     users = []
     if host_users.blank?
@@ -41,7 +41,7 @@ def sysadmins
         where("group_id IN (?)", groups.collect(&:id)).
         collect(&:user_id)
       host_users = users.uniq
-      REDIS_CACHE.set("HOST_UID:" + name, host_users.to_json)
+      REDIS_CACHE.set("#{HOST_UID_PREFIX}:#{name}", host_users.to_json)
     end
     host_users
   end

app/models/user.rb

@@ -17,16 +17,16 @@ class User < ActiveRecord::Base
   before_save :remove_user_cache
 
   def remove_user_cache
-    REDIS_CACHE.del( "USER_CACHE:" + "#{id}") if id.present?
+    REDIS_CACHE.del("#{USER_CACHE_PREFIX}:#{id}") if id.present?
   end
 
   def self.from_cache id
-    user = REDIS_CACHE.get( "USER_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:" + "#{id}", user)
-      REDIS_CACHE.expire( "USER_CACHE:" + "#{id}", REDIS_KEY_EXPIRY)
+      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
@@ -94,11 +94,25 @@ def name_email
     "#{name} (#{email})"
   end
 
-  def self.get_sysadmins uids
-    user_list = []
-    uids.each do |uid|
-      user_list << get_passwd_uid_response(User.find(uid).uid)
-    end
+  def self.get_sysadmins user_ids
+    users = User.
+      select(%Q(
+        id, 
+        name, 
+        uid, 
+        user_login_id, 
+        (
+          SELECT gid 
+          FROM groups 
+          INNER JOIN group_associations 
+            ON groups.id = group_associations.group_id 
+          WHERE group_associations.user_id = users.id
+          AND groups.name = users.user_login_id
+          LIMIT 1
+        ) AS gid
+      )).
+      where(id: user_ids)
+    user_list = users.map{ |user| get_passwd_uid_response(user) }
     user_list
   end
 
@@ -295,9 +309,7 @@ def self.response_array response
     user_response
   end
 
-
-  def self.get_passwd_uid_response uid
-    user = User.where(uid: uid).first
+  def self.get_passwd_uid_response user
     return [] if user.blank?
     user.user_passwd_response
   end
@@ -362,7 +374,14 @@ def user_passwd_response
     user_hash[:pw_name] = user_login_id
     user_hash[:pw_passwd]  = "x"
     user_hash[:pw_uid] = uid.to_i
-    user_hash[:pw_gid] = groups.where(name: user_login_id).first.gid if groups.where(name: user_login_id).count > 0
+
+    # If gid is supplied (avoid N+1)
+    if self.respond_to?(:gid) && gid
+      user_hash[:pw_gid] = gid.to_i
+    else
+      user_hash[:pw_gid] = groups.where(name: user_login_id).first.gid if groups.where(name: user_login_id).count > 0
+    end
+
     user_hash[:pw_gecos]  = "#{name}"
     user_hash[:pw_dir] = "#{HOME_DIR}/#{user_login_id}"
     user_hash[:pw_shell] = "/bin/bash"

config/initializers/redis.rb

@@ -11,14 +11,21 @@
 GROUP_NAME_RESPONSE = "GROUP_NAME:"
 GROUP_GID_RESPONSE = "GROUP_GID:"
 GROUP_ALL_RESPONSE = "GROUP_ALL"
+GROUP_RESPONSE = "G"
+GROUP_NAME_RESPONSE = "UG"
+GROUP_UID_RESPONSE = "G_UID"
 
 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

spec/models/user_spec.rb

@@ -324,7 +324,7 @@
 
   it "should return false if user is not active" do
     user = create(:user)
-    response = User.get_passwd_uid_response user.uid
+    response = User.get_passwd_uid_response user
     expect(response[:pw_name]).to eq(user.user_login_id)
   end