From ce4c8e957b377878e945e96c211aebac224aaf62 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Tue, 16 Apr 2019 09:31:51 +0800
Subject: [PATCH] PERF: Avoid looking up the same group multiple times during
 bulk invite.

Also cache the guardian check because it does a query to check if the
user is an owner of the group.
---
 app/jobs/regular/bulk_invite.rb   | 34 +++++++++++++++++++++++++++----
 lib/guardian/group_guardian.rb    |  2 +-
 spec/fixtures/csv/bulk_invite.csv |  2 +-
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/app/jobs/regular/bulk_invite.rb b/app/jobs/regular/bulk_invite.rb
index 61d28758a98..cd76fed56f6 100644
--- a/app/jobs/regular/bulk_invite.rb
+++ b/app/jobs/regular/bulk_invite.rb
@@ -11,6 +11,8 @@ module Jobs
       @logs    = []
       @sent    = 0
       @failed  = 0
+      @groups = {}
+      @valid_groups = {}
     end
 
     def execute(args)
@@ -19,6 +21,7 @@ module Jobs
 
       @current_user = User.find_by(id: args[:current_user_id])
       raise Discourse::InvalidParameters.new(:current_user_id) unless @current_user
+      @guardian = Guardian.new(@current_user)
 
       csv_path = "#{Invite.base_directory}/#{filename}"
 
@@ -60,14 +63,13 @@ module Jobs
 
       if group_names
         group_names = group_names.split(';')
-        guardian = Guardian.new(@current_user)
 
         group_names.each { |group_name|
-          group_detail = Group.find_by_name(group_name)
+          group = fetch_group(group_name)
 
-          if group_detail && guardian.can_edit_group?(group_detail)
+          if group && can_edit_group?(group)
             # valid group
-            groups.push(group_detail)
+            groups.push(group)
           else
             # invalid group
             save_log "Invalid Group '#{group_name}' at line number '#{csv_line_number}'"
@@ -145,6 +147,30 @@ module Jobs
       end
     end
 
+    def fetch_group(group_name)
+      group_name = group_name.downcase
+      group = @groups[group_name]
+
+      unless group
+        group = Group.find_by("lower(name) = ?", group_name)
+        @groups[group_name] = group
+      end
+
+      group
+    end
+
+    def can_edit_group?(group)
+      group_name = group.name.downcase
+      result = @valid_groups[group_name]
+
+      unless result
+        result = @guardian.can_edit_group?(group)
+        @valid_groups[group_name] = result
+      end
+
+      result
+    end
+
   end
 
 end
diff --git a/lib/guardian/group_guardian.rb b/lib/guardian/group_guardian.rb
index 492924c92e5..a9ad9741d63 100644
--- a/lib/guardian/group_guardian.rb
+++ b/lib/guardian/group_guardian.rb
@@ -5,7 +5,7 @@ module GroupGuardian
   # Automatic groups are not represented in the GROUP_USERS
   # table and thus do not allow membership changes.
   def can_edit_group?(group)
-    can_log_group_changes?(group) && !group.automatic
+    !group.automatic && can_log_group_changes?(group)
   end
 
   def can_log_group_changes?(group)
diff --git a/spec/fixtures/csv/bulk_invite.csv b/spec/fixtures/csv/bulk_invite.csv
index ea2d897a277..9d0d3b0a61b 100644
--- a/spec/fixtures/csv/bulk_invite.csv
+++ b/spec/fixtures/csv/bulk_invite.csv
@@ -1,2 +1,2 @@
 test2@discourse.org
-test@discourse.org,group1;group2,999
+test@discourse.org,GROUP1;group2,999