@@ -, +, @@
behaviour
19uu, 195u, etc.
---
Koha/SearchEngine/Elasticsearch.pm | 51 +++++++++++----
Koha/SearchEngine/Elasticsearch/Indexer.pm | 3 +
admin/searchengine/elasticsearch/field_config.yaml | 2 +
.../bug_24807-add-year-search-field-type.perl | 7 +++
.../admin/searchengine/elasticsearch/mappings.tt | 5 ++
t/db_dependent/Koha/SearchEngine/Elasticsearch.t | 73 +++++++++++++++++++++-
.../Koha/SearchEngine/Elasticsearch/Indexer.t | 33 ++--------
7 files changed, 130 insertions(+), 44 deletions(-)
create mode 100644 installer/data/mysql/atomicupdate/bug_24807-add-year-search-field-type.perl
--- a/Koha/SearchEngine/Elasticsearch.pm
+++ a/Koha/SearchEngine/Elasticsearch.pm
@@ -39,7 +39,7 @@ use Search::Elasticsearch;
use Try::Tiny;
use YAML::Syck;
-use List::Util qw( sum0 reduce );
+use List::Util qw( sum0 reduce all );
use MARC::File::XML;
use MIME::Base64;
use Encode qw(encode);
@@ -209,6 +209,8 @@ sub get_elasticsearch_mappings {
$es_type = 'integer';
} elsif ($type eq 'isbn' || $type eq 'stdno') {
$es_type = 'stdno';
+ } elsif ($type eq 'year') {
+ $es_type = 'year';
}
if ($search) {
@@ -469,28 +471,44 @@ sub _process_mappings {
# Copy (scalar) data since can have multiple targets
# with differing options for (possibly) mutating data
# so need a different copy for each
- my $_data = $data;
- $record_document->{$target} //= [];
+ my $data_copy = $data;
if (defined $options->{substr}) {
my ($start, $length) = @{$options->{substr}};
- $_data = length($data) > $start ? substr $data, $start, $length : '';
+ $data_copy = length($data) > $start ? substr $data_copy, $start, $length : '';
}
+
+ # Add data to values array for callbacks processing
+ my $values = [$data_copy];
+
+ # Value callbacks takes subfield data (or values from previous
+ # callbacks) as argument, and returns a possibly different list of values.
+ # Note that the returned list may also be empty.
if (defined $options->{value_callbacks}) {
- $_data = reduce { $b->($a) } ($_data, @{$options->{value_callbacks}});
+ foreach my $callback (@{$options->{value_callbacks}}) {
+ # Pass each value to current callback which returns a list
+ # (scalar is fine too) resulting either in a list or
+ # a list of lists that will be flattened by perl.
+ # The next callback will receive the possibly expanded list of values.
+ $values = [ map { $callback->($_) } @{$values} ];
+ }
}
+
+ # Skip mapping if all values has been removed
+ next unless @{$values};
+
if (defined $options->{property}) {
- $_data = {
- $options->{property} => $_data
- }
+ $values = [ map { { $options->{property} => $_ } } @{$values} ];
}
if (defined $options->{nonfiling_characters_indicator}) {
my $nonfiling_chars = $meta->{field}->indicator($options->{nonfiling_characters_indicator});
$nonfiling_chars = looks_like_number($nonfiling_chars) ? int($nonfiling_chars) : 0;
- if ($nonfiling_chars) {
- $_data = substr $_data, $nonfiling_chars;
- }
+ # Nonfiling chars does not make sense for multiple values
+ # Only apply on first element
+ $values->[0] = substr $values->[0], $nonfiling_chars;
}
- push @{$record_document->{$target}}, $_data;
+
+ $record_document->{$target} //= [];
+ push @{$record_document->{$target}}, @{$values};
}
}
@@ -885,6 +903,15 @@ sub _field_mappings {
return $value ? 'true' : 'false';
};
}
+ elsif ($target_type eq 'year') {
+ $default_options->{value_callbacks} //= [];
+ # Only accept years containing digits and "u"
+ push @{$default_options->{value_callbacks}}, sub {
+ my ($value) = @_;
+ # Replace "u" with "0" for sorting
+ return map { s/[u\s]/0/gr } ( $value =~ /[0-9u\s]{4}/g );
+ };
+ }
if ($search) {
my $mapping = [$target_name, $default_options];
--- a/Koha/SearchEngine/Elasticsearch/Indexer.pm
+++ a/Koha/SearchEngine/Elasticsearch/Indexer.pm
@@ -119,6 +119,9 @@ sub update_index {
type => 'data', # is just hard coded in Indexer.pm?
body => \@body
);
+ if ($response->{errors}) {
+ carp "One or more ElasticSearch errors occurred when indexing documents";
+ }
}
return $response;
}
--- a/admin/searchengine/elasticsearch/field_config.yaml
+++ a/admin/searchengine/elasticsearch/field_config.yaml
@@ -25,6 +25,8 @@ search:
type: integer
null_value: 0
ignore_malformed: true
+ year:
+ type: short
stdno:
type: text
analyzer: analyzer_stdno
--- a/installer/data/mysql/atomicupdate/bug_24807-add-year-search-field-type.perl
+++ a/installer/data/mysql/atomicupdate/bug_24807-add-year-search-field-type.perl
@@ -0,0 +1,7 @@
+$DBversion = 'XXX'; # will be replaced by the RM
+if( CheckVersion( $DBversion ) ) {
+ $dbh->do( "ALTER TABLE `search_field` MODIFY COLUMN `type` enum('','string','date','number','boolean','sum','isbn','stdno','year') NOT NULL" );
+ $dbh->do( "UPDATE `search_field` SET type = 'year' WHERE name = 'date-of-publication'" );
+ SetVersion( $DBversion );
+ print "Upgrade to $DBversion done (Bug 14957 - Add 'year' type to improve sorting behaviour)\n";
+}
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
+++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt
@@ -196,6 +196,11 @@ a.add, a.delete {
[% ELSE %]
[% END %]
+ [% IF search_field.type == "year" %]
+
+ [% ELSE %]
+
+ [% END %]
[% IF search_field.type == "number" %]
[% ELSE %]
--- a/t/db_dependent/Koha/SearchEngine/Elasticsearch.t
+++ a/t/db_dependent/Koha/SearchEngine/Elasticsearch.t
@@ -132,7 +132,7 @@ subtest 'get_elasticsearch_mappings() tests' => sub {
subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests' => sub {
- plan tests => 53;
+ plan tests => 59;
t::lib::Mocks::mock_preference('marcflavour', 'MARC21');
t::lib::Mocks::mock_preference('ElasticsearchMARCFormat', 'ISO2709');
@@ -297,7 +297,28 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests'
sort => 1,
marc_type => 'marc21',
marc_field => '952l',
+ },
+ {
+ name => 'copydate',
+ type => 'year',
+ facet => 0,
+ suggestible => 0,
+ searchable => 1,
+ sort => 1,
+ marc_type => 'marc21',
+ marc_field => '260c',
+ },
+ {
+ name => 'date-of-publication',
+ type => 'year',
+ facet => 0,
+ suggestible => 0,
+ searchable => 1,
+ sort => 1,
+ marc_type => 'marc21',
+ marc_field => '008_/7-10',
},
+
);
my $se = Test::MockModule->new('Koha::SearchEngine::Elasticsearch');
@@ -329,12 +350,14 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests'
$marc_record_1->append_fields(
MARC::Field->new('001', '123'),
MARC::Field->new('007', 'ku'),
+ MARC::Field->new('008', '901111s1962 xxk|||| |00| ||eng c'),
MARC::Field->new('020', '', '', a => '1-56619-909-3'),
MARC::Field->new('100', '', '', a => 'Author 1'),
MARC::Field->new('110', '', '', a => 'Corp Author'),
MARC::Field->new('210', '', '', a => 'Title 1'),
MARC::Field->new('240', '', '4', a => 'The uniform title with nonfiling indicator'),
MARC::Field->new('245', '', '', a => 'Title:', b => 'first record'),
+ MARC::Field->new('260', '', '', a => 'New York :', b => 'Ace ,', c => 'c1962'),
MARC::Field->new('999', '', '', c => '1234567'),
# ' ' for testing trimming of white space in boolean value callback:
MARC::Field->new('952', '', '', 0 => ' ', g => '123.30', o => $callno, l => 3),
@@ -344,20 +367,34 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests'
my $marc_record_2 = MARC::Record->new();
$marc_record_2->leader(' cam 22 a 4500');
$marc_record_2->append_fields(
+ MARC::Field->new('008', '901111s19uu xxk|||| |00| ||eng c'),
MARC::Field->new('100', '', '', a => 'Author 2'),
# MARC::Field->new('210', '', '', a => 'Title 2'),
# MARC::Field->new('245', '', '', a => 'Title: second record'),
+ MARC::Field->new('260', '', '', a => 'New York :', b => 'Ace ,', c => '1963-2003'),
MARC::Field->new('999', '', '', c => '1234568'),
MARC::Field->new('952', '', '', 0 => 1, g => 'string where should be numeric', o => $long_callno),
);
- my $records = [ $marc_record_1, $marc_record_2 ];
+
+ my $marc_record_3 = MARC::Record->new();
+ $marc_record_3->leader(' cam 22 a 4500');
+ $marc_record_3->append_fields(
+ MARC::Field->new('008', '901111s19uu xxk|||| |00| ||eng c'),
+ MARC::Field->new('100', '', '', a => 'Author 2'),
+ # MARC::Field->new('210', '', '', a => 'Title 3'),
+ # MARC::Field->new('245', '', '', a => 'Title: third record'),
+ MARC::Field->new('260', '', '', a => 'New York :', b => 'Ace ,', c => ' 89 '),
+ MARC::Field->new('999', '', '', c => '1234568'),
+ MARC::Field->new('952', '', '', 0 => 1, g => 'string where should be numeric', o => $long_callno),
+ );
+ my $records = [$marc_record_1, $marc_record_2, $marc_record_3];
$see->get_elasticsearch_mappings(); #sort_fields will call this and use the actual db values unless we call it first
my $docs = $see->marc_records_to_documents($records);
# First record:
- is(scalar @{$docs}, 2, 'Two records converted to documents');
+ is(scalar @{$docs}, 3, 'Two records converted to documents');
is_deeply($docs->[0]->{control_number}, ['123'], 'First record control number should be set correctly');
@@ -470,6 +507,16 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests'
'First document uniform_title__sort field should contain the title with the first four initial characters removed'
);
+ # Tests for 'year' type
+ is(scalar @{$docs->[0]->{'date-of-publication'}}, 1, 'First document date-of-publication field should contain one value');
+ is_deeply($docs->[0]->{'date-of-publication'}, ['1962'], 'First document date-of-publication field should be set correctly');
+
+ is_deeply(
+ $docs->[0]->{'copydate'},
+ ['1962'],
+ 'First document copydate field should be set correctly'
+ );
+
# Second record:
is(scalar @{$docs->[1]->{author}}, 1, 'Second document author field should contain one value');
@@ -494,6 +541,26 @@ subtest 'Koha::SearchEngine::Elasticsearch::marc_records_to_documents () tests'
'Second document local_classification__sort field should be set correctly'
);
+ # Tests for 'year' type
+ is_deeply(
+ $docs->[1]->{'copydate'},
+ ['1963', '2003'],
+ 'Second document copydate field should be set correctly'
+ );
+ is_deeply(
+ $docs->[1]->{'date-of-publication'},
+ ['1900'],
+ 'Second document date-of-publication field should be set correctly'
+ );
+
+ # Third record:
+
+ is_deeply(
+ $docs->[2]->{'copydate'},
+ ['0890'],
+ 'Third document copydate field should be set correctly'
+ );
+
# Mappings marc_type:
ok(!(defined $docs->[0]->{unimarc_title}), "No mapping when marc_type doesn't match marc flavour");
--- a/t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t
+++ a/t/db_dependent/Koha/SearchEngine/Elasticsearch/Indexer.t
@@ -19,7 +19,7 @@
use Modern::Perl;
-use Test::More tests => 3;
+use Test::More tests => 2;
use Test::MockModule;
use t::lib::Mocks;
@@ -35,11 +35,11 @@ SKIP: {
eval { Koha::SearchEngine::Elasticsearch->get_elasticsearch_params; };
- skip 'Elasticsearch configuration not available', 2
+ skip 'Elasticsearch configuration not available', 1
if $@;
subtest 'create_index() tests' => sub {
- plan tests => 5;
+ plan tests => 6;
my $se = Test::MockModule->new( 'Koha::SearchEngine::Elasticsearch' );
$se->mock( '_read_configuration', sub {
my ($self, $sub ) = @_;
@@ -72,6 +72,7 @@ subtest 'create_index() tests' => sub {
my $response = $indexer->update_index([1], $records);
is( $response->{errors}, 0, "no error on update_index" );
is( scalar(@{$response->{items}}), 1, "1 item indexed" );
+ is( $response->{items}[0]->{index}->{_id},"1", "We should get a string matching the bibnumber passed in");
is(
$indexer->drop_index(),
@@ -79,30 +80,4 @@ subtest 'create_index() tests' => sub {
'Dropping the index'
);
};
-
-subtest 'update_index() tests' => sub {
- plan tests => 2;
- my $kse = Test::MockModule->new( 'Koha::SearchEngine::Elasticsearch' );
- $kse->mock( 'marc_records_to_documents', sub {
- my ($self, $params ) = @_;
- return [1];
- });
-
- my $indexer;
- ok(
- $indexer = Koha::SearchEngine::Elasticsearch::Indexer->new({ 'index' => 'biblios' }),
- 'Creating a new indexer object'
- );
-
- my $searcher = $indexer->get_elasticsearch();
- my $se = Test::MockModule->new( ref $searcher );
- $se->mock( 'bulk', sub {
- my ($self, %params ) = @_;
- return $params{body};
- });
-
- my $bibnumber_array = $indexer->update_index([13],["faked"]);
- is( $bibnumber_array->[0]->{index}->{_id},"13", "We should get a string matching the bibnumber");
-};
-
}
--