From 7a87c0c2f00403a1921ee4f1491578740632d594 Mon Sep 17 00:00:00 2001 From: bernhard Date: Thu, 26 Oct 2023 13:26:54 +0200 Subject: [PATCH] Issue #2295: revert to JSON::XS It was attempted to reproduce the JSON::XS behavior with Cpanel::JSON::XS. The option type_all_string looked promising for that. But using that option would require a version of Cpanel::JSON::XS that is not provided per default by the current LTS version. This also means that boolean can now be passed as real booleans to JavaScript. --- Kernel/Output/HTML/Layout/Template.pm | 14 +- Kernel/System/JSON.pm | 27 +-- bin/otobo.CheckModules.pl | 32 ++- cpanfile | 10 +- cpanfile.docker | 7 +- scripts/test/JSON.t | 201 ++++++++++--------- scripts/test/Layout/Template/AddJSDataAjax.t | 18 +- scripts/test/Layout/Template/Render.t | 2 +- 8 files changed, 168 insertions(+), 143 deletions(-) diff --git a/Kernel/Output/HTML/Layout/Template.pm b/Kernel/Output/HTML/Layout/Template.pm index fd2fd0f7f4..2698eb1d38 100644 --- a/Kernel/Output/HTML/Layout/Template.pm +++ b/Kernel/Output/HTML/Layout/Template.pm @@ -381,8 +381,7 @@ JavaScript via Core.Config. The truthiness of the passed value is evaluated according to Perl rules. Contrary to JavaScript, the string C<'0'> is considered to be false. -Actually no boolean values C and C are passed to JavaScript. The values C<"1"> and C<""> are used -to indicate truthiness instead. +Using this method ensures that the boolean values C and C are passed to JavaScript. $LayoutObject->AddJSBoolean( Key => 'KeyBool1', # the key to store this data @@ -394,12 +393,13 @@ to indicate truthiness instead. sub AddJSBoolean { my ( $Self, %Param ) = @_; - return unless $Param{Key}; - - $Self->{_JSData} //= {}; - $Self->{_JSData}->{ $Param{Key} } = $Param{Value} ? q{1} : q{}; + my $JSONObject = $Kernel::OM->Get('Kernel::System::JSON'); + my $Value = $Param{Value} ? $JSONObject->True : $JSONObject->False; - return; + return $Self->AddJSData( + Key => $Param{Key}, + Value => $Value, + ); } 1; diff --git a/Kernel/System/JSON.pm b/Kernel/System/JSON.pm index 514919dc45..4a1b3e8b74 100644 --- a/Kernel/System/JSON.pm +++ b/Kernel/System/JSON.pm @@ -25,7 +25,8 @@ use utf8; # core modules # CPAN modules -use Cpanel::JSON::XS; +use JSON::XS; +use Types::Serialiser; use Try::Tiny; # OTOBO modules @@ -36,7 +37,7 @@ our @ObjectDependencies = ( =head1 NAME -Kernel::System::JSON - JSON lib that wraps Cpanel::JSON::XS +Kernel::System::JSON - JSON lib that wraps JSON::XS =head1 DESCRIPTION @@ -73,7 +74,6 @@ The result will be Perl string that may have code points greater 255. Data => $Data, SortKeys => 1, # (optional) (0|1) default 0, to sort the keys of the json data Pretty => 1, # (optional) (0|1) default 0, to pretty print - TypeAllString => 1, # (optional) (0|1) default 0, to pass numbers in double quotes ); =cut @@ -92,7 +92,7 @@ sub Encode { } # create a JSON::XS compatible object - my $JSONObject = Cpanel::JSON::XS->new; + my $JSONObject = JSON::XS->new; # grudgingly accept data that is neither a hash- nor an array reference $JSONObject->allow_nonref(1); @@ -107,10 +107,13 @@ sub Encode { $JSONObject->pretty(1); } - # put numbers into double quotes so that the JS side can be sure about what it will receive - if ( $Param{TypeAllString} ) { - $JSONObject->type_all_string(1); - } + # Briefly the option TypeAllString was supported. The aim was + # to put numbers into double quotes so that the JS side can be sure about what it will receive. + # However the type_all_string attribute is only available in Cpanel::JSON::XS >= 4.18. So this + # feature can't be used in OTOBO and the option has been removed. + #if ( $Param{TypeAllString} ) { + # $JSONObject->type_all_string(1); + #} # Serialise the Perl data structure into the format JSON. # @@ -176,13 +179,13 @@ sub Decode { return unless defined $Param{Data}; # create a JSON::XS compatible object that does the actual parsing - my $JSONObject = Cpanel::JSON::XS->new; + my $JSONObject = JSON::XS->new; # grudgingly accept data that is neither a hash- nor an array reference $JSONObject->allow_nonref(1); # Deserialize JSON and get a Perl data structure. - # Use Try::Tiny as Cpanel::JSON::XS->decode() dies when providing a malformed JSON string. + # Use Try::Tiny as JSON::XS->decode() dies when providing a malformed JSON string. # In that case we want to return an empty list. my $Success = 1; my $Scalar = try { @@ -220,7 +223,7 @@ as a JSON string instead. =cut sub True { - return Cpanel::JSON::XS::true; + return Types::Serialiser::true; } =head2 False() @@ -237,7 +240,7 @@ This returns the String C. =cut sub False { - return Cpanel::JSON::XS::false; + return Types::Serialiser::false; } =head2 ToBoolean() diff --git a/bin/otobo.CheckModules.pl b/bin/otobo.CheckModules.pl index 2f4b332230..bca91393cb 100755 --- a/bin/otobo.CheckModules.pl +++ b/bin/otobo.CheckModules.pl @@ -319,15 +319,15 @@ =head1 DESCRIPTION }, }, { - Module => 'Cpanel::JSON::XS', + Module => 'JSON::XS', Required => 1, - Comment => 'correct and fast JSON support, used by Mojo::JSON', + Comment => 'JSON parsing and generation', InstTypes => { - aptget => 'libcpanel-json-xs-perl', - emerge => 'dev-perl/Cpanel-JSON-XS', - yum => 'perl-Cpanel-JSON-XS', - zypper => 'perl-Cpanel-JSON-XS', - ports => 'converters/p5-Cpanel-JSON-XS', + aptget => 'libjson-xs-perl', + emerge => 'dev-perl/JSON-XS', + yum => 'perl-JSON-XS', + zypper => 'perl-JSON-XS', + ports => 'converters/p5-JSON-XS', }, }, { @@ -629,6 +629,18 @@ =head1 DESCRIPTION ports => undef, }, }, + { + Module => 'Cpanel::JSON::XS', + Features => ['storage:s3'], + Comment => 'correct and fast JSON support, used by Mojo::JSON', + InstTypes => { + aptget => undef, + emerge => undef, + yum => undef, + zypper => undef, + ports => undef, + }, + }, { Module => 'Mojolicious::Plugin::AWS', Features => ['storage:s3'], @@ -656,9 +668,9 @@ =head1 DESCRIPTION Comment => q{This version can't be installed with the MariaDB client library.}, }, ], - Features => ['db:mysql'], - Comment => 'Required to connect to a MariaDB or MySQL database.', - InstTypes => { + Features => ['db:mysql'], + Comment => 'Required to connect to a MariaDB or MySQL database.', + InstTypes => { aptget => 'libdbd-mysql-perl', emerge => 'dev-perl/DBD-mysql', zypper => 'perl-DBD-mysql', diff --git a/cpanfile b/cpanfile index 0f94d69988..dc124bd08f 100644 --- a/cpanfile +++ b/cpanfile @@ -11,8 +11,8 @@ requires 'Archive::Zip'; # Neater interface for capturing STDOUT and STDERR. requires 'Capture::Tiny'; -# correct and fast JSON support, used by Mojo::JSON -requires 'Cpanel::JSON::XS'; +# JSON parsing and generation +requires 'JSON::XS'; requires 'Date::Format'; @@ -253,6 +253,9 @@ feature 'optional', 'Support for feature optional' => sub { # support for the REST requests to the S3 storage requires 'Mojolicious', ">= 9.22"; + # correct and fast JSON support, used by Mojo::JSON + requires 'Cpanel::JSON::XS'; + # support for S3 using Mojo::UserAgent requires 'Mojolicious::Plugin::AWS'; @@ -378,6 +381,9 @@ feature 'storage:s3', 'AWS S3 compatible storage' => sub { # support for the REST requests to the S3 storage requires 'Mojolicious', ">= 9.22"; + # correct and fast JSON support, used by Mojo::JSON + requires 'Cpanel::JSON::XS'; + # support for S3 using Mojo::UserAgent requires 'Mojolicious::Plugin::AWS'; diff --git a/cpanfile.docker b/cpanfile.docker index 094ca26e52..c2ec2f93d2 100644 --- a/cpanfile.docker +++ b/cpanfile.docker @@ -11,8 +11,8 @@ requires 'Archive::Zip'; # Neater interface for capturing STDOUT and STDERR. requires 'Capture::Tiny'; -# correct and fast JSON support, used by Mojo::JSON -requires 'Cpanel::JSON::XS'; +# JSON parsing and generation +requires 'JSON::XS'; requires 'Date::Format'; @@ -250,6 +250,9 @@ requires 'Const::Fast'; # support for the REST requests to the S3 storage requires 'Mojolicious', ">= 9.22"; + # correct and fast JSON support, used by Mojo::JSON + requires 'Cpanel::JSON::XS'; + # support for S3 using Mojo::UserAgent requires 'Mojolicious::Plugin::AWS'; diff --git a/scripts/test/JSON.t b/scripts/test/JSON.t index c554a154e0..90fef253e3 100644 --- a/scripts/test/JSON.t +++ b/scripts/test/JSON.t @@ -108,12 +108,12 @@ my @EncodeTests = ( }, { Input => "$Twelve asdf" + 0, - Result => '12.0', # TODO: why the .0 ? + Result => '12', Name => 'JSON - "$Twelve asdf" numified by adding zero', }, { Input => "asdf" + 6, - Result => '6.0', # TODO: why the .0 ? + Result => '6', Name => 'JSON - non-numeral string plus six', }, { @@ -123,106 +123,107 @@ my @EncodeTests = ( }, { Input => "$Twelve asdf" * 1, - Result => '12.0', # TODO: why the .0 ? + Result => '12', Name => 'JSON - "$Twelve asdf" numified by multiplying with one', }, # TypeAllString - { - Input => -12, - Result => '"-12"', - Name => 'JSON - TypeAllString with -12', - Params => { - TypeAllString => 1, - }, - }, - { - Input => 12, - Result => '"12"', - Name => 'JSON - TypeAllString with 12', - Params => { - TypeAllString => 1, - }, - }, - { - Input => +12, - Result => '"12"', - Name => 'JSON - TypeAllString with +12', - Params => { - TypeAllString => 1, - }, - }, - { - Input => 0, - Result => '"0"', - Name => 'JSON - TypeAllString with number zero', - Params => { - TypeAllString => 1, - }, - }, - { - Input => "0", - Result => '"0"', - Name => 'JSON - TypeAllString with string containing the digit zero', - Params => { - TypeAllString => 1, - }, - }, - { - Input => "Çe pa un niméro", - Result => '"Çe pa un niméro"', - Name => 'JSON - TypeAllString with Kouri-Vini', - Params => { - TypeAllString => 1, - }, - }, - { - Input => { - AAA => "Çe pa un niméro", - BBB => 0, - CCC => "0", - DDD => -12, - EEE => "-12", - FFF => [ "Çe pa un niméro", 0, "0", -12, "-12" ], - }, - Result => <<'END_JSON', -{ - "AAA" : "Çe pa un niméro", - "BBB" : "0", - "CCC" : "0", - "DDD" : "-12", - "EEE" : "-12", - "FFF" : [ - "Çe pa un niméro", - "0", - "0", - "-12", - "-12" - ] -} -END_JSON - Name => 'JSON - TypeAllString with nested data', - Params => { - Pretty => 1, - TypeAllString => 1, - }, - }, - { - Input => $JSONObject->True(), - Result => '"true"', - Name => q{JSON - TypeAllString bool true, don't do this in production}, - Params => { - TypeAllString => 1, - }, - }, - { - Input => $JSONObject->False(), - Result => '"false"', - Name => q{JSON - TypeAllString bool false, don't do this in production}, - Params => { - TypeAllString => 1, - }, - }, + # These tests were meant for Cpanel::JSON::XS + # { + # Input => -12, + # Result => '"-12"', + # Name => 'JSON - TypeAllString with -12', + # Params => { + # TypeAllString => 1, + # }, + # }, + # { + # Input => 12, + # Result => '"12"', + # Name => 'JSON - TypeAllString with 12', + # Params => { + # TypeAllString => 1, + # }, + # }, + # { + # Input => +12, + # Result => '"12"', + # Name => 'JSON - TypeAllString with +12', + # Params => { + # TypeAllString => 1, + # }, + # }, + # { + # Input => 0, + # Result => '"0"', + # Name => 'JSON - TypeAllString with number zero', + # Params => { + # TypeAllString => 1, + # }, + # }, + # { + # Input => "0", + # Result => '"0"', + # Name => 'JSON - TypeAllString with string containing the digit zero', + # Params => { + # TypeAllString => 1, + # }, + # }, + # { + # Input => "Çe pa un niméro", + # Result => '"Çe pa un niméro"', + # Name => 'JSON - TypeAllString with Kouri-Vini', + # Params => { + # TypeAllString => 1, + # }, + # }, + # { + # Input => { + # AAA => "Çe pa un niméro", + # BBB => 0, + # CCC => "0", + # DDD => -12, + # EEE => "-12", + # FFF => [ "Çe pa un niméro", 0, "0", -12, "-12" ], + # }, + # Result => <<'END_JSON', + #{ + # "AAA" : "Çe pa un niméro", + # "BBB" : "0", + # "CCC" : "0", + # "DDD" : "-12", + # "EEE" : "-12", + # "FFF" : [ + # "Çe pa un niméro", + # "0", + # "0", + # "-12", + # "-12" + # ] + #} + #END_JSON + # Name => 'JSON - TypeAllString with nested data', + # Params => { + # Pretty => 1, + # TypeAllString => 1, + # }, + # }, + # { + # Input => $JSONObject->True(), + # Result => '"true"', + # Name => q{JSON - TypeAllString bool true, don't do this in production}, + # Params => { + # TypeAllString => 1, + # }, + # }, + # { + # Input => $JSONObject->False(), + # Result => '"false"', + # Name => q{JSON - TypeAllString bool false, don't do this in production}, + # Params => { + # TypeAllString => 1, + # }, + # }, # more about zero { @@ -232,12 +233,12 @@ END_JSON }, { Input => 0.000, - Result => q{0.0}, + Result => q{0}, # not obvious why there is no fractional part Name => 'JSON - float zero' }, { Input => -0.000, - Result => q{-0.0}, + Result => q{-0}, # not obvious why there is no fractional part Name => 'JSON - negative float zero' }, { diff --git a/scripts/test/Layout/Template/AddJSDataAjax.t b/scripts/test/Layout/Template/AddJSDataAjax.t index 797d74f7c9..7500ee1d50 100644 --- a/scripts/test/Layout/Template/AddJSDataAjax.t +++ b/scripts/test/Layout/Template/AddJSDataAjax.t @@ -67,7 +67,7 @@ END_HTML END_HTML }, @@ -81,7 +81,7 @@ END_HTML END_HTML }, @@ -95,7 +95,7 @@ END_HTML END_HTML }, @@ -109,7 +109,7 @@ END_HTML END_HTML }, @@ -123,7 +123,7 @@ END_HTML END_HTML }, @@ -137,7 +137,7 @@ END_HTML END_HTML }, @@ -151,7 +151,7 @@ END_HTML END_HTML }, @@ -165,7 +165,7 @@ END_HTML END_HTML }, @@ -179,7 +179,7 @@ END_HTML END_HTML }, diff --git a/scripts/test/Layout/Template/Render.t b/scripts/test/Layout/Template/Render.t index 87abed26c8..bf73f02fcb 100644 --- a/scripts/test/Layout/Template/Render.t +++ b/scripts/test/Layout/Template/Render.t @@ -398,7 +398,7 @@ console.log(22); Template => ' [% PROCESS "JSDataInsert" -%]', Result => ' -Core.Config.AddConfig({"Config.Test":"123","Config.Test2":["1","2",{"test":"test"}],"JS.String":{"String":"<\/script><\/script>"},"JS.String.CaseInsensitive":{"String":"<\/ScRiPt><\/ScRiPt>"},"Perl.Code":{"Perl":"Data"}}); +Core.Config.AddConfig({"Config.Test":123,"Config.Test2":[1,2,{"test":"test"}],"JS.String":{"String":"<\/script><\/script>"},"JS.String.CaseInsensitive":{"String":"<\/ScRiPt><\/ScRiPt>"},"Perl.Code":{"Perl":"Data"}}); ', }, {