allow language to be specified in the http accept_language header #223

elrayle · 2019-04-16T21:07:17Z

Adds a few other improvements…

allow * as a wildcard for language processing which will prevent filtering of literals by language
do not include parameters that do not have a value when building URLs
clean template built URL to remove any occurances of ?&, &&, or ending with &

Adds a few other language improvements… * do not include parameters that do not have a value when building URLs * clean template built URL to remove any occurances of ?&, &&, or ending with & * allow * as a wildcard for language processing which will prevent filtering of literals by language (TBD)

elrayle · 2019-04-16T23:26:34Z

.circleci/config.yml

@@ -29,7 +29,7 @@ jobs:
          project: qa

      - samvera/engine_cart_generate:
-          cache_key: v1-internal-test-app-{{ checksum "qa.gemspec" }}-{{ checksum "spec/test_app_templates/lib/generators/test_app_generator.rb" }}-{{ checksum "lib/generators/qa/install/install_generator.rb" }}-<< parameters.rails_version >>-<< parameters.ruby_version >>
+          cache_key: v2-internal-test-app-{{ checksum "qa.gemspec" }}-{{ checksum "spec/test_app_templates/lib/generators/test_app_generator.rb" }}-{{ checksum "lib/generators/qa/install/install_generator.rb" }}-<< parameters.rails_version >>-<< parameters.ruby_version >>


Required to force Circle CI cache to rebuild and pick up the new linked_data fixtures.

elrayle · 2019-04-16T23:27:09Z

app/controllers/qa/linked_data_terms_controller.rb

-      params[:lang]
+      request_language = request.env['HTTP_ACCEPT_LANGUAGE']
+      request_language = request_language.scan(/^[a-z]{2}/).first if request_language.present?
+      params[:lang] || request_language


Extends language specification to honor HTTP_ACCEPT_LANGUAGE if specified.

elrayle · 2019-04-16T23:27:35Z

app/controllers/qa/linked_data_terms_controller.rb

    end

    def subauthority
      params[:subauthority]
    end

    def replacement_params
-      params.reject { |k, _v| ['q', 'vocab', 'controller', 'action', 'subauthority', 'language', 'id'].include?(k) }
+      params.reject { |k, _v| ['q', 'vocab', 'controller', 'action', 'subauthority', 'lang', 'id'].include?(k) }


This should have always been lang instead of language.

elrayle · 2019-04-16T23:28:41Z

app/services/qa/iri_template_service.rb

-        url = url.gsub("{#{key}}", m.simple_value(substitutions[key]))
-        url = url.gsub("{?#{key}}", m.parameter_value(substitutions[key]))
+    class << self
+      # Construct an url from an IriTemplate making identified substitutions


This is mostly an indentation change by adding class << self. This allows the private methods to be specified under the private section.

elrayle · 2019-04-16T23:29:09Z

app/services/qa/iri_template_service.rb

+        # In the process of substitution, if a value is missing, you can end up with '?&', '&&', or ending with '&'.  These are all removed this method.
+        def clean(url)
+          url.gsub(/&&*/, '&').chomp('&').gsub('?&', '?')
+        end


The clean method is the only change to this service.

elrayle · 2019-04-16T23:31:12Z

app/services/qa/linked_data/authority_url_service.rb

            substitutions[action_request_variable(action_config, action)] = action_request
-            substitutions[action_subauth_variable(action_config)] = action_subauth_variable_value(action_config, subauthority) if subauthority.present?
+            substitutions[action_subauth_variable(action_config)] = action_subauth_variable_value(action_config, subauthority) if supports_subauthorities?(action_config) && subauthority.present?
+            substitutions[action_language_variable(action_config)] = language_value(language) if supports_language_parameter?(action_config) && language.present?


lang used to be passed in as part of the substitutions. This was an artifact of lang failing to be removed in the controller. Now that it is properly removed, it has to be passed in by name. This is cleaner as it makes the language processing more visible.

elrayle · 2019-04-16T23:35:07Z

app/services/qa/linked_data/language_service.rb

@@ -22,6 +24,7 @@ def literal_has_language_marker?(literal)
          def normalize_language(language)
            return language if language.blank?
            language = [language] unless language.is_a? Array
+            return nil if language.include?(WILDCARD)


By setting the language to nil, there will be no filtering. When an authority configuration sets the language to *, it overrides the site wide default and prevents filtering for that authority.

elrayle · 2019-04-16T23:37:02Z

lib/qa/authorities/linked_data/find_term.rb

@@ -37,7 +37,7 @@ def initialize(term_config)
      def find(id, language: nil, replacements: {}, subauth: nil, jsonld: false)
        raise Qa::InvalidLinkedDataAuthority, "Unable to initialize linked data term sub-authority #{subauth}" unless subauth.nil? || term_subauthority?(subauth)
        @language = Qa::LinkedData::LanguageService.preferred_language(user_language: language, authority_language: term_config.term_language)
-        url = Qa::LinkedData::AuthorityUrlService.build_url(action_config: term_config, action: :term, action_request: id, substitutions: replacements, subauthority: subauth)
+        url = Qa::LinkedData::AuthorityUrlService.build_url(action_config: term_config, action: :term, action_request: id, substitutions: replacements, subauthority: subauth, language: @language)


After determining the preferred_language, it was then being ignored by passing language to the AuthorityUrlService. Changing this to @language causes the preferred language to be correctly passed instead.

elrayle · 2019-04-16T23:38:19Z

lib/qa/authorities/linked_data/search_query.rb

@@ -35,7 +35,7 @@ def search(query, language: nil, replacements: {}, subauth: nil, context: false)
        raise Qa::InvalidLinkedDataAuthority, "Unable to initialize linked data search sub-authority #{subauth}" unless subauth.nil? || subauthority?(subauth)
        @context = context
        @language = language_service.preferred_language(user_language: language, authority_language: search_config.language)
-        url = authority_service.build_url(action_config: search_config, action: :search, action_request: query, substitutions: replacements, subauthority: subauth)
+        url = authority_service.build_url(action_config: search_config, action: :search, action_request: query, substitutions: replacements, subauthority: subauth, language: @language)


Same here. Needs to pass @language which is the preferred langauge, instead of always the user/http language which may be nil if not set.

elrayle · 2019-04-16T23:39:52Z

spec/lib/authorities/linked_data/search_config_spec.rb

@@ -12,7 +13,7 @@
        url: {
          :@context => 'http://www.w3.org/ns/hydra/context.jsonld',
          :@type => 'IriTemplate',
-          template: 'http://localhost/test_default/search?{?subauth}&{?query}&{?param1}&{?param2}',
+          template: 'http://localhost/test_default/search?{?subauth}&{?query}&{?param1}&{?param2}&{?lang}',


The updates to the expected full_config here corresponds to the changes to the lod_full_config.json fixture.

elrayle · 2019-04-16T23:40:12Z

spec/lib/authorities/linked_data/term_config_spec.rb

@@ -12,7 +13,7 @@
        url: {
          :@context => 'http://www.w3.org/ns/hydra/context.jsonld',
          :@type => 'IriTemplate',
-          template: 'http://localhost/test_default/term/{subauth}/{term_id}?{?param1}&{?param2}',
+          template: 'http://localhost/test_default/term/{subauth}/{term_id}?{?param1}&{?param2}&{?lang}',


The updates to the expected full_config here corresponds to the changes to the lod_full_config.json fixture.

elrayle · 2019-04-16T23:41:13Z

spec/lib/authorities/linked_data/term_config_spec.rb

@@ -156,7 +172,7 @@
      expect(min_config.term_language).to eq nil
    end
    it 'returns the preferred language for selecting literal values if configured for term' do
-      expect(full_config.term_language).to eq [:en]
+      expect(full_config.term_language).to eq [:es]


This value was changed in lod_full_config.json fixture to make each language different so we can be sure the language we are checking came for the right place in the config.

elrayle · 2019-04-16T23:44:29Z

spec/services/linked_data/language_service_spec.rb

-      it 'returns default language from Qa configuration' do
-        expect(subject).to match_array [:en]
+      context 'and site wide default language is set' do
+        before do


These test changes are covering the various scenarios for setting language, including the new WILDCARD.

carolyncole

I just had one question about the logic. Otherwise look solid.

carolyncole · 2019-04-17T16:37:40Z

app/services/qa/linked_data/authority_url_service.rb

+          end
+
+          def language_value(language)
+            return nil if language.blank? || !language.is_a?(Array)


@elrayle Just checking the logic here. If the language is 'EN' you will return nil, but if the language is ['EN'] you will return 'EN' . Is this correct?

Will return nil when language is... nil, :en, 'en'
Will return :en when language is... ['en']

The reason this is ok is because the parameter is expected to be an array of symbols (line 12)

# @param language [Array<Symbol>] languages for filtering returned literals (optional)

The language is normalized to be an Array<Symbol> by the language_service.

I could raise an exception if language is not nil or an Array<Symbol> to make this clearer.

Went with removing || !language.is_a?(Array) since the param doc says it should be an array.

…eneration

elrayle added enhancement linked data labels Apr 16, 2019

elrayle self-assigned this Apr 16, 2019

elrayle force-pushed the lang_improvements branch 3 times, most recently from ddbecbf to e34f01e Compare April 16, 2019 21:32

elrayle force-pushed the lang_improvements branch 3 times, most recently from e87ad68 to 5cd213d Compare April 16, 2019 22:32

elrayle commented Apr 16, 2019

View reviewed changes

cjcolvar self-requested a review April 17, 2019 16:30

carolyncole reviewed Apr 17, 2019

View reviewed changes

add linked data test authorities to list of what causes CCI cache reg…

d403b70

…eneration

elrayle force-pushed the lang_improvements branch from 5cd213d to d403b70 Compare April 17, 2019 18:54

carolyncole approved these changes Apr 17, 2019

View reviewed changes

carolyncole merged commit b866dfc into master Apr 17, 2019

carolyncole deleted the lang_improvements branch April 17, 2019 19:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow language to be specified in the http accept_language header #223

allow language to be specified in the http accept_language header #223

elrayle commented Apr 16, 2019

elrayle Apr 16, 2019

elrayle Apr 16, 2019

elrayle Apr 16, 2019 •

edited

elrayle Apr 16, 2019

elrayle Apr 16, 2019

elrayle Apr 16, 2019

elrayle Apr 16, 2019 •

edited

elrayle Apr 16, 2019

elrayle Apr 16, 2019 •

edited

elrayle Apr 16, 2019

elrayle Apr 16, 2019

elrayle Apr 16, 2019

elrayle Apr 16, 2019

carolyncole left a comment

carolyncole Apr 17, 2019

elrayle Apr 17, 2019

elrayle Apr 17, 2019

allow language to be specified in the http accept_language header #223

allow language to be specified in the http accept_language header #223

Conversation

elrayle commented Apr 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elrayle Apr 16, 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

elrayle Apr 16, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elrayle Apr 16, 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

carolyncole left a 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 Apr 16, 2019 •

edited

elrayle Apr 16, 2019 •

edited

elrayle Apr 16, 2019 •

edited