Bugzilla – Attachment 48268 Details for
Bug 15875
Refactor DB accessors from "z39.50 admin setup / z3950servers.pl" to C4::Breeding
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15875 - Refactor DB accessors from "z39.50 admin setup / z3950servers.pl" to C4::Breeding
Bug-15875---Refactor-DB-accessors-from-z3950-admin.patch (text/plain), 11.39 KB, created by
Olli-Antti Kivilahti
on 2016-02-22 10:38:40 UTC
(
hide
)
Description:
Bug 15875 - Refactor DB accessors from "z39.50 admin setup / z3950servers.pl" to C4::Breeding
Filename:
MIME Type:
Creator:
Olli-Antti Kivilahti
Created:
2016-02-22 10:38:40 UTC
Size:
11.39 KB
patch
obsolete
>From 96914c563eb5688351d29def9697a242a78f3552 Mon Sep 17 00:00:00 2001 >From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> >Date: Mon, 22 Feb 2016 12:33:07 +0200 >Subject: [PATCH] Bug 15875 - Refactor DB accessors from "z39.50 admin setup / > z3950servers.pl" to C4::Breeding > >z3950servers.pl is a horrible mess of hard SQL in the perl script/Controller-layer. >Also a lack of proper internal API for mutating Z3950servers makes writing tests much harder. > >This patch refactors z3950servers.pl by moving SQL to C4::Breeding under proper accessors. >--- > C4/Breeding.pm | 165 +++++++++++++++++++++++++++++++++++++++++++++++++- > admin/z3950servers.pl | 92 +++++++++++----------------- > 2 files changed, 200 insertions(+), 57 deletions(-) > >diff --git a/C4/Breeding.pm b/C4/Breeding.pm >index af7a4c2..9d3d0df 100644 >--- a/C4/Breeding.pm >+++ b/C4/Breeding.pm >@@ -20,6 +20,7 @@ package C4::Breeding; > > use strict; > use warnings; >+use Data::Dumper; > > use C4::Biblio; > use C4::Koha; >@@ -28,6 +29,10 @@ use MARC::File::USMARC; > use C4::ImportBatch; > use C4::AuthoritiesMarc; #GuessAuthTypeCode, FindDuplicateAuthority > >+use Koha::Exception::DB; >+use Koha::Exception::DuplicateObject; >+use Koha::Exception::BadParameter; >+ > use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); > > BEGIN { >@@ -675,6 +680,164 @@ sub Z3950SearchAuth { > ); > } > >+=head getZ3950server >+ >+ my $servers = C4::Breeding::listZ3950servers({id => 443, name => "geigei"}); >+ >+@RETURNS ARRAYRef of z3950servers-rows >+ >+=cut >+ >+sub listZ3950servers { >+ my ($params) = @_; >+ my $dbh=C4::Context->dbh; >+ my ($sth, $sql); >+ >+ $sql = "SELECT * FROM z3950servers "; >+ my @params; >+ my @andedColumns; >+ if (ref($params) eq 'HASH') { >+ while (my ($column, $value) = each(%$params)) { >+ push(@andedColumns, "$column = ?"); >+ push(@params, $value); >+ } >+ } >+ if (@params) { >+ $sql .= 'WHERE '.join(' AND ', @andedColumns); >+ } >+ $sql .= ' ORDER BY rank,name'; >+ >+ eval { >+ $sth = $dbh->prepare($sql); >+ $sth->execute( @params ); >+ }; >+ if ($@ || $sth->err) { >+ my @cc = caller(0); >+ my $paramsStr = Data::Dumper::Dumper($params); >+ Koha::Exception::DB->throw(error => $cc[3].'():>'.($@ || $sth->errstr)."\nWith search terms\n$paramsStr\n"); >+ } >+ return $sth->fetchall_arrayref({}); >+} >+ >+=head getZ3950server >+ >+ my $server = C4::Breeding::getZ3950server({id => 443, name => "geigei"}); >+ >+=cut >+ >+sub getZ3950server { >+ my ($params) = @_; >+ my $servers = listZ3950servers($params); >+ >+ if (not(ref($servers) eq 'ARRAY') || not(scalar(@$servers))) { >+ return undef; >+ } >+ elsif (scalar(@$servers) > 1) { >+ my @cc = caller(0); >+ my $paramsStr = Data::Dumper::Dumper($params); >+ Koha::Exception::DuplicateObject->throw(error => $cc[3]."():> Too many Z3950 servers found with search terms\n$paramsStr\n"); >+ } >+ return $servers->[0]; >+} >+ >+sub updateZ3950server { >+ my ($params) = @_; >+ my $dbh=C4::Context->dbh; >+ my ($sth, $sql); >+ >+ unless($params->{name}) { >+ my @cc = caller(0); >+ Koha::Exception::BadParameter->throw( error => $cc[3]."():> Missing attribute 'name'!" ); >+ } >+ >+ eval { >+ $sth=$dbh->prepare("update z3950servers set host=?, port=?, db=?, userid=?, password=?, name=?, checked=?, rank=?,syntax=?,encoding=?,timeout=?,recordtype=? where name=?"); >+ $sth->execute( >+ $params->{'host'}, >+ $params->{'port'}, >+ $params->{'db'}, >+ $params->{'userid'}, >+ $params->{'password'}, >+ $params->{'name'}, >+ $params->{'checked'} ? 1 : 0, >+ $params->{'rank'}, >+ $params->{'syntax'}, >+ $params->{'encoding'}, >+ $params->{'timeout'}, >+ $params->{'recordtype'}, >+ $params->{'name'}, >+ ); >+ }; >+ if ($@ || $sth->err) { >+ my @cc = caller(0); >+ Koha::Exception::DB->throw(error => $cc[3].'():>'.($@ || $sth->errstr)); >+ } >+} >+ >+sub addZ3950server { >+ my ($params) = @_; >+ my $dbh=C4::Context->dbh; >+ my ($sth, $sql); >+ >+ unless($params->{name}) { >+ my @cc = caller(0); >+ Koha::Exception::BadParameter->throw( error => $cc[3]."():> Missing attribute 'name'!" ); >+ } >+ >+ eval { >+ $sth=$dbh->prepare( >+ "INSERT INTO z3950servers " . >+ " (host,port,db,userid,password,name,checked,rank,syntax,encoding,timeout,recordtype) " . >+ "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" ); >+ $sth->execute( >+ $params->{'host'}, >+ $params->{'port'}, >+ $params->{'db'}, >+ $params->{'userid'}, >+ $params->{'password'}, >+ $params->{'name'}, >+ $params->{'checked'} ? 1 : 0, >+ $params->{'rank'}, >+ $params->{'syntax'}, >+ $params->{'encoding'}, >+ $params->{'timeout'}, >+ $params->{'recordtype'}, >+ ); >+ }; >+ if ($@ || $sth->err) { >+ my @cc = caller(0); >+ Koha::Exception::DB->throw(error => $cc[3].'():>'.($@ || $sth->errstr)); >+ } >+} >+ >+sub upsertZ3950server { >+ my ($params) = @_; >+ my $dbh=C4::Context->dbh; >+ my ($sth, $sql, $server); >+ >+ $server = getZ3950server( $params->{id} ? {id => $params->{id}} : {name => $params->{name}} ); >+ if ($server) { >+ C4::Breeding::updateZ3950server($params); >+ } >+ else { >+ C4::Breeding::addZ3950server($params); >+ } >+} >+ >+sub deleteZ3950server { >+ my ($id) = @_; >+ my $dbh=C4::Context->dbh; >+ my ($sth, $sql); >+ >+ eval { >+ $sth=$dbh->prepare("DELETE FROM z3950servers WHERE id=?"); >+ $sth->execute($id); >+ }; >+ if ($@ || $sth->err) { >+ my @cc = caller(0); >+ Koha::Exception::DB->throw(error => $cc[3].'():>'.($@ || $sth->errstr)); >+ } >+} >+ > 1; > __END__ >- >diff --git a/admin/z3950servers.pl b/admin/z3950servers.pl >index 1dbd118..2b7060a 100755 >--- a/admin/z3950servers.pl >+++ b/admin/z3950servers.pl >@@ -25,6 +25,7 @@ use CGI; > use C4::Context; > use C4::Auth; > use C4::Output; >+use C4::Breeding; > > sub StringSearch { > my ($searchstring,$type)=@_; >@@ -90,14 +91,8 @@ if ( $op eq 'add_form' ) { > > #---- if primkey exists, it's a modify action, so read values to modify... > my $data; >- if ($searchfield) { >- my $dbh = C4::Context->dbh; >- my $sth = $dbh->prepare( >-"select host,port,db,userid,password,name,id,checked,rank,syntax,encoding,timeout,recordtype from z3950servers where (name = ?) order by rank,name" >- ); >- $sth->execute($searchfield); >- $data = $sth->fetchrow_hashref; >- $sth->finish; >+ if (defined($searchfield)) { >+ $data = C4::Breeding::getZ3950server({name => $searchfield}); > } > $template->param( $_ => $data->{$_} ) > for (qw( host port db userid password checked rank timeout encoding )); >@@ -107,58 +102,25 @@ if ( $op eq 'add_form' ) { > ################## ADD_VALIDATE ################################## > # called by add_form, used to insert/modify data in DB > } elsif ($op eq 'add_validate') { >- my $dbh=C4::Context->dbh; >- my $sth=$dbh->prepare("select * from z3950servers where name=?"); >- $sth->execute($input->param('searchfield')); >- my $checked = $input->param('checked') ? 1 : 0; >- if ($sth->rows) { >+ my $params = castCGIToZ3950serverParams($input); >+ my $dbh=C4::Context->dbh; >+ my ($sth, $sql, $server); >+ >+ $server = C4::Breeding::getZ3950server( $params->{id} ? {id => $params->{id}} : {name => $params->{name}} ); >+ if ($server) { >+ C4::Breeding::updateZ3950server($params); > $template->param(confirm_update => 1); >- $sth=$dbh->prepare("update z3950servers set host=?, port=?, db=?, userid=?, password=?, name=?, checked=?, rank=?,syntax=?,encoding=?,timeout=?,recordtype=? where name=?"); >- $sth->execute($input->param('host'), >- $input->param('port'), >- $input->param('db'), >- $input->param('userid'), >- $input->param('password'), >- $input->param('searchfield'), >- $checked, >- $input->param('rank'), >- $input->param('syntax'), >- $input->param('encoding'), >- $input->param('timeout'), >- $input->param('recordtype'), >- $input->param('searchfield'), >- ); >- } >- else { >+ } >+ else { >+ C4::Breeding::addZ3950server($params); > $template->param(confirm_add => 1); >- $sth=$dbh->prepare( >- "INSERT INTO z3950servers " . >- "(host,port,db,userid,password,name,checked,rank,syntax,encoding,timeout,recordtype) " . >- "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" ); >- $sth->execute( >- $input->param('host'), $input->param('port'), >- $input->param('db'), $input->param('userid'), >- $input->param('password'), $input->param('searchfield'), >- $checked, $input->param('rank'), >- $input->param('syntax'), $input->param('encoding'), >- $input->param('timeout'), $input->param('recordtype') >- ); > } >- $sth->finish; >- > # END $OP eq ADD_VALIDATE > ################## DELETE_CONFIRM ################################## > # called by default form, used to confirm deletion of data in DB > } elsif ($op eq 'delete_confirm') { > $template->param( delete_confirm => 1 ); >- my $dbh = C4::Context->dbh; >- >- my $sth2 = $dbh->prepare( >-"select host,port,db,userid,password,name,id,checked,rank,syntax,encoding,timeout,recordtype from z3950servers where (name = ?) order by rank,name" >- ); >- $sth2->execute($searchfield); >- my $data = $sth2->fetchrow_hashref; >- $sth2->finish; >+ my $data = C4::Breeding::getZ3950server({name => $searchfield}); > > $template->param( > host => $data->{'host'}, >@@ -179,10 +141,8 @@ if ( $op eq 'add_form' ) { > # called by delete_confirm, used to effectively confirm deletion of data in DB > } elsif ($op eq 'delete_confirmed') { > $template->param(delete_confirmed => 1); >- my $dbh=C4::Context->dbh; >- my $sth=$dbh->prepare("delete from z3950servers where name=?"); >- $sth->execute($searchfield); >- $sth->finish; >+ my $server = C4::Breeding::getZ3950server({name => $searchfield}); >+ C4::Breeding::deleteZ3950server($server->{id}); > # END $OP eq DELETE_CONFIRMED > ################## DEFAULT ################################## > } else { # DEFAULT >@@ -219,3 +179,23 @@ if ( $op eq 'add_form' ) { > } > } #---- END $OP eq DEFAULT > output_html_with_http_headers $input, $cookie, $template->output; >+ >+sub castCGIToZ3950serverParams { >+ my ($cgi) = @_; >+ >+ my $params = {}; >+ $params->{'host'} = $input->param('host'), >+ $params->{'port'} = $input->param('port'), >+ $params->{'db'} = $input->param('db'), >+ $params->{'userid'} = $input->param('userid'), >+ $params->{'password'} = $input->param('password'), >+ $params->{'name'} = $input->param('searchfield'), >+ $params->{'checked'} = $input->param('checked') ? 1 : 0, >+ $params->{'rank'} = $input->param('rank'), >+ $params->{'syntax'} = $input->param('syntax'), >+ $params->{'encoding'} = $input->param('encoding'), >+ $params->{'timeout'} = $input->param('timeout'), >+ $params->{'recordtype'} = $input->param('recordtype'), >+ $params->{'searchfield'} = $input->param('searchfield'), >+ return $params; >+} >\ No newline at end of file >-- >1.9.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 15875
: 48268