Skip to content

Commit

Permalink
Merge pull request #11427 from vpereira/search_controller_search_method
Browse files Browse the repository at this point in the history
Refactor SearchController extracting query to query object
  • Loading branch information
hennevogel committed Aug 3, 2021
2 parents e9ce5d8 + 517bf9e commit 8767edc
Show file tree
Hide file tree
Showing 5 changed files with 167 additions and 52 deletions.
31 changes: 16 additions & 15 deletions src/api/.rubocop_todo.yml
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config --auto-gen-only-exclude --exclude-limit 100`
# on 2021-07-06 09:58:38 UTC using RuboCop version 1.18.2.
# on 2021-07-29 08:04:28 UTC using RuboCop version 1.18.4.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand Down Expand Up @@ -121,7 +121,7 @@ Lint/UselessMethodDefinition:
- 'app/services/consistency_check_job_service/project_consistency_checker.rb'
- 'test/test_helper.rb'

# Offense count: 806
# Offense count: 811
# Configuration parameters: IgnoredMethods, CountRepeatedAttributes.
Metrics/AbcSize:
Max: 235
Expand Down Expand Up @@ -224,7 +224,7 @@ Metrics/ClassLength:
- 'test/unit/publish_flag_test.rb'
- 'test/unit/user_test.rb'

# Offense count: 191
# Offense count: 192
# Configuration parameters: IgnoredMethods, Max.
Metrics/CyclomaticComplexity:
Exclude:
Expand Down Expand Up @@ -293,6 +293,7 @@ Metrics/CyclomaticComplexity:
- 'app/models/project_status/calculator.rb'
- 'app/models/relationship.rb'
- 'app/models/repository.rb'
- 'app/models/search_finder.rb'
- 'app/models/service.rb'
- 'app/models/unregistered_user.rb'
- 'app/models/update_notification_events.rb'
Expand All @@ -313,7 +314,7 @@ Metrics/CyclomaticComplexity:
- 'test/functional/zzz_post_consistency_test.rb'
- 'test/node_matcher.rb'

# Offense count: 909
# Offense count: 911
# Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods.
Metrics/MethodLength:
Max: 227
Expand Down Expand Up @@ -532,10 +533,10 @@ Naming/PredicateName:
- 'app/models/bs_request_action_maintenance_release.rb'
- 'app/models/bs_request_action_submit.rb'
- 'app/models/channel.rb'
- 'app/models/concerns/project_distribution.rb'
- 'app/models/package.rb'
- 'app/models/patchinfo.rb'
- 'app/models/project.rb'
- 'app/models/concerns/project_distribution.rb'
- 'app/models/repository.rb'
- 'app/models/user.rb'
- 'app/models/user_basic_strategy.rb'
Expand Down Expand Up @@ -657,7 +658,7 @@ RSpec/AnyInstance:
- 'spec/models/project_spec.rb'
- 'spec/policies/package_policy_spec.rb'

# Offense count: 826
# Offense count: 870
# Configuration parameters: Prefixes.
# Prefixes: when, with, without
RSpec/ContextWording:
Expand All @@ -671,7 +672,7 @@ RSpec/DescribeClass:
- 'spec/routing/webui/projects/public_key_spec.rb'
- 'spec/routing/webui/projects/ssl_certificate_spec.rb'

# Offense count: 432
# Offense count: 434
# Cop supports --auto-correct.
# Configuration parameters: SkipBlocks, EnforcedStyle.
# SupportedStyles: described_class, explicit
Expand Down Expand Up @@ -766,7 +767,7 @@ RSpec/IteratedExpectation:
Exclude:
- 'spec/models/user_spec.rb'

# Offense count: 310
# Offense count: 316
# Cop supports --auto-correct.
RSpec/LeadingSubject:
Enabled: false
Expand All @@ -779,7 +780,7 @@ RSpec/MessageSpies:
- 'spec/controllers/webui/apidocs_controller_spec.rb'
- 'spec/models/kiwi/image_spec.rb'

# Offense count: 1376
# Offense count: 1402
# Configuration parameters: AllowSubject.
RSpec/MultipleMemoizedHelpers:
Max: 27
Expand Down Expand Up @@ -1040,7 +1041,7 @@ Rails/TimeZone:
- 'test/unit/binary_release.rb'
- 'test/unit/package_remove_test.rb'

# Offense count: 118
# Offense count: 122
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: nested, compact
Expand Down Expand Up @@ -1082,7 +1083,7 @@ Style/ConditionalAssignment:
Exclude:
- 'app/models/bs_request_action_submit.rb'

# Offense count: 686
# Offense count: 697
# Configuration parameters: AllowedConstants.
Style/Documentation:
Enabled: false
Expand All @@ -1094,7 +1095,7 @@ Style/ExplicitBlockArgument:
- 'app/lib/backend/connection.rb'
- 'app/models/project.rb'

# Offense count: 1438
# Offense count: 1454
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, always_true, never
Expand Down Expand Up @@ -1177,21 +1178,21 @@ Style/SoleNestedConditional:
Style/StringConcatenation:
Enabled: false

# Offense count: 333
# Offense count: 336
# Cop supports --auto-correct.
# Configuration parameters: MinSize.
# SupportedStyles: percent, brackets
Style/SymbolArray:
EnforcedStyle: brackets

# Offense count: 228
# Offense count: 236
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, MinSize, WordRegex.
# SupportedStyles: percent, brackets
Style/WordArray:
Enabled: false

# Offense count: 1864
# Offense count: 1909
# Cop supports --auto-correct.
# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
# URISchemes: http, https
Expand Down
44 changes: 8 additions & 36 deletions src/api/app/controllers/search_controller.rb
Expand Up @@ -171,6 +171,11 @@ def search(what, render_all)

opts = {}

if what == :request
opts[:withhistory] = 1 if params[:withhistory]
opts[:withfullhistory] = 1 if params[:withfullhistory]
end

output = "<collection matches=\"#{matches}\">\n"

xml = {} # filled by filter
Expand All @@ -181,43 +186,10 @@ def search(what, render_all)
end
search_items = filter_items_from_cache(items, xml, key_template)

includes = []
preloads = []
case what
when :package
relation = Package.where(id: search_items).order('projects.name', :name)
includes = [:project]
when :project
relation = Project.where(id: search_items).order(:name)
if render_all
includes = [:repositories]
else
relation = relation.select('projects.id,projects.name')
end
when :repository
relation = Repository.where(id: search_items)
includes = [:project]
when :request
relation = BsRequest.where(id: search_items).order(:id)
preloads = [:reviews, { review_history_elements: :user },
{ bs_request_actions: :bs_request_action_accept_info }]
opts[:withhistory] = 1 if params[:withhistory]
opts[:withfullhistory] = 1 if params[:withfullhistory]
when :person
relation = User.where(id: search_items).order(:login)
when :channel, :channel_binary
relation = ChannelBinary.where(id: search_items)
when :released_binary
relation = BinaryRelease.where(id: search_items)
when :issue
relation = Issue.where(id: search_items)
includes = [:issue_tracker]
else
logger.fatal "strange model: #{what}"
end
relation = relation.includes(includes).references(includes).preload(preloads)
search_finder = SearchFinder.new(what: what, search_items: search_items,
render_all: render_all, params: params)

# TODO: support sort_by and order parameters?
relation = search_finder.call

unless items.empty?
relation.each do |item|
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/models/channel_binary.rb
Expand Up @@ -6,7 +6,7 @@ class ChannelBinary < ApplicationRecord

validate do |channel_binary|
if channel_binary.project && channel_binary.repository
errors.add_to_base('Associated project has to match with repository.project') unless channel_binary.repository.project == channel_binary.project
errors.add(:base, :invalid, message: 'Associated project has to match with repository.project') unless channel_binary.repository.project == channel_binary.project
end
end

Expand Down
88 changes: 88 additions & 0 deletions src/api/app/models/search_finder.rb
@@ -0,0 +1,88 @@
class SearchFinder
include ActiveModel::Validations

attr_reader :included_classes, :relation, :what, :render_all, :params, :search_items, :preloaded_classes

validates :what, inclusion: [:package, :project, :repository, :request, :person, :channel, :channel_binary, :released_binary, :issue]

def initialize(what:, search_items: [], render_all: false, params: {})
@what = what
@render_all = render_all
@search_items = search_items
@params = params
@included_classes = []
@preloaded_classes = []
end

def call
return [] unless valid?

case what
when :package
@relation = packages
when :project
@relation = projects
when :repository
@relation = repositories
when :request
@relation = bs_requests
@preloads = bs_request_preloads
when :person
@relation = users
when :channel, :channel_binary
@relation = channel_binaries
when :released_binary
@relation = binary_releases
when :issue
@relation = issues
end
@relation.includes(@included_classes).references(@included_classes).preload(@preloaded_classes)
end

private

def bs_request_preloads
[:reviews, { review_history_elements: :user },
{ bs_request_actions: :bs_request_action_accept_info }]
end

def packages
@included_classes = [:project]
Package.where(id: search_items).order('projects.name', :name)
end

def projects
if render_all
@included_classes = [:repositories]
Project.where(id: search_items).order(:name)
else
Project.where(id: search_items).order(:name).select('projects.id,projects.name')
end
end

def repositories
@included_classes = [:project]
Repository.where(id: search_items)
end

def bs_requests
BsRequest.where(id: search_items).order(:id)
end

def users
User.where(id: search_items).order(:login)
end

def channel_binaries
ChannelBinary.where(id: search_items)
end

def binary_releases
BinaryRelease.where(id: search_items)
end

def issues
@included_classes = [:issue_tracker]
Issue.where(id: search_items)
end
end
54 changes: 54 additions & 0 deletions src/api/spec/models/search_finder_spec.rb
@@ -0,0 +1,54 @@
require 'rails_helper'

RSpec.describe SearchFinder do
describe 'validation' do
context 'valid' do
let(:finder) { described_class.new(what: :package) }

it { expect(finder).to be_valid }
end

context 'invalid' do
let(:finder) { described_class.new(what: :packaage) }

it { expect(finder).not_to be_valid }
end
end

describe 'search for package' do
let!(:package) { create(:package, name: 'bar') }
let(:finder) { described_class.new(what: :package, search_items: [package.id]) }

it { expect(finder.call).to include(package) }

describe 'instance variables' do
before do
finder.call
end

it { expect(finder.included_classes).to include(:project) }
end
end

describe 'search for project' do
let!(:project) { create(:project, name: 'foo') }

context 'render all = false' do
let(:finder) { described_class.new(what: :project, search_items: [project.id]) }

it { expect(finder.call).to include(project) }
end

context 'render all = true' do
let(:finder) { described_class.new(what: :project, render_all: true, search_items: [project.id]) }

before do
finder.call
end

it { expect(finder.call).to include(project) }
it { expect(finder.included_classes).to include(:repositories) }
it { expect(finder.relation.first.title).not_to be_nil }
end
end
end

0 comments on commit 8767edc

Please sign in to comment.