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

Using ValueDoesNotMatch does not add an error #73

Open
mrceperka opened this issue Nov 21, 2024 · 1 comment
Open

Using ValueDoesNotMatch does not add an error #73

mrceperka opened this issue Nov 21, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@mrceperka
Copy link
Contributor

mrceperka commented Nov 21, 2024

Library version

0.1.0, 0.2.0

Description

When using throw ValueDoesNotMatch, resulting InvalidData exception contains empty errors.

Steps to reproduce

Input class

<?php declare(strict_types = 1);


use LogicException;
use Nette\Utils\Json;
use Orisai\ObjectMapper\Callbacks\After;
use Orisai\ObjectMapper\Context\FieldContext;
use Orisai\ObjectMapper\Exception\ValueDoesNotMatch;
use Orisai\ObjectMapper\MappedObject;
use Orisai\ObjectMapper\Processing\Value;
use Orisai\ObjectMapper\Rules\AnyOf;
use Orisai\ObjectMapper\Rules\MappedObjectValue;
use Orisai\ObjectMapper\Rules\NullValue;
use Orisai\ObjectMapper\Rules\StringValue;
use Throwable;
use function is_array;

class Input implements MappedObject
{

	#[AnyOf([new StringValue(), new NullValue()])]
	#[After('afterFieldSettings')]
	public array|null $settings = null;

	protected static function afterFieldSettings(string $value, FieldContext $context): array
	{
		$json = [];

		if ($value !== '') {
			try {
				$json = Json::decode($value, Json::FORCE_ARRAY);
				if (is_array($json) === false) {
					throw new LogicException('settings must be an object');
				}
			} catch (Throwable) {
				throw ValueDoesNotMatch::createFromString("message:invalidJson", Value::of($value));
			}
		}

		return $json;
	}

}

Error formating

		if ($e instanceof InvalidData) {
			$errorPrinter = new ErrorVisualPrinter(new TypeToStringConverter());

			return $errorPrinter->printError($e);
		}

Actual Behavior

There are no errors but there are some invalid fields.

{
      "type": "shape",
      "fields": {
          "settings": {
              "type": "message",
              "message": "message:invalidJson"
          }
      },
      "errors": []
}

Because there are no errors in InvalidData exception I've tried the error printer (with TypeToStringConverter) but it does some prefixing which breaks the passed message. That's probably not a bug but a feature of the TypeToStringConverter.

throw ValueDoesNotMatch::createFromString("message:invalidJson", ...) will be converted via ErrorPrinter to "settings: message:invalidJson" in this case.

Expected Behavior

Should contain that invalid field error?

Addition information

No response

@mrceperka mrceperka added the bug Something isn't working label Nov 21, 2024
@mabar
Copy link
Member

mabar commented Nov 26, 2024

I am not sure what to do here (except better documenting the types and callbacks and documenting the printers)

Each exception thrown for a field is mapped to the field. All the fields in "fields" key are in fact invalid fields.
This is not the case when the error is thrown from the before/after mapped object callbacks. In this and only this case the error is added to the object and printed in errors instead of fields.
If you want to add an error to the object, either throw in the after class callback or use method addError() of the type accessible in context of after class callback.

Mixing errors from object and its fields is a no-go, but some improvements may be done:

  • rename fields to fieldErrors and errors to typeErrors? (same logic is used also for ArrayShape, that is currently not used by mapper itself, but may be used in custom rules and callbacks)
  • add a way to access object type from field callback (I'd really prefer to not do that)
  • represent fields with specific datatypes differently in printer, even if they are not mapped by the object mapper (extension of Handling stdClass and other datasource objects #67 issue)
  • rules like JsonRule may be added

I'll write completely new printers in future, because current one are not so easy to read and also do not count with printing invalid values, large hierarchies and the planned extended data types support, but it is crazy complex...

Btw, your example misses the null value in callback, this would fail the validation completely.
Steps to reproduce should also contain call to the processor, with an input causing the error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants