Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand context when the value is an URI and expasion was requested in the config #204

Merged
merged 2 commits into from
Feb 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/models/qa/linked_data/config/context_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def add_group(group_id, group_map)
return if groups.key? group_id
i18n_key = Qa::LinkedData::Config::Helper.fetch(group_map, :group_label_i18n, nil)
default = Qa::LinkedData::Config::Helper.fetch(group_map, :group_label_default, nil)
return groups[group_id] = I18n.t(i18n_key, default) if i18n_key.present?
return groups[group_id] = I18n.t(i18n_key, default: default) if i18n_key.present?
Copy link
Contributor Author

@elrayle elrayle Feb 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUGFIX: passing default incorrectly to i18n

groups[group_id] = default
end
end
Expand Down
89 changes: 75 additions & 14 deletions app/models/qa/linked_data/config/context_property_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ module Qa
module LinkedData
module Config
class ContextPropertyMap
VALUE_ON_ERROR = [].freeze

attr_reader :group_id, # id that identifies which group the property should be in
:label # plain text label extracted from locales or using the default
:label

attr_reader :property_map, :ldpath, :prefixes
private :property_map, :ldpath, :prefixes
attr_reader :property_map, :ldpath, :expansion_label_ldpath, :expansion_id_ldpath, :prefixes
private :property_map, :ldpath, :expansion_label_ldpath, :expansion_id_ldpath, :prefixes

# @param [Hash] property_map defining information to return to provide context
# @option property_map [String] :group_id (optional) default label to use for a property (default: no label)
Expand All @@ -35,30 +37,55 @@ def initialize(property_map = {}, prefixes = {})
@ldpath = Qa::LinkedData::Config::Helper.fetch_required(property_map, :ldpath, false)
@selectable = Qa::LinkedData::Config::Helper.fetch_boolean(property_map, :selectable, false)
@drillable = Qa::LinkedData::Config::Helper.fetch_boolean(property_map, :drillable, false)
@expansion_label_ldpath = Qa::LinkedData::Config::Helper.fetch(property_map, :expansion_label_ldpath, nil)
@expansion_id_ldpath = Qa::LinkedData::Config::Helper.fetch(property_map, :expansion_id_ldpath, nil)
@prefixes = prefixes
end

# Can this property be the selected value?
# @return true if can be selected; otherwise, false
# @return [Boolean] true if this property's value can be selected; otherwise, false
def selectable?
@selectable
end

# Can this property be used as a new query
# @return true if can be selected; otherwise, false
# @return [Boolean] true if this property's value can be used to drill up/down to another level; otherwise, false
def drillable?
@drillable
end

def group?
group_id.present?
end
Copy link
Contributor Author

@elrayle elrayle Feb 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

READABILITY: Just moved group? up so all the ? methods are together. No code change.


# Should this URI value be expanded to include its label?
# @return [Boolean] true if this property's value is expected to be a URI and its label should be included in the value; otherwise, false
def expand_uri?
expansion_label_ldpath.present?
end

# Values of this property for a specfic subject URI
# @return [Array<String>] values for this property
def values(graph, subject_uri)
output = ldpath_program.evaluate subject_uri, graph
output.present? ? output['property'].uniq : nil
rescue
'PARSE ERROR'
ldpath_evaluate(basic_program, graph, subject_uri)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY: Moved the ldpath evaluate process to a private method so it can be shared by multiple methods.

end

def group?
group_id.present?
# Values of this property for a specfic subject URI with URI values expanded to include id and label.
# @return [Array<Hash>] expanded values for this property
# @example returned values
# [{
# uri: "http://id.loc.gov/authorities/genreForms/gf2014026551",
# id: "gf2014026551",
# label: "Space operas"
# }]
def expanded_values(graph, subject_uri)
values = values(graph, subject_uri)
return values unless expand_uri?
return values unless values.respond_to? :map!
values.map! do |uri|
{ uri: uri, id: expansion_id(graph, uri), label: expansion_label(graph, uri) }
end
values
end

private
Expand All @@ -70,12 +97,46 @@ def extract_label
default
end

def ldpath_program
return @program if @program.present?
def basic_program
@basic_program ||= ldpath_program(ldpath)
end

def expansion_label_program
@expansion_label_program ||= ldpath_program(expansion_label_ldpath)
end

def expansion_id_program
@expansion_id_program ||= ldpath_program(expansion_id_ldpath)
end

def ldpath_program(ldpath)
program_code = ""
prefixes.each { |key, url| program_code << "@prefix #{key} : <#{url}> \;\n" }
program_code << "property = #{ldpath} \;"
@program = Ldpath::Program.parse program_code
Ldpath::Program.parse program_code
rescue => e
Rails.logger.warn("WARNING: #{I18n.t('qa.linked_data.ldpath.parse_logger_error')} (ldpath='#{ldpath}')\n cause: #{e.message}")
raise StandardError, I18n.t('qa.linked_data.ldpath.parse_error')
end

def ldpath_evaluate(program, graph, subject_uri)
return VALUE_ON_ERROR if program.blank?
output = program.evaluate subject_uri, graph
output.present? ? output['property'].uniq : nil
rescue => e
Rails.logger.warn("WARNING: #{I18n.t('qa.linked_data.ldpath.evaluate_logger_error')} (ldpath='#{ldpath}')\n cause: #{e.message}")
raise StandardError, I18n.t('qa.linked_data.ldpath.evaluate_error')
end

def expansion_label(graph, uri)
label = ldpath_evaluate(expansion_label_program, graph, RDF::URI(uri))
label.size == 1 ? label.first : label
end

def expansion_id(graph, uri)
return uri if expansion_id_ldpath.blank?
id = ldpath_evaluate(expansion_id_program, graph, RDF::URI(uri))
id.size == 1 ? id.first : id
end
end
end
Expand Down
23 changes: 18 additions & 5 deletions app/services/qa/linked_data/mapper/context_mapper_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,37 @@ class << self
def map_context(graph:, context_map:, subject_uri:)
context = []
context_map.properties.each do |property_map|
values = property_map.values(graph, subject_uri)
next if values.blank?
context << construct_context(context_map, property_map, values)
populated_property_map = populate_property_map(context_map, property_map, graph, subject_uri)
next if populated_property_map.blank?
context << populated_property_map
end
context
end

private

def construct_context(context_map, property_map, values)
def populate_property_map(context_map, property_map, graph, subject_uri)
begin
values = property_values(property_map, graph, subject_uri)
rescue => e
values = Qa::LinkedData::Config::ContextPropertyMap::VALUE_ON_ERROR
error = e.message
end

property_info = {}
property_info["group"] = context_map.group_label(property_map.group_id) if property_map.group? # TODO: should be group label
property_info["group"] = context_map.group_label(property_map.group_id) if property_map.group?
property_info["property"] = property_map.label
property_info["values"] = values
property_info["selectable"] = property_map.selectable?
property_info["drillable"] = property_map.drillable?
property_info["error"] = error if error.present?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

property_info
end

def property_values(property_map, graph, subject_uri)
return property_map.expanded_values(graph, subject_uri) if property_map.expand_uri?
property_map.values(graph, subject_uri)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,31 @@ def map_values(graph:, predicate_map:, sort_key:, preferred_language: nil, conte
graph.subjects.each do |subject|
next if subject.anonymous? # skip blank nodes
values = graph_mapper_service.map_values(graph: graph, predicate_map: predicate_map, subject_uri: subject) do |value_map|
next value_map if context_map.blank?
context = {}
context = context_mapper_service.map_context(graph: graph, context_map: context_map, subject_uri: subject) if context_map.present?
value_map[:context] = context
value_map
map_context(graph, sort_key, context_map, value_map, subject)
Copy link
Contributor Author

@elrayle elrayle Feb 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RUBOCOP: Extracted to a private method to appease rubocop.

end
search_matches << values unless sort_key.present? && values[sort_key].blank?
search_matches << values if result_subject? values, sort_key
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY: Extracted to a private method so the check for sort_key can happen here and in the context mapping code.

end
search_matches = deep_sort_service.new(search_matches, sort_key, preferred_language).sort
search_matches
end

private

# The graph mapper creates the basic value_map for all subject URIs, but we only want the ones that represent search results.
def result_subject?(value_map, sort_key)
return true unless sort_key.present? # if sort_key is not defined, then all subjects are considered matches
return false unless value_map.key? sort_key # otherwise, sort_key must be in the basic value_map
value_map[sort_key].present? # AND have a value for this to be a search result
end

def map_context(graph, sort_key, context_map, value_map, subject)
return value_map if context_map.blank?
return value_map unless result_subject? value_map, sort_key
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PERFORMANCE: Added this check because we shouldn't take the time to map context if the current subject isn't an actual search result.

context = {}
context = context_mapper_service.map_context(graph: graph, context_map: context_map, subject_uri: subject) if context_map.present?
value_map[:context] = context
value_map
end
end
end
end
Expand Down
9 changes: 9 additions & 0 deletions config/locales/qa.en.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
en:
qa:
linked_data:
ldpath:
evaluate_error: LDPATH EVALUATION ERROR (See log for more information)
evaluate_logger_error: LDPath failed to evaluate the graph with the provided ldpath.
parse_error: LDPATH PARSE ERROR (See log for more information)
parse_logger_error: LDPath failed to parse during ldpath program creation.
24 changes: 24 additions & 0 deletions spec/fixtures/authorities/linked_data/lod_full_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@
"dates": {
"group_label_i18n": "qa.linked_data.authority.locnames_ld4l_cache.dates",
"group_label_default": "Dates"
},
"hierarchy": {
"group_label_i18n": "qa.linked_data.authority.locgenres_ld4l_cache.hierarchy",
"group_label_default": "Hierarchy"
}
},
"properties": [
Expand All @@ -128,6 +132,26 @@
"ldpath": "madsrdf:identifiesRWO/madsrdf:birthDate/schema:label",
"selectable": false,
"drillable": false
},
{
"group_id": "hierarchy",
"property_label_i18n": "qa.linked_data.authority.locgenres_ld4l_cache.narrower",
"property_label_default": "Narrower",
"ldpath": "skos:narrower :: xsd:string",
"selectable": true,
"drillable": true,
"expansion_label_ldpath": "skos:prefLabel ::xsd:string",
"expansion_id_ldpath": "loc:lccn ::xsd:string"
},
{
"group_id": "hierarchy",
"property_label_i18n": "qa.linked_data.authority.locgenres_ld4l_cache.broader",
"property_label_default": "Broader",
"ldpath": "skos:broader :: xsd:string",
"selectable": true,
"drillable": true,
"expansion_label_ldpath": "skos:prefLabel ::xsd:string",
"expansion_id_ldpath": "loc:lccn ::xsd:string"
}
]
},
Expand Down
24 changes: 24 additions & 0 deletions spec/lib/authorities/linked_data/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@
dates: {
group_label_i18n: "qa.linked_data.authority.locnames_ld4l_cache.dates",
group_label_default: "Dates"
},
hierarchy: {
group_label_i18n: "qa.linked_data.authority.locgenres_ld4l_cache.hierarchy",
group_label_default: "Hierarchy"
}
},
properties: [
Expand All @@ -153,6 +157,26 @@
ldpath: "madsrdf:identifiesRWO/madsrdf:birthDate/schema:label",
selectable: false,
drillable: false
},
{
group_id: "hierarchy",
property_label_i18n: "qa.linked_data.authority.locgenres_ld4l_cache.narrower",
property_label_default: "Narrower",
ldpath: "skos:narrower :: xsd:string",
selectable: true,
drillable: true,
expansion_label_ldpath: "skos:prefLabel ::xsd:string",
expansion_id_ldpath: "loc:lccn ::xsd:string"
},
{
group_id: "hierarchy",
property_label_i18n: "qa.linked_data.authority.locgenres_ld4l_cache.broader",
property_label_default: "Broader",
ldpath: "skos:broader :: xsd:string",
selectable: true,
drillable: true,
expansion_label_ldpath: "skos:prefLabel ::xsd:string",
expansion_id_ldpath: "loc:lccn ::xsd:string"
}
]
},
Expand Down
24 changes: 24 additions & 0 deletions spec/lib/authorities/linked_data/search_config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@
dates: {
group_label_i18n: "qa.linked_data.authority.locnames_ld4l_cache.dates",
group_label_default: "Dates"
},
hierarchy: {
group_label_i18n: "qa.linked_data.authority.locgenres_ld4l_cache.hierarchy",
group_label_default: "Hierarchy"
}
},
properties: [
Expand All @@ -77,6 +81,26 @@
ldpath: "madsrdf:identifiesRWO/madsrdf:birthDate/schema:label",
selectable: false,
drillable: false
},
{
group_id: "hierarchy",
property_label_i18n: "qa.linked_data.authority.locgenres_ld4l_cache.narrower",
property_label_default: "Narrower",
ldpath: "skos:narrower :: xsd:string",
selectable: true,
drillable: true,
expansion_label_ldpath: "skos:prefLabel ::xsd:string",
expansion_id_ldpath: "loc:lccn ::xsd:string"
},
{
group_id: "hierarchy",
property_label_i18n: "qa.linked_data.authority.locgenres_ld4l_cache.broader",
property_label_default: "Broader",
ldpath: "skos:broader :: xsd:string",
selectable: true,
drillable: true,
expansion_label_ldpath: "skos:prefLabel ::xsd:string",
expansion_id_ldpath: "loc:lccn ::xsd:string"
}
]
},
Expand Down
10 changes: 5 additions & 5 deletions spec/models/linked_data/config/context_map_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@
before do
# simulate the existence of the i18n entries
allow(I18n).to receive(:t).and_call_original
allow(I18n).to receive(:t).with('qa.linked_data.authority.locnames_ld4l_cache.dates', 'default_Dates').and_return('Dates')
allow(I18n).to receive(:t).with('qa.linked_data.authority.locnames_ld4l_cache.authoritative_label', 'default_Authoritative Label').and_return('Authoritative Label')
allow(I18n).to receive(:t).with('qa.linked_data.authority.locnames_ld4l_cache.birth_date', 'default_Birth').and_return('Birth')
allow(I18n).to receive(:t).with('qa.linked_data.authority.locnames_ld4l_cache.death_date', 'default_Death').and_return('Death')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEADCODE: These weren't needed for the tests to pass.

end

let(:context_map) do
Expand Down Expand Up @@ -75,6 +71,10 @@
describe '#group_label' do
context 'when map defines group_label_i18n key' do
context 'and i18n translation is defined in locales' do
before do
allow(I18n).to receive(:t).with('qa.linked_data.authority.locnames_ld4l_cache.dates', default: 'default_Dates').and_return('Dates')
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

READABILITY: Moved closer to test.


it 'returns the translated text' do
expect(subject.group_label(:dates)).to eq 'Dates'
end
Expand All @@ -83,7 +83,7 @@
context 'and i18n translation is NOT defined in locales' do
context 'and default is defined in the map' do
before do
allow(I18n).to receive(:t).with('qa.linked_data.authority.locnames_ld4l_cache.dates', 'default_Dates').and_return('default_Dates')
allow(I18n).to receive(:t).with('qa.linked_data.authority.locnames_ld4l_cache.dates', default: 'default_Dates').and_return('default_Dates')
end

it 'returns the default value' do
Expand Down