Skip to content

Commit

Permalink
First steps to extracting the search engine into it's own object.
Browse files Browse the repository at this point in the history
  • Loading branch information
epugh committed Aug 17, 2023
1 parent 1255216 commit 7c88bd4
Show file tree
Hide file tree
Showing 19 changed files with 432 additions and 181 deletions.
23 changes: 11 additions & 12 deletions app/controllers/api/v1/clone/tries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@ class TriesController < Api::ApiController
# rubocop:disable Metrics/MethodLength
def create
new_try_params = {
escape_query: @try.escape_query,
api_method: @try.api_method,
custom_headers: @try.custom_headers,
field_spec: @try.field_spec,
number_of_rows: @try.number_of_rows,
query_params: @try.query_params,
search_engine: @try.search_engine,
search_url: @try.search_url,
escape_query: @try.escape_query,
search_endpoint: @try.search_endpoint,
field_spec: @try.field_spec,
number_of_rows: @try.number_of_rows,
query_params: @try.query_params,

}

@new_try = @case.tries.build new_try_params
Expand Down Expand Up @@ -49,15 +47,16 @@ def set_try
end

def try_params
# Eric: I think we don't need to have all of these unless cloning a try chagnes the search end point..'
params.require(:try).permit(
:name,
:search_url,
# :search_url,
:field_spec,
:query_params,
:search_engine,
# :search_engine,
:escape_query,
:api_method,
:custom_headers,
# :api_method,
# :custom_headers,
:number_of_rows
)
end
Expand Down
32 changes: 28 additions & 4 deletions app/controllers/api/v1/tries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,20 @@ def create
if params[:parent_try_number] # We need special translation from try_number to the try.id
parameters_to_use[:parent_id] = @case.tries.where(try_number: params[:parent_try_number]).first.id
end

@try = @case.tries.build parameters_to_use

search_endpoint_params_to_use = search_endpoint_params
puts 'Here are the search_endpoint_params_to_use'
# not quite right because it could be via team, needs to be a scope.
search_endpoint_params_to_use['owner_id'] = @case.owner_id
puts search_endpoint_params_to_use

unless search_endpoint_params_to_use['search_engine'].nil?
search_endpoint = SearchEndpoint.find_or_create_by search_endpoint_params_to_use
@try.search_endpoint = search_endpoint
end

try_number = @case.last_try_number + 1

@try.try_number = try_number
Expand Down Expand Up @@ -76,17 +88,29 @@ def set_try
def try_params
params.require(:try).permit(
:escape_query,
:api_method,
:custom_headers,
# :api_method,
# :custom_headers,
:field_spec,
:name,
:number_of_rows,
:query_params,
:search_engine,
:search_url,
# :search_engine,
# :search_url,
:parent_id
)
end

def search_endpoint_params
params_to_return = params.require(:try).permit(
:api_method,
:custom_headers,
:search_engine,
:search_url
)
# map from the old name to the new name
params_to_return['endpoint_url'] = params_to_return.delete 'search_url'
params_to_return
end
end
end
end
34 changes: 34 additions & 0 deletions app/models/search_endpoint.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

# == Schema Information
#
# Table name: search_endpoints
#
# id :bigint not null, primary key
# api_method :string(255)
# custom_headers :string(1000)
# endpoint_url :string(500)
# name :string(255)
# search_engine :string(50)
# created_at :datetime not null
# updated_at :datetime not null
# owner_id :integer
#

class SearchEndpoint < ApplicationRecord
# Associations
# too late now!
# rubocop:disable Rails/HasAndBelongsToMany
has_and_belongs_to_many :teams,
join_table: 'teams_search_endpoints'
# rubocop:enable Rails/HasAndBelongsToMany

belongs_to :owner,
class_name: 'User', optional: true

has_many :tries, dependent: :nullify, inverse_of: :search_endpoint

# Validations
# validates :case_name, presence: true
# validates_with ScorerExistsValidator
end
39 changes: 20 additions & 19 deletions app/models/try.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,18 @@
#
# Table name: tries
#
# id :integer not null, primary key
# ancestry :string(3072)
# api_method :string(255)
# escape_query :boolean default(TRUE)
# field_spec :string(500)
# name :string(50)
# custom_headers :string(1000)
# number_of_rows :integer default(10)
# query_params :string(20000)
# search_engine :string(50) default("solr")
# search_url :string(500)
# try_number :integer
# created_at :datetime not null
# updated_at :datetime not null
# case_id :integer
# id :integer not null, primary key
# ancestry :string(3072)
# escape_query :boolean default(TRUE)
# field_spec :string(500)
# name :string(50)
# number_of_rows :integer default(10)
# query_params :string(20000)
# try_number :integer
# created_at :datetime not null
# updated_at :datetime not null
# case_id :integer
# search_endpoint_id :bigint
#
# Indexes
#
Expand All @@ -40,7 +37,9 @@ class Try < ApplicationRecord
scope :latest, -> { order(id: :desc).first } # The try created the most recently

# Associations
belongs_to :case, optional: true # shouldn't be optional, but was in rails 4
belongs_to :case, optional: true # shouldn't be optional, but was in rails 4

belongs_to :search_endpoint, optional: true # see above too!#dependent: :nullify

has_many :curator_variables,
dependent: :destroy,
Expand All @@ -53,7 +52,9 @@ class Try < ApplicationRecord
before_create :set_defaults

def args
case search_engine
return unless search_endpoint

case search_endpoint.search_engine
when 'solr'
solr_args
when 'es'
Expand Down Expand Up @@ -124,9 +125,9 @@ def id_from_field_spec

def index_name_from_search_url
# NOTE: currently all supported engines have the index name as second to last element, refactor when this changes
case search_engine
case search_endpoint.search_engine
when 'solr', 'es', 'os'
search_url.split('/')[-2]
search_endpoint.endpoint_url.split('/')[-2]
end
end

Expand Down
8 changes: 4 additions & 4 deletions app/views/api/v1/tries/_try.json.jbuilder
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# frozen_string_literal: true

json.args try.args
json.custom_headers try.custom_headers
json.custom_headers try.search_endpoint&.custom_headers
json.curator_vars try.curator_vars_map
json.escape_query try.escape_query
json.api_method try.api_method
json.api_method try.search_endpoint&.api_method
json.field_spec try.field_spec
json.name try.name
json.number_of_rows try.number_of_rows
json.query_params try.query_params
json.search_engine try.search_engine
json.search_url try.search_url
json.search_engine try.search_endpoint&.search_engine
json.search_url try.search_endpoint&.endpoint_url
json.try_number try.try_number
16 changes: 16 additions & 0 deletions db/migrate/20230816172133_extract_search_endpoint.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
class ExtractSearchEndpoint < ActiveRecord::Migration[7.0]
def change

create_table :search_endpoints do |t|
t.string "name", limit: 255
t.integer "owner_id"
t.string "search_engine", limit: 50
t.string "endpoint_url", limit: 500
t.string "api_method"
t.string "custom_headers", limit: 1000
t.timestamps
end

add_column :tries, :search_endpoint_id, :bigint, foreign_key: true
end
end
7 changes: 7 additions & 0 deletions db/migrate/20230816173857_create_join_to_teams.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class CreateJoinToTeams < ActiveRecord::Migration[7.0]
def change
create_join_table :search_endpoints, :teams, table_name: :teams_search_endpoints do |t|
end

end
end
22 changes: 22 additions & 0 deletions db/migrate/20230816183004_create_search_endpoints_from_tries.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
class CreateSearchEndpointsFromTries < ActiveRecord::Migration[7.0]
def change
Try.all.each do |try|

# Go through and find each unique set of values from all the tries,
# and create a single end point that is used by multiple tries where it
# doesn't change.
search_endpoint = SearchEndpoint.find_or_create_by search_engine: try.search_engine,
endpoint_url: try.search_url,
api_method: try.api_method,
custom_headers: try.custom_headers,
owner_id: try.case.owner_id,
name: "#{try.search_engine} #{try.search_url}"

try.search_endpoint = search_endpoint

search_endpoint.save!
try.save!

end
end
end
10 changes: 10 additions & 0 deletions db/migrate/20230816195233_drop_search_endpoint_fields_from_try.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class DropSearchEndpointFieldsFromTry < ActiveRecord::Migration[7.0]
def change
# Now that we've moved all the fields over to SearchEndpoint, lets
# clean up after ourselves.
remove_column :tries, :search_engine
remove_column :tries, :search_url
remove_column :tries, :api_method
remove_column :tries, :custom_headers
end
end
23 changes: 18 additions & 5 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions test/controllers/api/v1/clone/cases_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ class CasesControllerTest < ActionController::TestCase

assert_equal the_try.query_params, cloned_try.query_params
assert_equal 'id:id title:title', cloned_try.field_spec
assert_equal the_try.search_url, cloned_try.search_url
assert_equal the_try.search_endpoint, cloned_try.search_endpoint
assert_equal 'Try 0', cloned_try.name
assert_equal the_try.search_engine, cloned_try.search_engine
# assert_equal the_try.search_engine, cloned_try.search_engine
assert_equal the_try.escape_query, cloned_try.escape_query
assert_equal the_try.api_method, cloned_try.api_method
# assert_equal the_try.api_method, cloned_try.api_method
assert_equal the_try.curator_variables.size, cloned_try.curator_variables.size
end
end
Expand Down
8 changes: 4 additions & 4 deletions test/controllers/api/v1/clone/tries_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,22 @@ class TriesControllerTest < ActionController::TestCase
def assert_try_matches_response response, try
assert_equal try.query_params, response['query_params']
assert_nil_or_equal try.field_spec, response['field_spec']
assert_equal try.search_url, response['search_url']
assert_equal try.search_endpoint.endpoint_url, response['search_url']
assert_equal try.try_number, response['try_number']
assert_equal try.name, response['name']
assert_equal try.solr_args, response['args']
assert_equal try.escape_query, response['escape_query']
assert_equal try.api_method, response['api_method']
assert_equal try.search_endpoint.api_method, response['api_method']

assert_curator_vars_equal try.curator_vars_map, response['curator_vars']
end

def assert_tries_match a_try, try
assert_equal try.case_id, a_try.case_id
assert_equal try.query_params, a_try.query_params
assert_equal try.search_url, a_try.search_url
assert_equal try.search_endpoint, a_try.search_endpoint
assert_equal try.escape_query, a_try.escape_query
assert_equal try.api_method, a_try.api_method
# assert_equal try.api_method, a_try.api_method
end

def assert_curator_vars_equal vars, response_vars
Expand Down
Loading

0 comments on commit 7c88bd4

Please sign in to comment.