jsonmapper

jsonmapper copied to clipboard

The default setting is to be strict about NULL values if they are not allowed:

    /**
     * Throw an exception, if null value is found
     * but the type of attribute does not allow nulls.
     *
     * @var bool
     */
    public $bStrictNullTypes = true;

However there is one explicit test case in ArrayTest.php that verifies that NULL inside a JSON array of DateTime is translated to NULL in the final array:

    /**
     * Test for an array of classes "@var ClassName[]" with
     * flat/simple json values (string)
     */
    public function testMapTypedSimpleArray()
    {
        $jm = new JsonMapper();
        $sn = $jm->map(
            json_decode('{"typedSimpleArray":["2014-01-02",null,"2014-05-07"]}'),
            new JsonMapperTest_Array()
        );
        $this->assertIsArray($sn->typedSimpleArray);
        $this->assertCount(3, $sn->typedSimpleArray);
        $this->assertInstanceOf('DateTime', $sn->typedSimpleArray[0]);
        $this->assertNull($sn->typedSimpleArray[1]);
        $this->assertInstanceOf('DateTime', $sn->typedSimpleArray[2]);
        $this->assertSame(
            '2014-01-02', $sn->typedSimpleArray[0]->format('Y-m-d')
        );
        $this->assertSame(
            '2014-05-07', $sn->typedSimpleArray[2]->format('Y-m-d')
        );
    }

Every other test I have seen cares about null values to be either explicitly allowed, or the values are transformed into simple type values that match the required type:

    /**
     * Test for an array of strings - "@var string[]"
     */
    public function testStrArray()
    {
        $jm = new JsonMapper();
        $sn = $jm->map(
            json_decode('{"strArray":["str",false,2.048]}'),
            new JsonMapperTest_Array()
        );
        $this->assertSame(["str", "", "2.048"], $sn->strArray);
    }

One could argue that in the first case, the task is to keep the number of elements, and a side quest is to wrap the strings into DateTime objects if possible, and NULL would not be transferable into a useful DateTime object.

I have not seen another test verifying that non-wrapping objects like JsonMapperTest_Simple are in fact allowing or denying NULL. If I extend the very first test in ArrayTest.php, the added NULL is put into the resulting array just the same:

    /**
     * Test for an array of classes "@var Classname[]"
     */
    public function testMapTypedArray()
    {
        $jm = new JsonMapper();
        $sn = $jm->map(
            json_decode('{"typedArray":[{"str":"stringvalue"},{"fl":"1.2"}, null]}'), # ADDED NULL HERE
            new JsonMapperTest_Array()
        );
        $this->assertIsArray($sn->typedArray);
        $this->assertCount(3, $sn->typedArray); # GET ONE ELEMENT MORE BACK
        $this->assertContainsOnlyInstancesOf(JsonMapperTest_Simple::class, $sn->typedArray); # THAT IS NULL, not the class.
        $this->assertSame('stringvalue', $sn->typedArray[0]->str);
        $this->assertSame(1.2, $sn->typedArray[1]->fl);
    }

It looks like this is likely the reason for the influx in recent "complaints" about array elements being of unexpected type. And I think no matter how we'd fix it, it is a change in behavior that warrants a major version release.

Any insights from anyone?

Referencing #224 here for the same major release.

From what I recall, arrays don't check bStrictNullTypes, which is why I had a problem. @cweiske previously said here that guarding array elements behind this var would require a new major version. Possibly adding an extra bStrictNullTypesInArrays would allow avoiding a BC break, but it seems clunky.

Array elements are generally not subject to the same level of scrutiny as object properties from my experience - see also #225

I'd happily accept a patch that extends the bStrictNullTypes option to null values in array - when a new option is added to get the old behavior back (disabled by default), just as described in #211. That way a new major version would be more secure, while software relying on the old behavior could simply activate the new option when updating JsonMapper.

Just to rephrase:

• bStrictNullTypes defaults to on and will create a breaking behaviour by rejecting array entries that are NULL.
• A new compatibility flag is added to retain the current behaviour of allowing NULL in arrays.

Let's see if I can get there.

Released with v5.0.0.