JSON::XS 3.0 changes the way booleans are represented

Bug #1257264 reported by Jeff Godin
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenSRF
Fix Released
Medium
Unassigned
2.1
Fix Committed
Medium
Unassigned
2.2
Fix Released
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

Tags: pullrequest
Revision history for this message
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.

Revision history for this message
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?

Revision history for this message
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.

Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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)
Changed in opensrf:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.