diff --git a/lib/elasticsearch_client.rb b/lib/elasticsearch_client.rb deleted file mode 100644 index 04cfe623c..000000000 --- a/lib/elasticsearch_client.rb +++ /dev/null @@ -1,70 +0,0 @@ -module ElasticsearchClient - class << self - def mappings_properties(mappings) - return mappings if es7? - - mappings["generic-document"] - end - - def compatible_mappings(properties) - return { "properties" => properties } if es7? - - { "generic-document" => { "properties" => properties } } - end - - def compatible_identifier(params) - return params if es7? - - params.merge("_type" => "generic-document") - end - - def put_mapping(index_name:, mapping:, client: Services.elasticsearch) - return client.indices.put_mapping(index: index_name, body: mapping) if es7? - - client.indices.put_mapping(index: index_name, type: "generic-document", body: mapping) - end - - def delete(id:, index_name:, client: Services.elasticsearch) - client.delete(compatible_params(index: index_name, id: id)) - end - - def search(index_name:, body:, client: Services.elasticsearch) - return client.search(index: index_name, track_total_hits: true, body: body) if es7? - - client.search(index: index_name, type: "generic-document", body:) - end - - def index(id:, index_name:, atts:, params: {}, client: Services.elasticsearch) - client.index(compatible_params(index: index_name, id:, body: atts).merge(params)) - end - - def es7? - return true if ENV["USE_ELASTICSEARCH_7"] - return false if ENV["USE_ELASTICSEARCH_6"] - - return true if opensearch? - - es_version >= Gem::Version.new("7.0.0") - end - - def reload_version - @es_version = nil - end - - private - - def compatible_params(params) - return params if es7? - - params.merge(type: "generic-document") - end - - def opensearch? - Services.elasticsearch.info.dig("version", "distribution") == "opensearch" - end - - def es_version - @es_version ||= Gem::Version.new(Services.elasticsearch.info.dig("version", "number")) - end - end -end diff --git a/lib/elasticsearch_response.rb b/lib/elasticsearch_response.rb deleted file mode 100644 index 7c1863d0f..000000000 --- a/lib/elasticsearch_response.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -class ElasticsearchResponse - def initialize(response) - @response = response - end - - # Returns the total hits count as Integer. Compatible with Elasticsearch 6.x and 7.x. - def total_hits - total = @response.dig("hits", "total") - total = total["value"] if total.is_a?(Hash) - total || 0 - end -end diff --git a/lib/index.rb b/lib/index.rb index befb0a28a..4e7ab9028 100644 --- a/lib/index.rb +++ b/lib/index.rb @@ -118,7 +118,7 @@ def documents_by_format(format, field_definitions) def raw_search(payload) logger.debug "Request payload: #{payload.to_json}" - ElasticsearchClient.search(index_name: @index_name, body: payload, client: @client) + @client.search(index: @index_name, track_total_hits: true, body: payload) end # Convert a best bet query to a string formed by joining the normalised diff --git a/lib/index/elasticsearch_processor.rb b/lib/index/elasticsearch_processor.rb index 9f052c2b4..f200c7f15 100644 --- a/lib/index/elasticsearch_processor.rb +++ b/lib/index/elasticsearch_processor.rb @@ -14,12 +14,12 @@ def initialize(client:) end def save(presenter) - @actions << { index: ElasticsearchClient.compatible_identifier(presenter.identifier) } + @actions << { index: presenter.identifier } @actions << presenter.document end def delete(presenter) - @actions << { delete: ElasticsearchClient.compatible_identifier(presenter.identifier) } + @actions << { delete: presenter.identifier } end def commit diff --git a/lib/indexer/popularity_lookup.rb b/lib/indexer/popularity_lookup.rb index e940b0f1b..5cc8dd75d 100644 --- a/lib/indexer/popularity_lookup.rb +++ b/lib/indexer/popularity_lookup.rb @@ -77,7 +77,7 @@ def traffic_index_size query: { match_all: {} }, size: 0, }) - ElasticsearchResponse.new(response).total_hits + response.dig("hits", "total", "value") end end diff --git a/lib/rummager.rb b/lib/rummager.rb index f29aff91a..65bef8361 100644 --- a/lib/rummager.rb +++ b/lib/rummager.rb @@ -142,7 +142,6 @@ require "search/suggestion_blocklist" require "search/timed_cache" -require "elasticsearch_response" require "index" require "index_finder" require "index_group" @@ -164,7 +163,6 @@ require "scroll_enumerator" require "elasticsearch_config" -require "elasticsearch_client" require "clusters/clusters" require "clusters/cluster" require "search_config" diff --git a/lib/schema/index_schema.rb b/lib/schema/index_schema.rb index 0592f41b1..ce4eccfd8 100644 --- a/lib/schema/index_schema.rb +++ b/lib/schema/index_schema.rb @@ -14,9 +14,9 @@ def es_mappings "document_type" => @document_type_field, } @elasticsearch_types.each_value do |value| - properties = properties.merge(value.es_config) + properties.merge!(value.es_config) end - ElasticsearchClient.compatible_mappings(properties) + { "properties" => properties } end def elasticsearch_type(elasticsearch_type_name) diff --git a/lib/schema_synchroniser.rb b/lib/schema_synchroniser.rb index a5b0333c5..c8f67e4ee 100644 --- a/lib/schema_synchroniser.rb +++ b/lib/schema_synchroniser.rb @@ -5,7 +5,7 @@ def initialize(index_name, client = Services.elasticsearch) end def sync_mappings(mapping, logger = Logger.new($stdout)) - ElasticsearchClient.put_mapping(index_name: @index_name, mapping:, client: @client) + @client.indices.put_mapping(index: @index_name, body: mapping) logger.info "Updated mappings for index: #{@index_name}" rescue OpenSearch::Transport::Transport::Errors::BadRequest => e logger.warn "Unable to update mappings for index: #{@index_name}; #{e.message}" diff --git a/lib/scroll_enumerator.rb b/lib/scroll_enumerator.rb index b434f734f..ede1b23b8 100644 --- a/lib/scroll_enumerator.rb +++ b/lib/scroll_enumerator.rb @@ -17,7 +17,7 @@ def initialize(client:, index_names:, search_body:, batch_size: DEFAULT_BATCH_SI @client = client @index_names = index_names page = initial_scroll_result(batch_size, search_body) - @size = ElasticsearchResponse.new(page).total_hits + @size = page.dig("hits", "total", "value") # Pull out the results as they are needed super() do |yielder| diff --git a/lib/search/aggregate_example_fetcher.rb b/lib/search/aggregate_example_fetcher.rb index 0637d4196..b9f03d6c2 100644 --- a/lib/search/aggregate_example_fetcher.rb +++ b/lib/search/aggregate_example_fetcher.rb @@ -104,7 +104,7 @@ def prepare_response(slugs, response_list) result = {} slugs.zip(response_list) do |slug, response| result[slug] = { - total: ElasticsearchResponse.new(response).total_hits, + total: response.dig("hits", "total", "value"), examples: response["hits"]["hits"].map { |hit| apply_multivalued(hit["_source"] || {}) }, } end diff --git a/lib/search/presenters/result_set_presenter.rb b/lib/search/presenters/result_set_presenter.rb index 8e13e3c25..94e281fe1 100644 --- a/lib/search/presenters/result_set_presenter.rb +++ b/lib/search/presenters/result_set_presenter.rb @@ -25,7 +25,7 @@ def initialize(search_params:, def present response = { results: presented_results, - total: ElasticsearchResponse.new(es_response).total_hits || 0, + total: es_response.dig("hits", "total", "value") || 0, start: search_params.start, search_params.aggregate_name => presented_aggregates, suggested_queries:, diff --git a/lib/sitemap/generator.rb b/lib/sitemap/generator.rb index 887486a3b..c2bf28a22 100644 --- a/lib/sitemap/generator.rb +++ b/lib/sitemap/generator.rb @@ -73,7 +73,7 @@ def get_all_documents page = initial_scroll scroll_id = page["_scroll_id"] documents = page["hits"]["hits"] - total_documents_count = ElasticsearchResponse.new(page).total_hits + total_documents_count = page.dig("hits", "total", "value") total_page_count = total_documents_count.fdiv(SCROLL_BATCH_SIZE).ceil chunk = [homepage] diff --git a/lib/tasks/delete.rake b/lib/tasks/delete.rake index 11601a720..2a51190f5 100644 --- a/lib/tasks/delete.rake +++ b/lib/tasks/delete.rake @@ -10,7 +10,7 @@ namespace :delete do id = args[:link] abort "Missing argument. Usage: rake 'delete:by_link[link]'" if args[:link].nil? - ElasticsearchClient.delete(id:, index_name: SearchConfig.govuk_index_name) + Services.elasticsearch.delete(index: SearchConfig.govuk_index_name, id:) end desc " diff --git a/lib/tasks/indices.rake b/lib/tasks/indices.rake index 74a9d0a30..e01ea7253 100644 --- a/lib/tasks/indices.rake +++ b/lib/tasks/indices.rake @@ -139,7 +139,7 @@ the existing data, you will need to run the \"migrate_schema\" task instead, whi mappings = search_config.schema_config.elasticsearch_mappings(index_name) synchroniser = SchemaSynchroniser.new(index_name, Services.elasticsearch(cluster:)) - synchroniser.sync_mappings(ElasticsearchClient.mappings_properties(mappings), logger) + synchroniser.sync_mappings(mappings, logger) puts "Successfully synchronised #{index_name} index" rescue OpenSearch::Transport::Transport::Errors::BadRequest => e puts "Unable to synchronise index #{index_name} due to #{e.message}" diff --git a/spec/integration/schema_synchroniser_spec.rb b/spec/integration/schema_synchroniser_spec.rb index c24745449..7115ef9f8 100644 --- a/spec/integration/schema_synchroniser_spec.rb +++ b/spec/integration/schema_synchroniser_spec.rb @@ -36,10 +36,6 @@ synchroniser.sync_mappings(mapping, logger) response = Services.elasticsearch.indices.get_mapping(index: SearchConfig.govuk_index_name) - if ElasticsearchClient.es7? - expect(response.values.dig(0, "mappings", "properties", "test")).to eq({ "type" => "keyword" }) - else - expect(response.values.dig(0, "mappings", "generic-document", "properties", "test")).to eq({ "type" => "keyword" }) - end + expect(response.values.dig(0, "mappings", "properties", "test")).to eq({ "type" => "keyword" }) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index eca591e07..cb621fa49 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -117,12 +117,6 @@ ClimateControl.modify(TZ: "UTC") { example.run } end - config.around(:each, :unit) do |example| - ClimateControl.modify(USE_ELASTICSEARCH_7: "true") do - example.run - end - end - if config.files_to_run.one? config.default_formatter = "doc" end diff --git a/spec/support/integration_test_helper.rb b/spec/support/integration_test_helper.rb index b930e5211..3dd08f2b8 100644 --- a/spec/support/integration_test_helper.rb +++ b/spec/support/integration_test_helper.rb @@ -70,7 +70,7 @@ def insert_document(index_name, attributes, id: nil, type: "edition", version: n atts[:link] ||= id clusters.each do |cluster| - ElasticsearchClient.index(id:, index_name:, atts:, params: version_details, client: client(cluster:)) + client(cluster:).index({ index: index_name, id:, body: atts }.merge(version_details)) end id diff --git a/spec/unit/elasticsearch_client_spec.rb b/spec/unit/elasticsearch_client_spec.rb deleted file mode 100644 index 0eafb85eb..000000000 --- a/spec/unit/elasticsearch_client_spec.rb +++ /dev/null @@ -1,177 +0,0 @@ -RSpec.describe ElasticsearchClient do - describe ".es7?" do - let(:es_client) { double("Elasticsearch") } - let(:indices_client) { double("IndicesClient") } - let(:version_info) { nil } - - after do - ElasticsearchClient.reload_version - end - - before do - allow(Services).to receive(:elasticsearch).and_return(es_client) - allow(es_client).to receive_messages(index: {}, - search: {}, - indices: indices_client, - delete: {}, - info: version_info, - track_total_hits: false) - allow(indices_client).to receive_messages(put_mapping: {}) - end - - context "when USE_ELASTICSEARCH_7 is set" do - it "returns true and does not query Elasticsearch" do - ClimateControl.modify USE_ELASTICSEARCH_6: nil, USE_ELASTICSEARCH_7: "true" do - expect(described_class.es7?).to eq(true) - - expect(es_client).not_to have_received(:info) - end - end - end - - context "when USE_ELASTICSEARCH_6 is set" do - it "returns false and does not query Elasticsearch" do - ClimateControl.modify USE_ELASTICSEARCH_6: "true", USE_ELASTICSEARCH_7: nil do - expect(described_class.es7?).to eq(false) - - expect(es_client).not_to have_received(:info) - end - end - end - - context "when no override is set" do - around do |example| - ClimateControl.modify( - USE_ELASTICSEARCH_7: nil, - USE_ELASTICSEARCH_6: nil, - ) do - example.run - end - end - context "when Elasticsearch version is 7.x" do - let(:version_info) do - { "version" => { "number" => "7.10.2" } } - end - it "returns true" do - expect(described_class.es7?).to eq(true) - end - it "calls 'search' with the right parameters, without including type" do - described_class.search(index_name: "index", body: { a: :b }, client: es_client) - expect(es_client).to have_received(:search).with(index: "index", - track_total_hits: true, - body: { a: :b }) - end - it "calls 'index' with the right parameters, without including type" do - described_class.index(id: 123, index_name: "index", atts: { a: :b }, params: { c: :d }, client: es_client) - expect(es_client).to have_received(:index).with(id: 123, - index: "index", - body: { a: :b }, - c: :d) - end - it "returns mappings without type" do - expect(described_class.compatible_mappings({ a: :b })) - .to eq({ "properties" => { a: :b } }) - end - it "calls 'put_mapping' with the right parameters, without including type" do - described_class.put_mapping(index_name: "index", mapping: { a: :b }, client: es_client) - expect(indices_client).to have_received(:put_mapping).with(index: "index", - body: { a: :b }) - end - it "calls 'delete' with the right parameters, without including type" do - described_class.delete(id: 123, index_name: "index", client: es_client) - expect(es_client).to have_received(:delete).with(index: "index", id: 123) - end - it "returns mappings for Elasticsearch version 7, which does not include a type" do - expect(described_class.mappings_properties(a: :b)).to eq({ a: :b }) - end - it "returns an identifier for bulk operations for Elasticsearch version 7, which does not include a type" do - expect(described_class.compatible_identifier({ a: :b })).to eq({ a: :b }) - end - end - - context "when the server is OpenSearch" do - let(:version_info) do - { "version" => { "distribution" => "opensearch" } } - end - it "returns true" do - expect(described_class.es7?).to eq(true) - end - it "calls 'search' with the right parameters, without including type" do - described_class.search(index_name: "index", body: { a: :b }, client: es_client) - expect(es_client).to have_received(:search).with(index: "index", - track_total_hits: true, - body: { a: :b }) - end - it "calls 'index' with the right parameters, without including type" do - described_class.index(id: 123, index_name: "index", atts: { a: :b }, params: { c: :d }, client: es_client) - expect(es_client).to have_received(:index).with(id: 123, - index: "index", - body: { a: :b }, - c: :d) - end - it "returns mappings without type" do - expect(described_class.compatible_mappings({ a: :b })) - .to eq({ "properties" => { a: :b } }) - end - it "calls 'put_mapping' with the right parameters, without including type" do - described_class.put_mapping(index_name: "index", mapping: { a: :b }, client: es_client) - expect(indices_client).to have_received(:put_mapping).with(index: "index", - body: { a: :b }) - end - it "calls 'delete' with the right parameters, without including type" do - described_class.delete(id: 123, index_name: "index", client: es_client) - expect(es_client).to have_received(:delete).with(index: "index", id: 123) - end - it "returns mappings for Opensearch, which does not include a type" do - expect(described_class.mappings_properties(a: :b)).to eq({ a: :b }) - end - it "returns an identifier for bulk operations for Opensearch, which does not include a type" do - expect(described_class.compatible_identifier({ a: :b })).to eq({ a: :b }) - end - end - - context "when Elasticsearch version is 6.x" do - let(:version_info) do - { "version" => { "number" => "6.8.23" } } - end - it "returns false" do - expect(described_class.es7?).to eq(false) - end - it "calls 'search' with the right parameters, including type" do - described_class.search(index_name: "index", body: { a: :b }, client: es_client) - expect(es_client).to have_received(:search).with(index: "index", - body: { a: :b }, - type: "generic-document") - end - it "calls 'index' with the right parameters, including type" do - described_class.index(id: 123, index_name: "index", atts: { a: :b }, params: { c: :d }, client: es_client) - expect(es_client).to have_received(:index).with(id: 123, - index: "index", - body: { a: :b }, - c: :d, - type: "generic-document") - end - it "returns mappings with type" do - expect(described_class.compatible_mappings({ a: :b })) - .to eq("generic-document" => { "properties" => { a: :b } }) - end - it "calls 'put_mapping' with the right parameters, including type" do - described_class.put_mapping(index_name: "index", mapping: { a: :b }, client: es_client) - expect(indices_client).to have_received(:put_mapping).with(index: "index", - body: { a: :b }, - type: "generic-document") - end - it "calls 'delete' with the right parameters, including type" do - described_class.delete(id: 123, index_name: "index", client: es_client) - expect(es_client).to have_received(:delete).with(index: "index", id: 123, type: "generic-document") - end - it "returns mappings for Elasticsearch version 6, which includes a type" do - expect(described_class.mappings_properties("generic-document" => { a: :b })).to eq({ a: :b }) - end - it "returns an identifier for bulk operations for Elasticsearch version 6, which includes a type" do - expect(described_class.compatible_identifier({ a: :b })).to eq({ "_type" => "generic-document", a: :b }) - end - end - end - end -end diff --git a/spec/unit/elasticsearch_index_spec.rb b/spec/unit/elasticsearch_index_spec.rb index 112bc91a0..8c3c26fab 100644 --- a/spec/unit/elasticsearch_index_spec.rb +++ b/spec/unit/elasticsearch_index_spec.rb @@ -80,7 +80,7 @@ stub_request(:post, search_pattern).with( body: { query: { term: { format: "organisation" } }, _source: { includes: %w[title link] }, sort: %w[_doc] }, ).to_return( - body: { _scroll_id: "abcdefgh", hits: { total: 10, hits: [] } }.to_json, + body: { _scroll_id: "abcdefgh", hits: { total: { value: 10, relation: "eq" }, hits: [] } }.to_json, headers: { "Content-Type" => "application/json" }, ) @@ -105,7 +105,7 @@ stub_request(:post, search_pattern).with( body: "{\"query\":{\"term\":{\"format\":\"organisation\"}},\"_source\":{\"includes\":[\"title\",\"link\"]},\"sort\":[\"_doc\"]}", ).to_return( - body: { _scroll_id: "abcdefgh", hits: { total: 10, hits: [] } }.to_json, + body: { _scroll_id: "abcdefgh", hits: { total: { value: 10, relation: "eq" }, hits: [] } }.to_json, headers: { "Content-Type" => "application/json" }, ) diff --git a/spec/unit/result_set_presenter_spec.rb b/spec/unit/result_set_presenter_spec.rb index 6321500ba..bd45b6fee 100644 --- a/spec/unit/result_set_presenter_spec.rb +++ b/spec/unit/result_set_presenter_spec.rb @@ -41,7 +41,7 @@ def sample_es_response(extra = {}) { "hits" => { "hits" => sample_docs, - "total" => 3, + "total" => { "value" => 3, "relation" => "eq" }, }, }.merge(extra) end @@ -67,7 +67,7 @@ def search_presenter(options) results = { "hits" => { "hits" => [], - "total" => 0, + "total" => { "value" => 0, "relation" => "eq" }, }, } @output = described_class.new( diff --git a/spec/unit/search/aggregate_example_fetcher_spec.rb b/spec/unit/search/aggregate_example_fetcher_spec.rb index 862c6af4d..a8dae6203 100644 --- a/spec/unit/search/aggregate_example_fetcher_spec.rb +++ b/spec/unit/search/aggregate_example_fetcher_spec.rb @@ -38,7 +38,7 @@ def query_for_example_query(field, value, return_fields, query, filter) def response_for_example(total_examples, titles) { "hits" => { - "total" => total_examples, + "total" => { "value" => total_examples, "relation" => "eq" }, "hits" => titles.map do |title| { "_source" => { "title" => title } } end, @@ -66,7 +66,7 @@ def stub_index(_name) it "map a response to aggregates without fields" do fetcher = described_class.new(@index, {}, Search::QueryParameters.new, @builder) slugs = %w[a-slug-name] - response_list = [{ "hits" => { "total" => 1, "hits" => [{ "_id" => "a-slug-name" }] } }] + response_list = [{ "hits" => { "total" => { "value" => 1, "relation" => "eq" }, "hits" => [{ "_id" => "a-slug-name" }] } }] response = fetcher.send(:prepare_response, slugs, response_list) @@ -125,7 +125,7 @@ def stub_index(_name) response_for_example(1, %w[example_3]), ] }) - expect( + expect(@fetcher.fetch("sector" => %w[sector_1 sector_2])).to eq( "sector" => { "sector_1" => { total: 3, examples: [ @@ -137,7 +137,7 @@ def stub_index(_name) { "title" => "example_3" }, ] }, }, - ).to eq(@fetcher.fetch("sector" => %w[sector_1 sector_2])) + ) end end @@ -187,7 +187,7 @@ def stub_index(_name) response_for_example(1, %w[example_3]), ] }) - expect( + expect(@fetcher.fetch("sector" => %w[sector_1 sector_2])).to eq( "sector" => { "sector_1" => { total: 3, examples: [ @@ -199,7 +199,7 @@ def stub_index(_name) { "title" => "example_3" }, ] }, }, - ).to eq(@fetcher.fetch("sector" => %w[sector_1 sector_2])) + ) end end @@ -285,7 +285,7 @@ def stub_index(_name) .with(expected_queries).and_return({ "responses" => stub_responses }) end - expect( + expect(@fetcher.fetch("sector" => (0..999).map { |sector_num| "sector_#{sector_num}" })).to eq( "sector" => Hash[ (0..999).map do |sector_num| [ @@ -294,7 +294,7 @@ def stub_index(_name) ] end, ], - ).to eq(@fetcher.fetch("sector" => (0..999).map { |sector_num| "sector_#{sector_num}" })) + ) end end end