Passing the DomainObject to the validateRequest() method of the hydrator class

[Ilyes512]

I am about to create my own hydrator base class so I can get my hands on the current DomainObject when validating. I was wondering if this made sense, and if so, if you would except a merge request for it?

So the change would mainly be made here: https://github.com/woohoolabs/yin/blob/master/src/JsonApi/Hydrator/UpdateHydratorTrait.php#L107-L108 I will pass the $domainObject-var to the $this->validateRequest($request); call and update the abstract methods signatures. The same changes would also be made for the create side/hydrator.

The reason I would like to do this is to pass the current object that contains data that I would like to use when validating the incoming request.

Any thoughts?

[Ilyes512]

So I made a fork and made the changes needed to solve my current issue: master...signet-connectivity:pass-object-to-validator

I guess its a backwards incompatible change. I could solve this without making these changes, but I would have to copy/paste some of the trait methods.

I think in the end that it's hard to extend/reuse AbstractHydrator because of the traits here.

The traits are both an implementation and a contract. I think separating just that would make things easier to extend.

[kocsismate]

Hi @Ilyes512

Sorry for not not replying so long. I'll carve out some free time for Yin this week so that I can look into the open tickets.

[Ilyes512]

No problem :) BTW, I am also no a 100% sure if this is the way to go.

[kocsismate]

Can you please tell me why the $domainObject is needed for validation? Does it have more information besides the data from the request?

Regarding the BC impact, I have a possible idea: we could add a new UpdateHydratorTrait::validateDomainObject() method which would be called after request validation. I like this idea because request validation was separated from the object validation this way, but I'm curious if it would work for your use-case?

[Ilyes512]

Does it have more information besides the data from the request?

Yes. So what I have is an entity that has 2 attributes, limit and maxLimit. Users are able to adjust limit between 0 and maxLimit. Where maxLimit is different for each entity (dynamic), but not configurable by the user (it is set and updated by another service).

Regarding your solution... I think that will work in my case. First check that the received limit is an int and higher than 0. And after that, validate if it's lower than the maxLimit.

The validateDomainObject.... would it receive the object before or after being hydrated (after going through getAttributeHydrator)?.

[kocsismate]

Thanks for the quick answers!

The validateDomainObject.... would it receive the object before or after being hydrated (after going through getAttributeHydrator)?.

My idea is to run validateDomainObject() after it has been hydrated

[kocsismate]

I added e55ee84. Can you please test it?

[Ilyes512]

Yes! I tested this out on my actual problem... and it works for my case :).

Just one remark (cause now it wouldn't be backwards imcompatible). Maybe also pass the JsonApiRequestInterface $request to validateDomainObject. I don't have a case now where I would need the request, but if something is in the request, but it's not in the DomainObject... maybe I this is a step to far ¯_(ツ)_/¯

To merge my requests (internal work projects) I would need this to be released to a stable version first... I guess it won't take long before it's tagged :)?

[kocsismate]

Maybe also pass the JsonApiRequestInterface $request to validateDomainObject. I don't have a case now where I would need the request, but if something is in the request, but it's not in the DomainObject... maybe I this is a step to far

Huh, although I think it's really a bit too much of overengineering, I'm ok to add it (e.g. CreateHydratorTrait::validateClientGeneratedId() already does something similar). And I've just realized that the Exception Factory should be added as well. Unfortunately, I missed to add it to validateRequest() (or maybe it has a reason I forgot). However, as I've already started to work on v5.0, I'm going to fix this inconsistency issue there.

To merge my requests (internal work projects) I would need this to be released to a stable version first... I guess it won't take long before it's tagged :)?

I have to wait a little bit for another feedback (#103), but I can release the next version soon if everything goes well.

[kocsismate]

I've just release 4.3.0 including the validation of the domain object. :)