Skip to content

Commit

Permalink
Issue #2295: revert to JSON::XS
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bschmalhofer committed Oct 26, 2023
1 parent 5a147c6 commit 7a87c0c
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 143 deletions.
14 changes: 7 additions & 7 deletions Kernel/Output/HTML/Layout/Template.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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<true> and C<false> 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<true> and C<false> are passed to JavaScript.
$LayoutObject->AddJSBoolean(
Key => 'KeyBool1', # the key to store this data
Expand All @@ -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;
27 changes: 15 additions & 12 deletions Kernel/System/JSON.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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.
#
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -220,7 +223,7 @@ as a JSON string instead.
=cut

sub True {
return Cpanel::JSON::XS::true;
return Types::Serialiser::true;
}

=head2 False()
Expand All @@ -237,7 +240,7 @@ This returns the String C<q{false}>.
=cut

sub False {
return Cpanel::JSON::XS::false;
return Types::Serialiser::false;
}

=head2 ToBoolean()
Expand Down
32 changes: 22 additions & 10 deletions bin/otobo.CheckModules.pl
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
{
Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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',
Expand Down
10 changes: 8 additions & 2 deletions cpanfile
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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';

Expand Down Expand Up @@ -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';

Expand Down
7 changes: 5 additions & 2 deletions cpanfile.docker
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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';

Expand Down
Loading

0 comments on commit 7a87c0c

Please sign in to comment.