JSON::XS 3.0 changes the way booleans are represented

Bug #1257264 reported by Jeff Godin on 2013-12-03
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenSRF
Medium
Unassigned
2.1
Medium
Unassigned
2.2
Medium
Unassigned

Bug Description

When using JSON::XS to convert objects back and forth between perl and JSON, we look for booleans by nature of them being JSON::XS::Boolean objects.

Starting with JSON::XS 3.0, JSON::XS::Boolean has been replaced with JSON::PP::Boolean. From the JSON::XS::Boolean docs:

"Since 3.0, JSON::PP::Boolean has replaced it. Support for JSON::XS::Boolean will be removed in a future release."

Actually, JSON::XS introduces a dependency on Types::Serialiser, and boolean values are represented as Types::Serialiser::Boolean, but according to the Types::Serialiser documentation:

"For historical reasons, the "Types::Serialiser::Boolean" stash is just an alias for "JSON::PP::Boolean". When printed, the classname with usually be "JSON::PP::Boolean", but isa tests and stash pointer comparison will normally work correctly (i.e. Types::Serialiser::true ISA JSON::PP::Boolean, but also ISA Types::Serialiser::Boolean)."

Given a working OpenSRF install, if you upgrade to JSON::XS 3.01 and restart services, you can see broken behavior in srfsh:

Normal operation:

srfsh# request open-ils.actor open-ils.actor.org_unit_setting.values.ranged.retrieve "AUTHTOKEN" "4"

Received Data: {
  "cat.label.font.family":"monospace",
  "cat.spine.line.width":8,
  "cat.label.font.size":10,
  "circ.booking_reservation.default_elbow_room":"1 day",
  "cat.spine.line.margin":0,
  "circ.hold_go_home_interval":"6 months",
  "cat.spine.line.height":9,
  "circ.grace.extend":true,
  "cat.label.font.weight":"normal"
}

Operation after upgrading to JSON::XS 3.01 (using "cpan JSON::XS") and restarting services:

srfsh# request open-ils.actor open-ils.actor.org_unit_setting.values.ranged.retrieve "5c774127b236abada4968345956b7b67" "4"

Received Data: {
  "cat.label.font.family":"monospace",
  "cat.spine.line.width":8,
  "cat.label.font.size":10,
  "circ.booking_reservation.default_elbow_room":"1 day",
  "cat.spine.line.margin":0,
  "circ.hold_go_home_interval":"6 months",
  "cat.spine.line.height":9,
  "circ.grace.extend":{
    "__c":"JSON::PP::Boolean",
    "__p":null
  },
  "cat.label.font.weight":"normal"
}

Note that the value for circ.grace.extend is no longer "true".

OpenSRF "make check" also fails after installing JSON::XS 3.01.

Two approaches for fixing this, both contained in their own mostly-unpolished but tested working branches. Feedback desired:

user/jeff/fix_json_xs_booleans_option1
http://git.evergreen-ils.org/?p=working/OpenSRF.git;a=shortlog;h=refs/heads/user/jeff/fix_json_xs_booleans_option1

user/jeff/fix_json_xs_booleans_option2
http://git.evergreen-ils.org/?p=working/OpenSRF.git;a=shortlog;h=refs/heads/user/jeff/fix_json_xs_booleans_option2

Jeff Godin (jgodin) wrote :

Thanks to Scott Myers for mentioning symptoms after upgrading JSON::XS on one of his systems.

This is probably not something that will affect most supported Linux distributions for an undetermined amount of time. JSON::XS 3.01 was released 2013-10-29 and is not yet packaged in the distros I checked.

If we adapt soon, we'll be ready when 3.x does start showing up in distro packages.

Mike Rylander (mrylander) wrote :

I've pushed a third option which avoids testing the class-ness directly, based on the JSON::XS (and JSON) documentation: http://git.evergreen-ils.org/?p=working/OpenSRF.git;a=shortlog;h=refs/heads/user/miker/json_bool_api

Thoughts?

Galen Charlton (gmc) wrote :

After doing some poking, I favor Mike's approach: it's the most concise, and it's portable to earlier versions of JSON::XS.

Galen Charlton (gmc) wrote :

I've put together a two-patch branch which incorporates Mike's patch (with a more descriptive commit message) and corrections to the automated tests:

http://git.evergreen-ils.org/?p=working/OpenSRF.git;a=shortlog;h=refs/heads/user/gmcharlt/lp1257264_fix_plus_test_cases

tags: added: pullrequest
Jeff Godin (jgodin) wrote :

Thanks, Mike. I like that approach better -- it seems more future-proof / less reliant on internals, and JSON::XS seems to have had is_bool since the 1.x days in 2007.

I've signed off on Galen's latest branch and incorporated an additional commit with the tests I had added in my not-yet-pushed branch today.

Running it through the paces now:

Debian Wheezy system alternating between the following two versions of JSON::XS:

* the packaged 2.320-1+b1 version of libjson-xs-perl
* after installing JSON::XS 3.01 via "cpan JSON:XS" (which also installs Types::Serialiser)

"make check" for OpenSRF succeeds for branch user/jeff/lp1257264_fix_plus_test_cases_signoff
"make check" for OpenSRF succeeds for rel_2_2 with cherry-picked commits user/jeff/lp1257264_fix_plus_test_cases_backport

Testing service startup

Testing with Evergreen 2.5.1 and the two above JSON::XS versions.

service startup, previously problematic srfsh query, and staff client login, retrieval of admin user by db id, visit user editor and user permission editor: success!

signed off branch: user/jeff/lp1257264_fix_plus_test_cases_signoff
backported (clean cherry pick) to rel_2_2: user/jeff/lp1257264_fix_plus_test_cases_backport

My latest commit lacks a signoff at this point, but other than that, I think we're looking good.

Galen Charlton (gmc) wrote :

Pushed Jeff's branch with the signoffs and the additional tests to master and rel_2_2. Thanks, everybody!

Changed in opensrf:
milestone: none → 2.3.0-beta
status: New → Fix Committed
importance: Undecided → Medium
Galen Charlton (gmc) on 2014-02-28
Changed in opensrf:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers