Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expect::structure() and normalization #9

Closed
pavel-barton opened this issue Aug 23, 2019 · 1 comment
Closed

Expect::structure() and normalization #9

pavel-barton opened this issue Aug 23, 2019 · 1 comment

Comments

@pavel-barton
Copy link

@pavel-barton pavel-barton commented Aug 23, 2019

Version: 1.0.0

Bug Description

We encountered a strange behavior when using normalization with Expect::structure(). It seemed that when specifying a before() method on a property inside the structure and passing an object to the Nette\Schema\Processor::process() method, it would not get normalized.

I managed to track it down to the Nette\Schema\Elements\Structure::normalize() method, specifically the is_array method in the condition, which was indeed the case:

public function normalize($value, Context $context)
{
    $value = $this->doNormalize($value, $context);
    if (is_array($value)) {
        foreach ($value as $key => $val) {
            $itemSchema = $this->items[$key] ?? $this->otherItems;
            if ($itemSchema) {
                $context->path[] = $key;
                $value[$key] = $itemSchema->normalize($val, $context);
                array_pop($context->path);
            }
        }
    }
    return $value;
}

Steps To Reproduce

This code snippet reproduces the problem:

use Nette\Schema\Expect;
use Nette\Schema\Processor;
use Nette\Utils\ArrayHash;

// The example from https://doc.nette.org/en/3.0/schema#toc-custom-normalization, just wrapped it in a structure
$schema = Expect::structure([
    'data' => Expect::arrayOf('string')->before(function ($v) { return explode(' ', $v); })
]);

$values = ['data' => 'a b c'];
$processor = new Processor();

// Simple array
// stdClass { data => [ 'a', 'b', 'c' ] }
$arrayResult = $processor->process($schema, $values);

// Non-iterable class
// Nette\Schema\ValidationException: The option 'data' expects to be array, string 'a b c' given
$objectResult = $processor->process($schema, (object) $values);

// An ArrayHash, or any class that implements \Traversable
// Nette\Schema\ValidationException: The option 'data' expects to be array, string 'a b c' given
$traversableResult = $processor->process($schema, ArrayHash::from($values));

Expected Behavior

If the structure is an object, it's properties get properly normalized.

Possible Solution

I tried my best with rewriting the Nette\Schema\Elements\Structure::normalize() method to support objects, and I came up with this:

public function normalize($value, Context $context)
{
    $value = $this->doNormalize($value, $context);
    if (is_array($value) || is_object($value)) {
        // When non-iterable object is received, iterate through its public properties
        $properties = is_iterable($value) ? $value : get_object_vars($value);

        foreach ($properties as $key => $val) {
            $itemSchema = $this->items[$key] ?? $this->otherItems;
            if ($itemSchema) {
                $context->path[] = $key;

                if (is_object($value)) {
                    $value->{$key} = $itemSchema->normalize($val, $context);
                } else {
                    $value[$key] = $itemSchema->normalize($val, $context);
                }

                array_pop($context->path);
            }
        }
    }
    return $value;
}

With this modification, all of the examples I showed in Steps to Reproduce section work as expected:

use Nette\Schema\Expect;
use Nette\Schema\Processor;
use Nette\Utils\ArrayHash;

$schema = Expect::structure([
    'data' => Expect::arrayOf('string')->before(function ($v) { return explode(' ', $v); })
]);

$values = ['data' => 'a b c'];
$processor = new Processor();

// Simple array
$arrayResult = $processor->process($schema, $values);

// Non-iterable class
$objectResult = $processor->process($schema, (object) $values);

// An ArrayHash, or any class that implements \Traversable
$traversableResult = $processor->process($schema, ArrayHash::from($values));


dump($arrayResult);          // stdClass { data => [ 'a', 'b', 'c' ] }
dump($objectResult);         // stdClass { data => [ 'a', 'b', 'c' ] }
dump($traversableResult);    // stdClass { data => [ 'a', 'b', 'c' ] }

Edit: I created a PR in case the solution would be acceptable.

@dg

This comment has been minimized.

Copy link
Member

@dg dg commented Sep 15, 2019

Thanks for the perfect problem description and PR!

I fixed support for non-traversable objects like stdClass. The schema is not suitable for traversable objects IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.