refactor optional attributes for linked data search/fetch to pass as hash #279

elrayle · 2019-11-06T17:08:55Z

The list of parameters that are passed to search and find methods for the linked data module are long. There is a need to add another parameter. In prep for that, this refactors the optional parameters into a single request_header parameter. This same change is also in AuthorityUrlService which builds the URLs used in search and find and is the primary consumer of the optional params.

All other changes are in the callers of these methods, which is coming from LinkdedDataTermsController actions.

The longer individual parameter lists are deprecated.

elrayle · 2019-11-06T17:49:29Z

app/controllers/qa/linked_data_terms_controller.rb


  delegate :cors_allow_origin_header, to: Qa::ApplicationController

+  class_attribute :request_header_service_class
+  self.request_header_service_class = Qa::LinkedData::RequestHeaderService


This service is moving some of the parameter processing out of the controller. It simplifies the controller code and allows the service to focus on the parameter processing and construction of the request_header being passed to search and find.

This looks much cleaner 👍

elrayle · 2019-11-06T17:50:14Z

app/controllers/qa/linked_data_terms_controller.rb

@@ -190,64 +200,19 @@ def id
      params[:id]
    end

-    def language


The removed parameter processing methods now live in the RequestHeaderService

elrayle · 2019-11-06T17:51:32Z

app/services/qa/linked_data/authority_url_service.rb

-        def build_url(action_config:, action:, action_request:, substitutions: {}, subauthority: nil, language: nil) # rubocop:disable Metrics/ParameterLists
+        # @note All parameters after request_header are deprecated and will be removed in the next major release.
+        def build_url(action_config:, action:, action_request:, request_header: {}, substitutions: {}, subauthority: nil, language: nil) # rubocop:disable Metrics/ParameterLists
+          request_header = build_request_header(substitutions, subauthority, language) if request_header.empty?


This call is to provide deprecated availability of the individual parameters.

elrayle · 2019-11-06T17:52:19Z

app/services/qa/linked_data/authority_url_service.rb

@@ -37,29 +42,36 @@ def action_request_variable(action_config, action)
            action_config.qa_replacement_patterns[key]
          end

-          def supports_subauthorities?(action_config)


Since this is only called once from a private method, it is collapsed into the caller.

elrayle · 2019-11-06T17:52:37Z

app/services/qa/linked_data/authority_url_service.rb

-            action_config.subauthorities[subauthority.to_sym]
-          end
-
-          def supports_language_parameter?(action_config)


Since this is only called once from a private method, it is collapsed into the caller.

elrayle · 2019-11-06T17:53:06Z

app/services/qa/linked_data/authority_url_service.rb

+            request_header.fetch(:language, []).first
+          end
+
+          def build_request_header(substitutions, subauthority, language) # rubocop:disable Metrics/CyclomaticComplexity


providing support for deprecated individual parameters

is that worth a comment?

elrayle · 2019-11-06T17:54:06Z

lib/qa/authorities/linked_data/find_term.rb

-      def find(id, language: nil, replacements: {}, subauth: nil, format: nil, jsonld: false, performance_data: false) # rubocop:disable Metrics/ParameterLists, Metrics/MethodLength
-        # TODO: When jsonld parameter is removed, the format parameter should default to 'json'.  Not making this change now for backward compatibility of the default for jsonld parameter.
+      def find(id, request_header: {}, language: nil, replacements: {}, subauth: nil, format: 'json', performance_data: false) # rubocop:disable Metrics/ParameterLists
+        request_header = build_request_header(language: language, replacements: replacements, subauth: subauth, format: format, performance_data: performance_data) if request_header.empty?


This call is to provide deprecated availability of the individual parameters.

elrayle · 2019-11-06T17:54:20Z

lib/qa/authorities/linked_data/find_term.rb

@@ -290,6 +288,22 @@ def append_performance_data(results)
                          total_time_s: (access_time_s + normalize_time_s) }
          { performance: performance, results: results }
        end
+
+        def build_request_header(language:, replacements:, subauth:, format:, performance_data:) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity


providing support for deprecated individual parameters

elrayle · 2019-11-06T17:56:16Z

lib/qa/authorities/linked_data/find_term.rb

        @id = id
-        @performance_data = performance_data
-        @format = format
-        @jsonld = jsonld if @format.blank?


The changes in this PR and another one coming soon will require a major release change. So support of the deprecated jsonld parameter is being removed. There will be more deprecation removals before the major release, but they will come in a separate PR. This was removed now because of the parameter changes happening in this PR.

