Revert Project.public_or_visible_to_user changes

These changes were introduced in MR
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17088. In
https://gitlab.com/gitlab-com/infrastructure/issues/3772 we discovered
these changes lead to a pretty drastic increase in SQL response timings.

We'll revert these changes so we can work on a better solution in the
mean time without GitLab.com (or other installations) experiecing
reduced performance in the mean time.

app/finders/snippets_finder.rb

@@ -62,9 +62,8 @@ class SnippetsFinder < UnionFinder
     # Don't return any project related snippets if the user cannot read cross project
     return table[:id].eq(nil) unless Ability.allowed?(current_user, :read_cross_project)
 
-    projects = Project.public_or_visible_to_user(current_user, use_where_in: false) do |part|
-      part.with_feature_available_for_user(:snippets, current_user)
-    end.select(:id)
+    projects = Project.public_or_visible_to_user(current_user)
+      .with_feature_available_for_user(:snippets, current_user).select(:id)
 
     arel_query = Arel::Nodes::SqlLiteral.new(projects.to_sql)
     table[:project_id].in(arel_query)

app/models/project.rb

@@ -317,42 +317,18 @@ class Project < ActiveRecord::Base
 
   # Returns a collection of projects that is either public or visible to the
   # logged in user.
-  #
-  # A caller may pass in a block to modify individual parts of
-  # the query, e.g. to apply .with_feature_available_for_user on top of it.
-  # This is useful for performance as we can stick those additional filters
-  # at the bottom of e.g. the UNION.
-  #
-  # Optionally, turning `use_where_in` off leads to returning a
-  # relation using #from instead of #where. This can perform much better
-  # but leads to trouble when used in conjunction with AR's #merge method.
-  def self.public_or_visible_to_user(user = nil, use_where_in: true, &block)
-    # If we don't get a block passed, use identity to avoid if/else repetitions
-    block = ->(part) { part } unless block_given?
+  def self.public_or_visible_to_user(user = nil)
+    if user
+      authorized = user
+        .project_authorizations
+        .select(1)
+        .where('project_authorizations.project_id = projects.id')
 
-    return block.call(public_to_user) unless user
+      levels = Gitlab::VisibilityLevel.levels_for_user(user)
 
-    # If the user is allowed to see all projects,
-    # we can shortcut and just return.
-    return block.call(all) if user.full_private_access?
-
-    authorized = user
-      .project_authorizations
-      .select(1)
-      .where('project_authorizations.project_id = projects.id')
-    authorized_projects = block.call(where('EXISTS (?)', authorized))
-
-    levels = Gitlab::VisibilityLevel.levels_for_user(user)
-    visible_projects = block.call(where(visibility_level: levels))
-
-    # We use a UNION here instead of OR clauses since this results in better
-    # performance.
-    union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')])
-
-    if use_where_in
-      where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
+      where('EXISTS (?) OR projects.visibility_level IN (?)', authorized, levels)
     else
-      from("(#{union.to_sql}) AS #{table_name}")
+      public_to_user
     end
   end
 

changelogs/unreleased/revert-project-visibility-changes.yml

@@ -0,0 +1,5 @@
+---
+title: Revert Project.public_or_visible_to_user changes
+merge_request:
+author:
+type: fixed