Repository navigation
Representing a patch documents as data transfer objects #12
Description
Thanks for the excellent library :)
I'd quite like to be able to express a patch in PHP code rather than a JSON string, e.g. something like:
$FastJsonPatch = FastJsonPatch::fromJson($document);
$FastJsonPatch->apply(new JsonPatch(
new Add(path: '/contacts-', value: ['name' => 'Jane']),
new Replace(path: '/contacts/0/number', value: '+1 212 555 1212'),
new Remove('/contacts/1'),
));I think this could be done quite easily with a combination of simple value objects. I'd propose that these are separate objects to the existing blancks\JsonPatch\operations\PatchOperation, so that they have no behaviour or properties other than those that would be present in the JSON representation. For example:
namespace tbc;
final readonly class JsonPatch {
public function __construct(
public Operation ...$operations
)
// possibly implement iterator interface
}
final readonly class Remove implements Operation {
public function __construct(
public string $path
)
}Ideally the apply method typehint would be widened to public function apply(string|JsonPatch $patch): void so that the user can pass through their collection object.
This could either just stringify it and then pass it through to the existing json decoding in patchIterator. Or, I think it could keep it as an object and iterate it or it could keep it as an object and iterate it directly (the new DTOs just being used instead of the stdClass that currently comes from the decoded JSON patch).
The only real issue I can think of is picking a namespace / naming that is clear and concise for the end user, but does not conflict with the existing operation classes.
I would be very happy to put together a PR with an implementation for this if you'd be willing to consider it?
Activity
blancks commentedon Nov 27, 2025
Hi @acoulton, glad this project has been useful, and thank you for having a look at the source code!
Serializing the DTOs back to JSON only to decode them again sounds like a waste of CPU cycles to me.
I'm absolutely fine with your proposal. The
patchIteratormethod could be refactored to handle an object, maybe by breaking it down into a couple of more specific private methods to process the patch depending on whether it's a string or an instance of a class.About the conflicting naming issue, I would workaround this by refactoring the existing operation class naming to OperationHandler. For example:
AddHandler,CopyHandler, etc.Those classes are not meant for external use. As long as we don't change the existing abstract
PatchOperationclass or thePatchOperationInterface, there should be zero risk of a backward-incompatible change.Let me know what you think about this approach. And if you'd like to put together a PR, I would be absolutely happy with that!
acoulton commentedon Nov 27, 2025
@blancks thanks for the response. I agree, keeping them as objects should be much more efficient and I'm pretty sure that can be done safely within the private implementation.
Renaming the existing classes to
...Handlersounds like a good solution for the naming issue. My only comment would be that I don't think they are explicitly tagged or documented as@internalso although they might not be meant for external use, there may be end-users who don't realise that?I always lean quite cautious/strict on semver (maybe overcautious, but I've been bitten before 😆).
So I'd perhaps either do the rename as a 3.0 release (and mark them
@internalat the same time), or do something like:This will trigger phpstan warnings on the actual
extends Adddefinitions which would have to be ignored, but usage of the child class would be fine - and it would guarantee safety for anyone using the library in an unexpected way.What do you think?
Very happy to work on a PR.
blancks commentedon Nov 29, 2025
@acoulton They're not tagged as
@internal, indeed. But they're all final classes that are already registered for use inside theFastJsonPatchclass.The only scenario I can think of is if end-users somehow used them standalone, putting effort into figuring out the required data representation and dependencies to feed them.
This is something I thought to be extremely unlikely, in my honest opinion, but I totally see your point here.
The deprecation can break ci/cd pipelines since we are assuming people might have make use of those classes.
So I'm fine going strict with semver and bumping the major version 💪
blancks commentedon Nov 29, 2025
I have created a
devbranch that you can use as the base for the PR