elrayle · 2019-11-06T17:56:59Z

lib/qa/authorities/linked_data/find_term.rb

@@ -75,14 +73,14 @@ def load_graph(url:)
        def normalize_results
          normalize_start_dt = Time.now.utc

-          json = perform_normalization
+          results = perform_normalization


Results is more accurate name. It should have changed when support for n3 and ntriples was added.

elrayle · 2019-11-06T17:57:42Z

lib/qa/authorities/linked_data/search_query.rb

      # @example Json Results for Linked Data Search
      #   [ {"uri":"http://id.worldcat.org/fast/5140","id":"5140","label":"Cornell, Joseph"},
      #     {"uri":"http://id.worldcat.org/fast/72456","id":"72456","label":"Cornell, Sarah Maria, 1802-1832"},
      #     {"uri":"http://id.worldcat.org/fast/409667","id":"409667","label":"Cornell, Ezra, 1807-1874"} ]
-      def search(query, language: nil, replacements: {}, subauth: nil, context: false, performance_data: false) # rubocop:disable Metrics/ParameterLists
+      def search(query, request_header: {}, language: nil, replacements: {}, subauth: nil, context: false, performance_data: false) # rubocop:disable Metrics/ParameterLists
+        request_header = build_request_header(language: language, replacements: replacements, subauth: subauth, context: context, performance_data: performance_data) if request_header.empty?


This call is to provide deprecated availability of the individual parameters.

elrayle · 2019-11-06T17:58:00Z

lib/qa/authorities/linked_data/search_query.rb

@@ -180,6 +184,22 @@ def append_performance_data(results)
                          total_time_s: (access_time_s + normalize_time_s) }
          { performance: performance, results: results }
        end
+
+        def build_request_header(language:, replacements:, subauth:, context:, performance_data:) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity


providing support for deprecated individual parameters

…hash

ghost · 2019-11-07T12:21:09Z

app/controllers/qa/linked_data_terms_controller.rb


  delegate :cors_allow_origin_header, to: Qa::ApplicationController

+  class_attribute :request_header_service_class
+  self.request_header_service_class = Qa::LinkedData::RequestHeaderService


This looks much cleaner 👍

ghost · 2019-11-07T12:32:11Z

app/services/qa/linked_data/authority_url_service.rb

+            request_header.fetch(:language, []).first
+          end
+
+          def build_request_header(substitutions, subauthority, language) # rubocop:disable Metrics/CyclomaticComplexity


is that worth a comment?

elrayle · 2019-11-08T19:00:14Z

@geekscruff Thanks for the review. I added in a comment about deprecation of individual parameter above each of the build_request_header methods.

elrayle commented Nov 6, 2019

View reviewed changes

refactor optional attributes for linked data search/fetch to pass as …

102e479

…hash

elrayle force-pushed the refactor_opt_attrs branch 3 times, most recently from 709be1c to 7835382 Compare November 6, 2019 20:27

elrayle changed the title ~~[WIP] refactor optional attributes for linked data search/fetch to pass as hash~~ refactor optional attributes for linked data search/fetch to pass as hash Nov 6, 2019

ghost approved these changes Nov 7, 2019

View reviewed changes

update language in request header to use preferred language

1d405a2

elrayle force-pushed the refactor_opt_attrs branch from 51a67fb to 1d405a2 Compare November 8, 2019 18:59

elrayle merged commit 56e683e into master Nov 8, 2019

elrayle deleted the refactor_opt_attrs branch November 8, 2019 19:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor optional attributes for linked data search/fetch to pass as hash #279

refactor optional attributes for linked data search/fetch to pass as hash #279

elrayle commented Nov 6, 2019 •

edited

elrayle Nov 6, 2019

ghost Nov 7, 2019

elrayle Nov 6, 2019

elrayle Nov 6, 2019

elrayle Nov 6, 2019

elrayle Nov 6, 2019

elrayle Nov 6, 2019

ghost Nov 7, 2019

elrayle Nov 6, 2019

elrayle Nov 6, 2019

elrayle Nov 6, 2019

elrayle Nov 6, 2019

elrayle Nov 6, 2019

elrayle Nov 6, 2019

ghost Nov 7, 2019

ghost Nov 7, 2019

elrayle commented Nov 8, 2019

refactor optional attributes for linked data search/fetch to pass as hash #279

refactor optional attributes for linked data search/fetch to pass as hash #279

Conversation

elrayle commented Nov 6, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elrayle commented Nov 8, 2019

elrayle commented Nov 6, 2019 •

edited