From: Galen Charlton Date: Tue, 15 Dec 2020 23:07:00 +0000 (-0500) Subject: lp1863252 toward geosort X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=e135131dac1d873b3c84db6dee114f947cbe1f13;p=evergreen%2Fmasslnc.git lp1863252 toward geosort * fix swap of lat/lon that broke Math::Trig-based distance calculations TODO: might want to drop that entirely in favor of delegating to a DB call that uses earthdistance * first pass at implementing display of distance in the OPAC * include "Distance" label on mobile view of copy table * display warning if address input is not translated to coordinates * add distance column to TPAC copy table * add default item sort button to both TPAC and Bootstrap * changes to open-ils.geo registration - don't register with the public router - add to param redaction list * add temporary caching of address => coordinates results By default, coordinates are cached in memcached for 5 minutes. The cache key is derived from a SHA-2 hash of the input address. * implement query parameter log redaction as a PerLogHandler * Don't need perl-script for a PerlLogHandler Signed-off-by: Galen Charlton Signed-off-by: Terran McCanna --- diff --git a/Open-ILS/examples/apache_24/eg_vhost.conf.in b/Open-ILS/examples/apache_24/eg_vhost.conf.in index 2a78307d12..53f411ddb5 100644 --- a/Open-ILS/examples/apache_24/eg_vhost.conf.in +++ b/Open-ILS/examples/apache_24/eg_vhost.conf.in @@ -45,6 +45,15 @@ OSRFTranslatorConfig @sysconfdir@/opensrf_core.xml # Translator memcache server. Default is localhost # OSRFTranslatorCacheServer 127.0.0.1:11211 +# ---------------------------------------------------------------------------------- +# Log redaction +# ---------------------------------------------------------------------------------- + + # handler for redacting URL query parameters from + # the access log + PerlLogHandler OpenILS::WWW::EGWeb::log_handler + PerlAddVar OILSUrlParamToRedact "geographic-location" + # ---------------------------------------------------------------------------------- # Added content plugin diff --git a/Open-ILS/examples/opensrf.xml.example b/Open-ILS/examples/opensrf.xml.example index 8d26f16aef..c713fa38ba 100644 --- a/Open-ILS/examples/opensrf.xml.example +++ b/Open-ILS/examples/opensrf.xml.example @@ -753,6 +753,7 @@ vim:et:ts=4:sw=4: 5 + 300 diff --git a/Open-ILS/examples/opensrf_core.xml.example b/Open-ILS/examples/opensrf_core.xml.example index dfa0c3b5d9..aeba4ad94c 100644 --- a/Open-ILS/examples/opensrf_core.xml.example +++ b/Open-ILS/examples/opensrf_core.xml.example @@ -30,7 +30,6 @@ Example OpenSRF bootstrap configuration file for Evergreen open-ils.courses open-ils.curbside open-ils.fielder - open-ils.geo open-ils.pcrud open-ils.permacrud open-ils.reporter @@ -194,6 +193,8 @@ Example OpenSRF bootstrap configuration file for Evergreen open-ils.cstore.direct.actor.user.update open-ils.cstore.direct.actor.user.delete open-ils.search.z3950.apply_credentials + open-ils.geo + open-ils.actor.geo diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Geo.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Geo.pm index 0d2ba8c606..05874ded24 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Geo.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Geo.pm @@ -7,8 +7,10 @@ use OpenSRF::AppSession; use OpenILS::Application; use base qw/OpenILS::Application/; +use OpenSRF::Utils::SettingsClient; use OpenILS::Utils::CStoreEditor qw/:funcs/; use OpenILS::Utils::Fieldmapper; +use OpenSRF::Utils::Cache; use OpenILS::Application::AppUtils; my $U = "OpenILS::Application::AppUtils"; @@ -19,6 +21,20 @@ use Geo::Coder::OSM; use Geo::Coder::Google; use Math::Trig qw(great_circle_distance deg2rad); +use Digest::SHA qw(sha256_base64); + +my $cache; +my $cache_timeout; + +sub initialize { + my $conf = OpenSRF::Utils::SettingsClient->new; + + $cache_timeout = $conf->config_value( + "apps", "open-ils.geo", "app_settings", "cache_timeout" ) || 300; +} +sub child_init { + $cache = OpenSRF::Utils::Cache->new('global'); +} sub calculate_distance { my ($self, $conn, $pointA, $pointB) = @_; @@ -28,7 +44,7 @@ sub calculate_distance { return new OpenILS::Event("BAD_PARAMS", "desc" => "Malformed coordinates") unless scalar(@{ $pointA }) == 2; return new OpenILS::Event("BAD_PARAMS", "desc" => "Malformed coordinates") unless scalar(@{ $pointB }) == 2; - sub NESW { deg2rad($_[0]), deg2rad(90 - $_[1]) } + sub NESW { deg2rad($_[1]), deg2rad(90 - $_[0]) } # longitude, latitude my @A = NESW( $pointA->[0], $pointA->[1] ); my @B = NESW( $pointB->[0], $pointB->[1] ); my $km = great_circle_distance(@A, @B, 6378); @@ -131,7 +147,18 @@ sub retrieve_coordinates { # invoke 3rd party API for latitude/longitude lookup my $service = $e->retrieve_config_geolocation_service($service_id); return new OpenILS::Event("GEOCODING_NOT_ALLOWED") unless ($U->is_true($service)); + $address =~ s/^\s+//; + $address =~ s/\s+$//; return new OpenILS::Event("BAD_PARAMS", "desc" => "No address supplied") unless $address; + + # Return cached coordinates if available. We're assuming that any + # geolocation service will give roughly equivalent results, so we're + # using a hash of the user-supplied address as the cache key, not + # address + OU. + my $cache_key = 'geo.address.' . sha256_base64($address); + my $coords = OpenSRF::Utils::JSON->JSON2perl($cache->get_cache($cache_key)); + return $coords if $coords; + my $geo_coder; eval { if ($service->service_code eq 'Free') { @@ -170,8 +197,10 @@ sub retrieve_coordinates { # invoke 3rd party API for latitude/longitude lookup $latitude = $location->{lat}; $longitude = $location->{lon}; } + $coords = { latitude => $latitude, longitude => $longitude }; + $cache->put_cache($cache_key, OpenSRF::Utils::JSON->perl2JSON($coords), $cache_timeout); - return { latitude => $latitude, longitude => $longitude } + return $coords; } __PACKAGE__->register_method( method => "retrieve_coordinates", diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm index ca5b34e1d1..0fa267b3bd 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Record.pm @@ -7,6 +7,7 @@ use OpenILS::Utils::Fieldmapper; use OpenILS::Application::AppUtils; use Net::HTTP::NB; use IO::Select; +use List::MoreUtils qw(uniq); my $U = 'OpenILS::Application::AppUtils'; our $ac_types = ['toc', 'anotes', 'excerpt', 'summary', 'reviews']; @@ -59,12 +60,32 @@ sub load_record { $self->timelog("load user lists and settings"); } + # fetch geographic coordinates if user supplied an + # address + my $gl = $self->cgi->param('geographic-location'); + my $coords; + if ($gl) { + my $geo = OpenSRF::AppSession->create("open-ils.geo"); + $coords = $geo + ->request('open-ils.geo.retrieve_coordinates', $org, scalar $gl) + ->gather(1); + $geo->kill_me; + } + $ctx->{has_valid_coords} = 0; + if ($coords + && ref($coords) + && $$coords{latitude} + && $$coords{longitude} + ) { + $ctx->{has_valid_coords} = 1; + } + # run copy retrieval in parallel to bib retrieval # XXX unapi my $cstore = OpenSRF::AppSession->create('open-ils.cstore'); my $copy_rec = $cstore->request( 'open-ils.cstore.json_query.atomic', - $self->mk_copy_query($rec_id, $org, $copy_depth, $copy_limit, $copy_offset, $pref_ou) + $self->mk_copy_query($rec_id, $org, $copy_depth, $copy_limit, $copy_offset, $pref_ou, $coords) ); if ($self->cgi->param('badges')) { @@ -107,6 +128,24 @@ sub load_record { $ctx->{course_module_opt_in} = 1; } + $ctx->{ou_distances} = {}; + if ($ctx->{has_valid_coords}) { + my $circ_libs = [ uniq map { $_->{circ_lib} } @{$ctx->{copies}} ]; + my $foreign_copy_circ_libs = [ + map { $_->target_copy()->circ_lib() } + map { @{ $_->foreign_copy_maps() } } + @{ $ctx->{foreign_copies} } + ]; + push @{ $circ_libs }, @$foreign_copy_circ_libs; # some overlap is OK here + my $ou_distance_list = $U->simplereq( + 'open-ils.geo', + 'open-ils.geo.sort_orgs_by_distance_from_coordinate.include_distances', + [ $coords->{latitude}, $coords->{longitude} ], + $circ_libs + ); + $ctx->{ou_distances} = { map { $_->[0] => $_->[1] } @$ou_distance_list }; + } + # Add public copy notes to each copy - and while we're in there, grab peer bib records # and copy tags. Oh and if we're working with course materials, those too. my %cached_bibs = (); @@ -310,6 +349,7 @@ sub mk_copy_query { my $copy_limit = shift; my $copy_offset = shift; my $pref_ou = shift; + my $coords = shift; my $query = $U->basic_opac_copy_query( $rec_id, undef, undef, $copy_limit, $copy_offset, $self->ctx->{is_staff} @@ -341,21 +381,14 @@ sub mk_copy_query { }}; }; - my $ou_sort_param = [$org, $pref_ou ]; - my $gl = $self->cgi->param('geographic-location'); - if ($gl) { - my $geo = OpenSRF::AppSession->create("open-ils.geo"); - my $coords = $geo - ->request('open-ils.geo.retrieve_coordinates', $org, scalar $gl) - ->gather(1); - if ($coords - && ref($coords) - && $$coords{latitude} - && $$coords{longitude} - ) { - push(@$ou_sort_param, $$coords{latitude}, $$coords{longitude}); - } - } + my $ou_sort_param = [$org, $pref_ou ]; + if ($coords + && ref($coords) + && $$coords{latitude} + && $$coords{longitude} + ) { + push(@$ou_sort_param, $$coords{latitude}, $$coords{longitude}); + } # Unsure if we want these in the shared function, leaving here for now unshift(@{$query->{order_by}}, diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGWeb.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGWeb.pm index 0d2fe261b4..8d48debf5e 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGWeb.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGWeb.pm @@ -49,6 +49,27 @@ sub child_init { return Apache2::Const::OK; } +sub log_handler { + my $r = shift; + + my @params_to_redact = uniq $r->dir_config->get('OILSUrlParamToRedact'); + my $re = '('. join('|', map { quotemeta($_) } @params_to_redact) . ')=(?:[^&;]*)'; + + my $args = $r->args(); + $args =~ s/$re/$1=[REDACTED]/g; + $r->args($args); + my $req = $r->the_request(); # munging args doesn't update the + # original requested URI + $req =~ s/$re/$1=[REDACTED]/g; + $r->the_request($req); + + if ($r->headers_in->{Referer}) { + $r->headers_in->{Referer} =~ s/$re/$1=[REDACTED]/g; + } + + return Apache2::Const::OK; +} + sub handler { my $r = shift; my $stat = handler_guts($r); diff --git a/Open-ILS/src/templates-bootstrap/opac/css/mediaQuery.css.tt2 b/Open-ILS/src/templates-bootstrap/opac/css/mediaQuery.css.tt2 index 66439e7a1b..61055439c9 100644 --- a/Open-ILS/src/templates-bootstrap/opac/css/mediaQuery.css.tt2 +++ b/Open-ILS/src/templates-bootstrap/opac/css/mediaQuery.css.tt2 @@ -66,6 +66,7 @@ only screen and (max-width: 650px) { .copyTable td:nth-of-type(4):before { content: "Shelving Location"; display: flex;} .copyTable td:nth-of-type(5):before { content: "Status"; display: flex;} .copyTable td:nth-of-type(6):before { content: "Due Date"; display: flex;} + .copyTable td:nth-of-type(7):before { content: "[% l('Distance') %]"; display: flex;} .holdingsTable tr:nth-of-type(1):before { content: "Copy #1"; display: block; text-align:center; } .holdingsTable tr:nth-of-type(2):before { content: "Copy #2"; display: block; text-align:center;} diff --git a/Open-ILS/src/templates-bootstrap/opac/css/style.css.tt2 b/Open-ILS/src/templates-bootstrap/opac/css/style.css.tt2 index 77c6bc2e1b..74faa1edf1 100755 --- a/Open-ILS/src/templates-bootstrap/opac/css/style.css.tt2 +++ b/Open-ILS/src/templates-bootstrap/opac/css/style.css.tt2 @@ -87,6 +87,7 @@ only screen and (max-width: 650px) { .copyTable td:nth-of-type(4):before { content: "Shelving Location"; display: flex;} .copyTable td:nth-of-type(5):before { content: "Status"; display: flex;} .copyTable td:nth-of-type(6):before { content: "Due Date"; display: flex;} + .copyTable td:nth-of-type(7):before { content: "[% l('Distance') %]"; display: flex;} .holdingsTable tr:nth-of-type(1):before { content: "Copy #1"; display: block; text-align:center; } .holdingsTable tr:nth-of-type(2):before { content: "Copy #2"; display: block; text-align:center;} diff --git a/Open-ILS/src/templates-bootstrap/opac/parts/record/copy_table.tt2 b/Open-ILS/src/templates-bootstrap/opac/parts/record/copy_table.tt2 index 9bf4fd96cb..6851879ce6 100755 --- a/Open-ILS/src/templates-bootstrap/opac/parts/record/copy_table.tt2 +++ b/Open-ILS/src/templates-bootstrap/opac/parts/record/copy_table.tt2 @@ -25,6 +25,20 @@ FOREACH copy_info IN copies; END; END; -%] +[%- MACRO display_ou_distance(ou) BLOCK; + km = ctx.ou_distances.$ou; + IF km && km != '-1'; + IF ctx.get_org_setting(ctx.physical_loc || ctx.search_ou, 'opac.geographic_proximity_in_miles'); + distance = l('[_1] mi', POSIX.sprintf('%.01f', km / 1.609)); + ELSE; + distance = l('[_1] km', POSIX.sprintf('%.01f', km)); + END; + ELSE; + distance = '-'; + END; +%] +[% distance %] +[%- END %] [%- IF has_copies or ctx.foreign_copies; depth = CGI.param('copy_depth').defined ? CGI.param('copy_depth') : CGI.param('depth').defined ? CGI.param('depth') : ctx.copy_summary.last.depth; @@ -40,8 +54,19 @@ IF has_copies or ctx.foreign_copies; +[% p = 'geographic-location'; IF CGI.params.$p && !ctx.has_valid_coords %] +[% l('Sorry, your address is not recognized') %] +[% END %] + +[% p = 'geographic-location'; IF CGI.params.$p && ctx.has_valid_coords %] +
+[% FOREACH p IN CGI.params.keys; NEXT IF p == 'geographic-location' %] + +[% END %] +
[% END %] +[% END %] @@ -70,6 +95,8 @@ IF has_copies or ctx.foreign_copies; [%- IF ctx.use_courses %] + [%- IF ctx.geo_sort && ctx.has_valid_coords %] + [%- END %] @@ -92,6 +119,9 @@ IF has_copies or ctx.foreign_copies; + [%- IF ctx.geo_sort && ctx.has_valid_coords %] + + [%- END %] [%- END; # FOREACH peer END; # FOREACH bib @@ -250,6 +280,9 @@ END; # FOREACH bib
[% course.course_number %]
[% END %] [% END %] + [%- IF ctx.geo_sort && ctx.has_valid_coords %] + + [%- END %] [% IF copy_info.notes; %] diff --git a/Open-ILS/src/templates/opac/parts/record/copy_table.tt2 b/Open-ILS/src/templates/opac/parts/record/copy_table.tt2 index f62e96ef7c..dc47ab7d15 100644 --- a/Open-ILS/src/templates/opac/parts/record/copy_table.tt2 +++ b/Open-ILS/src/templates/opac/parts/record/copy_table.tt2 @@ -25,6 +25,20 @@ FOREACH copy_info IN copies; END; END; -%] +[%- MACRO display_ou_distance(ou) BLOCK; + km = ctx.ou_distances.$ou; + IF km && km != '-1'; + IF ctx.get_org_setting(ctx.physical_loc || ctx.search_ou, 'opac.geographic_proximity_in_miles'); + distance = l('[_1] mi', POSIX.sprintf('%.01f', km / 1.609)); + ELSE; + distance = l('[_1] km', POSIX.sprintf('%.01f', km)); + END; + ELSE; + distance = '-'; + END; +%] +[% distance %] +[%- END %] [%- IF has_copies or ctx.foreign_copies; depth = CGI.param('copy_depth').defined ? CGI.param('copy_depth') : CGI.param('depth').defined ? CGI.param('depth') : ctx.copy_summary.last.depth; @@ -42,7 +56,18 @@ IF has_copies or ctx.foreign_copies; +[% p = 'geographic-location'; IF CGI.params.$p && !ctx.has_valid_coords %] +[% l('Sorry, your address is not recognized') %] +[% END %] +[% p = 'geographic-location'; IF CGI.params.$p && ctx.has_valid_coords %] + +[% FOREACH p IN CGI.params.keys; NEXT IF p == 'geographic-location' %] + +[% END %] + + +[% END %] [% END %]
[% l("Due Date") %][% l("Courses") %][% l("Distance") %]
[% bib.target_copy.location.name | html %] [% bib.target_copy.status.name | html %] [% date.format(ctx.parse_datetime(copy_info.due_date, copy_info.circ_circ_lib),DATE_FORMAT) %][% display_ou_distance(bib.target_copy.circ_lib) %]
[% display_ou_distance(copy_info.circ_lib) %]
@@ -73,6 +98,9 @@ IF has_copies or ctx.foreign_copies; [%- IF use_courses %] [%- END %] + [%- IF ctx.geo_sort && ctx.has_valid_coords %] + + [%- END %] @@ -98,6 +126,9 @@ IF has_copies or ctx.foreign_copies; OR use_courses %] [%- END %] + [%- IF ctx.geo_sort && ctx.has_valid_coords %] + + [%- END %] [%- END; # FOREACH peer END; # FOREACH bib @@ -256,6 +287,9 @@ END; # FOREACH bib
[% course.course_number %]
[% END %] [% END %] + [%- IF ctx.geo_sort && ctx.has_valid_coords %] + + [%- END %] [% IF copy_info.notes; %]
[% l("Courses") %][% l("Distance") %]
[% display_ou_distance(bib.target_copy.circ_lib) %]
[% display_ou_distance(copy_info.circ_lib) %]