Skip to content

Depth handling: Adding errors to LazyAssertion from LazyAssertionException #348

@kafoso

Description

@kafoso

I have a suggestion for a feature and I intend to implement it myself and make a Pull request. However, I see that there is little activity in this repository with regards to Issues and existing Pull requests. Before diving into writing code, I'd appreciate a nudge whether it will stand a chance of every being looked at and merged.


The demand

Imagine you have several levels of things (arrays or objects) you want to validate. Think of JSON and objects within objects within objects... You get the idea. I do not want to validate the full data structure in the same PHP file. Let us, for better readability, stick with the concept of JSON deserializer classes (i.e. converting JSON data to PHP value-objects/records). Having all of this in a single JSON deserializer class will quickly make the file large (many lines of code), unreadable, and unmanageable. It is preferably having it split into several different deserializer classes.

Passing along the instance of Assert\LazyAssertion could work in some scenarios, but not in others. As soon as verifyNow is called, which it may sporadically be in child deserializers, the game is over. You can, of course, catch the exception, but there currently is no neat way of merging the errors from two independent lazy assertion chains.

Ultimately, what I want to achieve is: Go as deep as we can and report as many errors as we can at every stage. This will entail multiple independant and completely isolated instances of Assert\LazyAssertion; i.e. multiple calls to Assert\Assert::lazy().

The current state

Currently, in Assert\LazyAssertion, the $errors class property is private and unable to be interacted with from the outside.

The only place the $errors property is being assigned is here:

$this->errors[] = $e;

The only places the $errors property is being read are here:

if ($this->errors) {

throw \call_user_func([$this->exceptionClass, 'fromErrors'], $this->errors);

The suggested solution

I am not suggesting exposing the $errors class property to the outside. However, in Assert\LazyAssertion, I want to introduce a new method:

public function mergeLazyAssertionException(LazyAssertionException $lazyAssertionException): self
{
    foreach ($exception->getErrorExceptions() as $invalidArgumentException) {
        $this->errors[] = $invalidArgumentException;
    }

    return $this;
}

What this will enable us to do is the following:

use Assert\Assert;
use Assert\LazyAssertionException;

class FooDeserializer
{
    public function deserialize(mixed $data, string $propertyPath): int
    {
        $lazy = Assert::lazy();

        $lazy
            ->that($data, $propertyPath)
            ->integer();

        $lazy->verifyNow();

        return $data;
    }
}

class BarDeserializer
{
    public function __construct(private FooDeserializer $fooDeserializer)
    {
    }

    public function deserialize(mixed $data, string $propertyPath): int
    {
        /** @var int|null $deserialized */
        $deserialized = null;
        
        $lazy = Assert::lazy();

        $lazy
            ->that($data, $propertyPath)
            ->isArray()
            ->keyExists('bar');

        if (is_array($data) && array_key_exists('bar', $data)) {
            try {
                $deserialized = $this->fooDeserializer->deserialize(
                    $data['bar'],
                    sprintf(
                        '%s[%s]',
                        $propertyPath,
                        escapeshellarg('bar'),
                    ),
                );
            } catch (LazyAssertionException $e) {
                $lazy->mergeLazyAssertionException($e);
            }
        }

        $lazy->verifyNow();

        return $deserialized;
    }
}

Now, in the BarDeserializer, I will get everything from the FooDeserializer compiled together. This example alone is kept simple in order to clearly convey the message. However, a single try-catch, as shown in the above example, might be overkill for the given situtation. This is where we have to consider two things:

  1. We can have several try-catch blocks, wrapping calls to any number of sub-deserializers in the BarDeserializer class.
  2. The depth can be any number (a positive integer). We don't know how far the rabbit hole goes (custom implementations), but the beauty is: We don't have to care, because we now have a means of aggregating error messages, no matter which level in the tree the LazyAssertionException is thrown at.

Although I used JSON deserializers as the framework above, my suggested solution will apply in every kind of nesting scenario.


Do you want it? Yay/Nay

If this is a desirable feature, I'll get cracking on it and make a Pull request.

It may turn out that other logic besides the mergeLazyAssertionException method will be needed. I'll have to investigate as I implement this feature.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions