Introduce "schematic" normalizers

The blocker is closed as outdated so IS needs update

Think this is the issue, we can use the symfony property for that I think.

#3252872: Use CacheableSupportsMethodInterface for performance improvement in normalizers →

Updating IS.

👀👀👀👀

Changing the component to serialization because this is mostly not specific to JSON:API.

I am marking NR because I'm deep enough into this now that I would really like some maintainer/committer review to ensure this isn't totally off-base. I'm personally pretty proud of how this is implemented with the interface and the mostly-automatic integration of the base normalizer with the new Attribute.

So, yeah, this now requires a change to symfony/serializer which is a bummer, and I'm not quite sure what the work-around would be. I've chosen to just include the patch for now, in hopes that as we work on this that either 1) Symfony will make a minor change to two methods from private to protected, or 2) someone smarter than I comes up with a different alternative.

The issue is that we are adding a new method to normalizers to express information about their normalization, but the selection of said normalizer is hidden behind the serializer's ::normalize() method, where it finds the correct normalizer from all those registered. We need to be able to essentially statically-analyze the selected normalizer. We need to peer into ::getNormalizer(), which is currently private.

There is a dirty hack to invoke private methods with reflection, but I doubt that would pass a core quality gate and makes me feel very dirty.

Still leaving NR because I would love feedback on this and the overall approach. Not letting this slow down development, but it is a sticking point.

This should now be to the point of producing a spec-compliant schema for a resource object. There's plenty more work to be done, but I'm pretty happy with how relatively easy it was to implement the earlier generic work on schematic normalizers to JSON:API in basically an afternoon.

Re: our need for the Symfony Serializer to change two methods from private to protected, I've received some initial feedback from two Symfony maintainers and both asked good questions for follow-up.

The concrete bad news is that Symfony 6.4 and 7.0 are in feature-freeze, so the earliest this change would be accepted is 7.1.

Parallels were drawn between what we're doing and api-platform, which is an official-ish reference implementation of Symfony for doing fancy API things. They are generating JSON Schema using object reflection, and I laid out how this isn't much of a parallel to the Drupal entity and field APIs. We'll see if anyone's convinced by my reasoning enough to change the method visibility.

So optimistically, this would be blocked on Symfony 7. If this is a non-starter, there are two other options:

1. Most radical: Ditch the Symfony serializer/normalizer because we barely use it now and abuse it when we do. This would be a major lift/BC break, but it would also alleviate a lot of headaches encountered in the D8+ lifecycle.
2. Fork the Symfony Serializer and implement our own which implements SerializerInterface. This increases maintenance overhead and we are forked further from Symfony core, which we're trying to avoid. But there is precedence, e.g. we have our own YamlLoader.
3. (Ab)use NormalizerInterface::normalize()'s context parameter and allow normalizers to return a value object containing a schema. There is some precedence for returning a value object as JSON:API module needs cacheability metadata and so it passes around CacheableNormalizations. This is a bit of an escape hatch if we get cornered by other options. The downside is that is makes ::normalize() even more polymorphic and magical and also implies the value you pass is "real data," instead of potentially being "just" a supported class or interface name, which is all we might have during schema generation. Then again, that parameter is already mixed and so we could just define by convention that if the special context flag is passed, the value is to be regarded as the same as to ::supportsNormalization(). We could also use this approach to start and then deprecate it in favor of a more explicit method if we can ever make it publicly callable.

First off, im so happy to see you working on this issue. :)

I've been tring to wrap my head around this and am having a hard time hehe.

One of the things we might be able to do is perhaps use the serializer to keep track? We already overwrite the contructor for that and pass that up to the parent. Perhaps there is a way to add the required data to the normalizers there using our own interface. This also feels very tacky in a way, but could be a place where we can track that information and allow it to be pulled in the normalizer itself perhaps.

This is purely based on looking at code, have not tried and see if that would work and how that would look.

The suggestion to use another format for the json_schema in the github issue and go from there is also interesting, wouldn't that work? it might not be the fastest of things to serialize again and go through all the paces to get the schema and would add another whole normalization pipeline, but it could be cached pretty well I guess. This does seem to combine pretty well with the 'describeby' member that was added in jsonapi 1.1 though, which is a link. It could then be a link like '/jsonapi/node/article?format=json_schema'.

Basically, i have no real awnser here right now. I'm not convinced we will see Symfony change the visibility right now. Hopefully my thoughts are helpfull in a way.

Thanks @bbrala for the feedback. Sorry I couldn't make it to Lille this year to work on this in person! We got a lot done last time we pair programmed in Portland.

One of the things we might be able to do is perhaps use the serializer to keep track? We already overwrite the contructor for that and pass that up to the parent. Perhaps there is a way to add the required data to the normalizers there using our own interface. This also feels very tacky in a way, but could be a place where we can track that information and allow it to be pulled in the normalizer itself perhaps.

This is purely based on looking at code, have not tried and see if that would work and how that would look.

I think I follow what you're saying, and in theory yes we could do a lot of shenanigans within the @internal portion of JSON:API module. However there are two main gotchas to this approach: 1) despite these methods not being "technically" extend-able, modules like Extras violate that with imposters and so any normalizers from modules like that, while violating the internal rule, would need to also change. And 2) this would only address JSON:API's normalizers, but not those in the core typed data API, which provides all the schema for properties.

The suggestion to use another format for the json_schema in the github issue and go from there is also interesting, wouldn't that work?

I ran into your feedback while coming here to say that yes, I think this suggestion for treating JSON Schema as another "format" is going to be the path forward.

...it might not be the fastest of things to serialize again and go through all the paces to get the schema and would add another whole normalization pipeline, but it could be cached pretty well I guess.

Unless we're talking about different things here, and I don't think we are, I don't see a performance hit. The approach with ::getNormalizer() would depend on the same normalizer resolving process that was optimized recently with the help of upstream changes, and this is just another path for resolving a normalizer but for a different $format. Also as it stands I don't believe that performance should be a huge issue here, as the resulting schemas could be cached and won't vary much.

This does seem to combine pretty well with the 'describeby' member that was added in jsonapi 1.1 though, which is a link. It could then be a link like '/jsonapi/node/article?format=json_schema'.

That's an angle I hadn't explored yet, however I think it's really powerful and would open the door to a very low-code way for us to bring the functionality that currently lives in jsonapi_schema module → into core. I almost wonder if that shouldn't be the near-term goal vs. the OpenAPI integration, though OAI is basically just formatting the same data in a slightly different way, so these are not in competition.

Basically, i have no real awnser here right now. I'm not convinced we will see Symfony change the visibility right now. Hopefully my thoughts are helpfull in a way.

Yeah, after the third maintainer basically said "no," I agree, and honestly this is how the process should work. I iterated on the initial idea (an interface for the normalizer), it turns out that won't work and is blocked on upstream cooperation anyway, but we land on a solution that might even be "better."

I am toying with different ways of implementing this, however, that would still perhaps leverage an interface and/or trait to do most of the heavy lifting for the normalizers. I have some ideas that I'll toy around with in the MR as I refactor out of this solution and into the alternative for $format.

Marking NW as I only really had it in NR to solicit this kind of help.

Putting this back to NR as I would still love feedback from maintainers and committers.

The MR now contains a refactor of the initial approach, which I think overall is an improvement. TL;DR, you now request schema from the normalization system by specifying the $format as json_schema.

The only real nit I can see with this approach is that normalizers are resolved based on the data to be normalized and the format, and so there is theoretically a different universe of normalizers selected for schema vs. a specific format. That's pretty much unavoidable with this approach. In theory we could get 100% the same normalizers by specifying the format same as the eventual normalization and hinting in $context that we actually want the schema. (This is in spirit what we wanted by having access to ::getNormalizer().)

I don't love this but don't hate it, in theory. The practical reason we can't take this approach is that normalizers which don't care about the $format (which is actually the majority of them) need to be aware of that context, and might return a "normal" normalization instead of schema. We have to address this as it is with this change to NormalizerBase::checkFormat() to special-case the json_schema format as an exception to the "if I don't specify any formats, I serve them all" default.

Another question would be if the JSON:API implementation should wrap schema in CacheableNormalizations or not.

One thought about type-safety on the returned value (since we are using ::normalize() for schema as well as normalizations) could be to require it to specify the JSON Schema meta-schema it implements in $schema, which would be a quick check for the calling code to say "this is for sure a schema." I'm not sure if that's necessary, but one idea to throw out there if people are worried about non-core normalizers somehow getting tricked into returning a normalization when we really want a schema.

Making title less technical and more accurate as to the current goals.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Good bot.

Config schema generation is very different and a path forward for them is not very clear yet. The recent work on validation for config entities will help unlock this in the future, however.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Is there a high level summary for this issue? I can't tell from the huge changeset what exactly is being proposed here. What's the before/after? "Content entities were serialized in BLOBs before (in which cases?) but now they are serialized in JSON?" What's the benefit? Which APIs are affected, etc? Especially of an issue of this magnitude I think it would be important to outline these. It should help with reviews as well :)

+1 to what @Gábor Hojtsy said. Change records would be really helpful to understand this functionality too. 😇

Draft CR added. IS updated. MR rebased. Back to NR.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Conflict was from the conversion of typed data plugins from annotations to attributes 💯

Related: ✨ Generate JSON Schema for config entity types Active

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Rebased. Back to NR.

I went through the MR and have some comments. All and all it is a good implementation of what we talked about (quite) a while back.

Change record is available.
Issue summary seems up to date, although format is a little more freeform, would prefer to move to default setup. So keeping that tag.

Some small questions for now.

Related note, so close to having a proper JSON:API 1.0 and 1.1 schema available on jsonapi.org but it's blocked on infra: https://github.com/json-api/json-api/issues/1749

That doesn't mean we have to block this issue on that, and now that it's in a const it's easy enough to update.

I've gone through the changes and all previous comments. Think we got through everything and the feedback has been adressed. Remove NISU since the wording has changed referencing the other issue.

I'm gonna go out on a limb and push to RTBC. <3

I've added a couple of small comments to the MR that could be addressed.

I have addressed your comments.

A bit of a fat-fingered rebase but I am hoping that this is very close to RTBC still.

The comments by alex have been adressed. I agree with the argument for the array argument. Back to RTBC

bbrala → credited larowlan → .

Updated credits

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Putting this back to RTBC as it only needed a phpstan-related update relating to improved analysis as this awaits commit.

Updating tags. Marking this as a contrib blocker because new versions of OpenAPI and OpenAPI JSON:API depend on this.

Thank you!

I think the patch needs a reroll as it doesn't apply to 11.0.6.

I've rebased against 11.x, if it doesn't apply to a specific tag it would be because of a change in HEAD.