Bugzilla – Attachment 72916 Details for
Bug 20272
XSLT_Handler should use alphanumeric error codes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20272: Replace error numbers by codes in XSLT_Handler
Bug-20272-Replace-error-numbers-by-codes-in-XSLTHa.patch (text/plain), 13.23 KB, created by
Brendan Gallagher
on 2018-03-15 10:02:45 UTC
(
hide
)
Description:
Bug 20272: Replace error numbers by codes in XSLT_Handler
Filename:
MIME Type:
Creator:
Brendan Gallagher
Created:
2018-03-15 10:02:45 UTC
Size:
13.23 KB
patch
obsolete
>From 07c669bd5c304c596e79d9d8eb163c040fa8aaab Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Thu, 22 Feb 2018 14:52:29 +0100 >Subject: [PATCH] Bug 20272: Replace error numbers by codes in XSLT_Handler > >We remove the error numbers 1 to 7 by readable codes. >And remove the errstr attribute (not used widely). >Make XSLT_Handler a little bit less noisy by defaulting print_warns to >false unless $ENV{DEBUG} is set. (See also bug 19018). > >The unit has been changed accordingly. >(A few warnings are no longer tested.) > >Test plan: >Run t/db_dependent/XSLT_Handler.t > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> > >Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> >--- > Koha/XSLT_Handler.pm | 105 ++++++++++++++++-------------------------- > t/db_dependent/XSLT_Handler.t | 60 +++++++++++------------- > 2 files changed, 65 insertions(+), 100 deletions(-) > >diff --git a/Koha/XSLT_Handler.pm b/Koha/XSLT_Handler.pm >index 7541afa..0682c3c 100644 >--- a/Koha/XSLT_Handler.pm >+++ b/Koha/XSLT_Handler.pm >@@ -28,8 +28,7 @@ Koha::XSLT_Handler - Facilitate use of XSLT transformations > my $output = $xslt_engine->transform($xml, $xsltfilename); > $output = $xslt_engine->transform({ xml => $xml, file => $file }); > $output = $xslt_engine->transform({ xml => $xml, code => $code }); >- my $err= $xslt_engine->err; # error number >- my $errstr= $xslt_engine->errstr; # error message >+ my $err= $xslt_engine->err; # error code > $xslt_engine->refresh($xsltfilename); > > =head1 DESCRIPTION >@@ -37,7 +36,7 @@ Koha::XSLT_Handler - Facilitate use of XSLT transformations > A XSLT handler object on top of LibXML and LibXSLT, allowing you to > run XSLT stylesheets repeatedly without loading them again. > Errors occurring during loading, parsing or transforming are reported >- via the err and errstr attributes. >+ via the err attribute. > Reloading XSLT files can be done with the refresh method. > > =head1 METHODS >@@ -58,11 +57,7 @@ Koha::XSLT_Handler - Facilitate use of XSLT transformations > > =head2 err > >- Error number (see list of ERROR CODES) >- >-=head2 errstr >- >- Error message >+ Error code (see list of ERROR CODES) > > =head2 do_not_return_source > >@@ -71,35 +66,36 @@ Koha::XSLT_Handler - Facilitate use of XSLT transformations > > =head2 print_warns > >- If set, print error messages to STDERR. True by default. >+ If set, print error messages to STDERR. False by default. Looks at the >+ DEBUG environment variable too. > > =head1 ERROR CODES > >-=head2 Error 1 >+=head2 Error XSLTH_ERR_NO_FILE > > No XSLT file passed > >-=head2 Error 2 >+=head2 Error XSLTH_ERR_FILE_NOT_FOUND > > XSLT file not found > >-=head2 Error 3 >+=head2 Error XSLTH_ERR_LOADING > >- Error while loading stylesheet xml: [furter information] >+ Error while loading stylesheet xml: [optional warnings] > >-=head2 Error 4 >+=head2 Error XSLTH_ERR_PARSING_CODE > >- Error while parsing stylesheet: [furter information] >+ Error while parsing stylesheet: [optional warnings] > >-=head2 Error 5 >+=head2 Error XSLTH_ERR_PARSING_DATA > >- Error while parsing input: [furter information] >+ Error while parsing input: [optional warnings] > >-=head2 Error 6 >+=head2 Error XSLTH_ERR_TRANSFORMING > >- Error while transforming input: [furter information] >+ Error while transforming input: [optional warnings] > >-=head2 Error 7 >+=head2 Error XSLTH_NO_STRING_PASSED > > No string to transform > >@@ -125,9 +121,17 @@ use XML::LibXSLT; > > use base qw(Class::Accessor); > >-__PACKAGE__->mk_ro_accessors(qw( err errstr )); >+__PACKAGE__->mk_ro_accessors(qw( err )); > __PACKAGE__->mk_accessors(qw( do_not_return_source print_warns )); > >+use constant XSLTH_ERR_1 => 'XSLTH_ERR_NO_FILE'; >+use constant XSLTH_ERR_2 => 'XSLTH_ERR_FILE_NOT_FOUND'; >+use constant XSLTH_ERR_3 => 'XSLTH_ERR_LOADING'; >+use constant XSLTH_ERR_4 => 'XSLTH_ERR_PARSING_CODE'; >+use constant XSLTH_ERR_5 => 'XSLTH_ERR_PARSING_DATA'; >+use constant XSLTH_ERR_6 => 'XSLTH_ERR_TRANSFORMING'; >+use constant XSLTH_ERR_7 => 'XSLTH_NO_STRING_PASSED'; >+ > =head2 transform > > my $output= $xslt_engine->transform( $xml, $xsltfilename, [$format] ); >@@ -180,7 +184,7 @@ sub transform { > > #check if no string passed > if ( !defined $xml ) { >- $self->_set_error(7); >+ $self->_set_error( XSLTH_ERR_7 ); > return; #always undef > } > >@@ -193,7 +197,7 @@ sub transform { > my $parser = XML::LibXML->new(); > my $source = eval { $parser->parse_string($xml) }; > if ($@) { >- $self->_set_error( 5, $@ ); >+ $self->_set_error( XSLTH_ERR_5, $@ ); > return $retval; > } > my $result = eval { >@@ -213,7 +217,7 @@ sub transform { > : $stsh->output_as_chars( $transformed ); # default: chars > }; > if ($@) { >- $self->_set_error( 6, $@ ); >+ $self->_set_error( XSLTH_ERR_6, $@ ); > return $retval; > } > $self->{last_xsltfile} = $key; >@@ -259,8 +263,10 @@ sub _init { > my $self = shift; > > $self->_set_error; >- $self->{xslt_hash} = {}; >- $self->{print_warns} = 1 unless exists $self->{print_warns}; >+ $self->{xslt_hash} = {}; >+ $self->{print_warns} = exists $self->{print_warns} >+ ? $self->{print_warns} >+ : $ENV{DEBUG} // 0; > $self->{do_not_return_source} = 0 > unless exists $self->{do_not_return_source}; > >@@ -281,7 +287,7 @@ sub _load { > if ( !$filename && !$code ) { > my $last = $self->{last_xsltfile}; > if ( !$last || !exists $self->{xslt_hash}->{$last} ) { >- $self->_set_error(1); >+ $self->_set_error( XSLTH_ERR_1 ); > return; > } > return $last; >@@ -300,7 +306,7 @@ sub _load { > > #Check file existence (skipping URLs) > if( $filename && $filename !~ /^https?:\/\// && !-e $filename ) { >- $self->_set_error(2); >+ $self->_set_error( XSLTH_ERR_2 ); > return; > } > >@@ -310,7 +316,7 @@ sub _load { > $parser->load_xml( $self->_load_xml_args($filename, $code) ) > }; > if ($@) { >- $self->_set_error( 3, $@ ); >+ $self->_set_error( XSLTH_ERR_3, $@ ); > return; > } > >@@ -319,7 +325,7 @@ sub _load { > $rv = $code? $digest.$codelen: $filename; > $self->{xslt_hash}->{$rv} = eval { $xslt->parse_stylesheet($style_doc) }; > if ($@) { >- $self->_set_error( 4, $@ ); >+ $self->_set_error( XSLTH_ERR_4, $@ ); > delete $self->{xslt_hash}->{$rv}; > return; > } >@@ -335,43 +341,10 @@ sub _load_xml_args { > # Internal routine for handling error information. > > sub _set_error { >- my ( $self, $errno, $addmsg ) = @_; >+ my ( $self, $errcode, $warn ) = @_; > >- if ( !$errno ) { #clear the error >- $self->{err} = undef; >- $self->{errstr} = undef; >- return; >- } >- >- $self->{err} = $errno; >- if ( $errno == 1 ) { >- $self->{errstr} = "No XSLT file passed."; >- } >- elsif ( $errno == 2 ) { >- $self->{errstr} = "XSLT file not found."; >- } >- elsif ( $errno == 3 ) { >- $self->{errstr} = "Error while loading stylesheet xml:"; >- } >- elsif ( $errno == 4 ) { >- $self->{errstr} = "Error while parsing stylesheet:"; >- } >- elsif ( $errno == 5 ) { >- $self->{errstr} = "Error while parsing input:"; >- } >- elsif ( $errno == 6 ) { >- $self->{errstr} = "Error while transforming input:"; >- } >- elsif ( $errno == 7 ) { >- $self->{errstr} = "No string to transform."; >- } >- >- if ($addmsg) { >- $self->{errstr} .= " $addmsg"; >- } >- >- warn $self->{errstr} if $self->{print_warns}; >- return; >+ $self->{err} = $errcode; #set or clear error >+ warn 'XSLT_Handler: '. $warn if $warn && $self->{print_warns}; > } > > =head1 AUTHOR >diff --git a/t/db_dependent/XSLT_Handler.t b/t/db_dependent/XSLT_Handler.t >index 8d6bc5c..349d637 100644 >--- a/t/db_dependent/XSLT_Handler.t >+++ b/t/db_dependent/XSLT_Handler.t >@@ -21,7 +21,7 @@ use Modern::Perl; > > use FindBin; > use File::Slurp; >-use Test::More tests => 41; >+use Test::More tests => 35; > use Test::Warn; > > use Koha::XSLT_Handler; >@@ -29,15 +29,11 @@ use Koha::XSLT_Handler; > my $engine=Koha::XSLT_Handler->new; > is( ref $engine, 'Koha::XSLT_Handler', 'Testing creation of handler object' ); > >-warning_is { $engine->transform('') } #we passed no file at first time >- "No XSLT file passed.", >- "No XSLT warning correctly displayed"; >-is( $engine->err, 1, 'Engine returns error on no file' ); >+$engine->transform(''); >+is( $engine->err, Koha::XSLT_Handler::XSLTH_ERR_1, 'Engine returns error on no file' ); > >-warning_is { $engine->transform( '', 'thisfileshouldnotexist.%$#@' ) } >- "XSLT file not found.", >- "No XSLT warning correctly displayed"; >-is( $engine->err, 2, 'Engine returns error on bad file' ); >+$engine->transform( '', 'thisfileshouldnotexist.%$#@' ); >+is( $engine->err, Koha::XSLT_Handler::XSLTH_ERR_2, 'Engine returns error on bad file' ); > is( $engine->refresh( 'asdjhaskjh'), 0, 'Test on invalid refresh' ); > > #check first test xsl >@@ -51,28 +47,25 @@ $xsltfile_1= $path.$xsltfile_1; > my $output; > > # Undefined text tests >-warning_is { $output = $engine->transform( undef, $xsltfile_1 ) } >- "No string to transform.", >- "No string warning correctly displayed"; >-is( $engine->err, 7, 'Engine returns error on undefined text' ); >+$output = $engine->transform( undef, $xsltfile_1 ); >+is( $engine->err, Koha::XSLT_Handler::XSLTH_ERR_7, 'Engine returns error on undefined text' ); > > # Empty string tests >-warning_is { $output = $engine->transform( '', $xsltfile_1 ) } >- "Error while parsing input: Empty String", >- "Empty string warning correctly displayed"; >-is( $engine->err, 5, 'Engine returns error on empty string' ); >+$output = $engine->transform( '', $xsltfile_1 ); >+is( $engine->err, Koha::XSLT_Handler::XSLTH_ERR_5, 'Engine returns error on empty string' ); > > # Non-XML tests >+$engine->print_warns(1); > warning_like { $output = $engine->transform( 'abcdef', $xsltfile_1 ) } >- qr{^Error while parsing input: :1: parser error : Start tag expected, '<' not found}, >- "Non-XML warning correctly displayed"; >-is( $engine->err, 5, 'Engine returns error on non-xml' ); >+ qr{parser error : Start tag expected, '<' not found}, >+ "Non-XML warning correctly displayed"; >+is( $engine->err, Koha::XSLT_Handler::XSLTH_ERR_5, 'Engine returns error on non-xml' ); > > # Malformed XML tests > warning_like { $output = $engine->transform( '<a></b>', $xsltfile_1 ) } >- qr{^Error while parsing input: :1: parser error : Opening and ending tag mismatch: a line 1 and b}, >- "Malformed XML warning correctly displayed"; >-is( $engine->err, 5, 'Engine returns error on malformed xml' ); >+ qr{parser error : Opening and ending tag mismatch: a line 1 and b}, >+ "Malformed XML warning correctly displayed"; >+is( $engine->err, Koha::XSLT_Handler::XSLTH_ERR_5, 'Engine returns error on malformed xml' ); > > #Test not returning source on failure when asked for > #Include passing do_not_return via constructor on second engine >@@ -82,18 +75,19 @@ my $secondengine=Koha::XSLT_Handler->new( { > }); > $engine->do_not_return_source(1); > warning_like { $output = $engine->transform( '<a></b>', $xsltfile_1 ) } >- qr{^Error while parsing input: :1: parser error : Opening and ending tag mismatch: a line 1 and b}, >- "Malformed XML warning correctly displayed"; >+ qr{parser error : Opening and ending tag mismatch: a line 1 and b}, >+ "Malformed XML warning correctly displayed"; > is( defined $output? 1: 0, 0, 'Engine respects do_not_return_source==1'); >+$secondengine->print_warns(1); > warning_like { $output = $secondengine->transform( '<a></b>', $xsltfile_1 ) } >- qr{^Error while parsing input: :1: parser error : Opening and ending tag mismatch: a line 1 and b}, >- "Malformed XML warning correctly displayed"; >+ qr{parser error : Opening and ending tag mismatch: a line 1 and b}, >+ "Malformed XML warning correctly displayed"; > is( defined $output? 1: 0, 0, 'Second engine respects it too'); > undef $secondengine; #bye > $engine->do_not_return_source(0); > warning_like { $output = $engine->transform( '<a></b>', $xsltfile_1 ) } >- qr{^Error while parsing input: :1: parser error : Opening and ending tag mismatch: a line 1 and b}, >- "Malformed XML warning correctly displayed"; >+ qr{parser error : Opening and ending tag mismatch: a line 1 and b}, >+ "Malformed XML warning correctly displayed"; > is( defined $output? 1: 0, 1, 'Engine respects do_not_return_source==0'); > > #Testing valid refresh now >@@ -146,11 +140,9 @@ is( -e $path.$xsltfile_2, 1, "Found my test stylesheet $xsltfile_2" ); > exit if !-e $path.$xsltfile_2; > $xsltfile_2= $path.$xsltfile_2; > >-warning_like { $output = $engine->transform( $xml_2, $xsltfile_2 ) } >- qr{^Error while parsing stylesheet:}, >- "Bad XSL warning correctly displayed"; >-is( $engine->err, 4, 'Engine returned error for parsing bad xsl' ); >-is( defined($engine->errstr), 1, 'Error string contains text'); >+$engine->print_warns(0); >+$output = $engine->transform( $xml_2, $xsltfile_2 ); >+is( $engine->err, Koha::XSLT_Handler::XSLTH_ERR_4, 'Engine returned error for parsing bad xsl' ); > > #The third test xsl is okay again; main use is clearing two items from cache > my $xsltfile_3 = 'test03.xsl'; >-- >2.1.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=72916">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20272
:
72205
|
72206
|
72916
|
72917
|
75582
|
75583