JSON API does a great job at providing both normalization and denormalization of entities through its JsonApiDocumentTopLevelNormalizer ; this lead the Entity Share project to use it for client / server communciation between differents Drupal sites.

In light of recent changes to mark all the normalizer and decoder private services though, there's a problem, because jsonapi.entity.to_jsonapi only handles normalize, not denormalize ; as far as I can tell, this means that there's no way to port things like http://cgit.drupalcode.org/entity_share/tree/modules/entity_share_client...

I feel like it'd be really nice to add denormalize to Drupal\jsonapi\EntityToJsonApi, making the denormalization process both easier to grasp for consumers and working with a private normalizer.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DeFr created an issue. See original summary.

Grimreaper’s picture

Hello,

Thanks @DeFr for opening this issue.

I knew that one day using an internal service of JsonAPI would cause problem for Entity share. I didn't took the time before to open an issue on JsonAPI.

e0ipso’s picture

I think this feature request it totally fair. However it opens up the module's API surface.

DeFr’s picture

Status: Active » Needs review
FileSize
4.88 KB

Attaching a patch to get some discussion started, both on the approach and some side effect.

Currently added denormalize, and a matching test, which isn't curently passing because there's some back and forth for the various ids, which might warrant some discussion: for the node, before the normalize call, base field ids (nid, vid) are PHP integers, and entity reference field target ids are string ( e.g. uid[0]['target_id'] = '1' ). After the denormalize call, the situation is reversed ; base fields come out as string and related entities caming have their id retrieved from the uuid, and ends up as integer.

Status: Needs review » Needs work

The last submitted patch, 4: 2939827-4-api-for-denormalize.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

DeFr’s picture

Status: Needs work » Needs review
FileSize
4.98 KB

Relaxing the test to accept type difference for now. Also fixing coding standards.

Status: Needs review » Needs work

The last submitted patch, 6: 2939827-6-api-for-denormalize.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Wim Leers’s picture

Grimreaper’s picture

Hello,

Rebasing the patch.

I was trying to test if it could solve problem on Entity share, with the last code JSON API code base, Entity share is broken. I will create an issue for that.

Grimreaper’s picture

Wim Leers’s picture

I'm wondering if this is kind of the same problem as #2955615: Field properties are not being denormalized?

Wim Leers’s picture

Version: 8.x-1.x-dev » 8.x-2.x-dev

#11: past self: no, it's not the same problem. Not at all. This issue asking for a new public (supported) API to denormalize a JSON API representation (a JSON string) into an entity object.

That makes this definitely a pure feature request; moving this to 8.x-2.x.

Wim Leers’s picture

Status: Needs work » Postponed (maintainer needs more info)

Now, for an actual answer to the feature being requested here.

I have a question for the entity_share module maintainers: do you really need to denormalize JSON API representations of entities? Isn't it possible to instead use subrequests?

That'd work like this:

  1. Perform a POST subrequest for each received JSON API entity representation.
  2. If it didn't exist yet, it'll result in a 2xx response.
  3. If it did already exist, you'll get a 409 (indicating conflict). You can then decide whether to PATCH or not.

Using this approach would mean:

  • Less logic in entity_share!
  • Smaller API surface to maintain for JSON API!
  • Win-win? 🙂
Wim Leers’s picture

Project: JSON:API » Entity Share
Version: 8.x-2.x-dev » 8.x-1.x-dev
Status: Postponed (maintainer needs more info) » Active

Moving to entity_share to get feedback.

Grimreaper’s picture

Hi all,

Thanks @Wim for pushing this.

Sorry for the delay. I had to wait to be at DrupalDevDays to have at least time to say thank you.

I don't think how Entity share works internally is important. That's why when (hopefully one day...) there will be tests #2909022: Write tests, I think we will focus on functionnal tests.

What people want/need is to deploy content from a website to another.

Using JSON API or maybe one day also GraphQL, denormalizing entities or using subrequests is not important.

They want an UI (or CLI commands) to share content entities.

When you tell "subrequest", do you mean using this module https://www.drupal.org/project/subrequests?

Or parsing the JSON API response, and then doing POST requests "manually"?

Currently I am focused on #2983249: Follow up changes in JSON API.

Because Entity share is partially broken with the last dev version of JSON API. Thanks for your change records to have highlighted changes.

So yeah if your solution will avoid Entity share to be broken, yeah \o/, let's go for it.

Just a question: is it ok for translations? Adding a translation to an existing entity?

Note: I have subscribed to issues notifications on JSONAPI, So I got a notification, but as there are too many notifications ( ;) ) I didn't see that there was a comment addressed to me. Sorry.

Grimreaper’s picture

Project: Entity Share » JSON:API
Version: 8.x-1.x-dev » 8.x-2.x-dev

Moving back to jsonapi to get a reply ;)

Wim Leers’s picture

Project: JSON:API » Entity Share
Version: 8.x-2.x-dev » 8.x-1.x-dev

So yeah if your solution will avoid Entity share to be broken, yeah \o/, let's go for it.

👍

Just a question: is it ok for translations? Adding a translation to an existing entity?

There is no proper support for translation yet in JSON API, but we'll be working on that soon in #2794431: [META] Formalize translations support. We'll then add test coverage, and then yes, the same will be true for translations :)
Adding new translations definitely doesn't work yet today. That would be added in #2794431 or in another issue.

Note: I have subscribed to issues notifications on JSONAPI, So I got a notification, but as there are too many notifications ( ;) ) I didn't see that there was a comment addressed to me. Sorry.

No problem! Hopefully the amount of issue queue activity will reduce in the next 10 weeks or so, as the JSON API module approaches completion :)

Grimreaper’s picture

Thanks @Wim Leers for the quick reply.

Just a last question not answered from comment #15:

When you tell "subrequest", do you mean using this module https://www.drupal.org/project/subrequests?

Or parsing the JSON API response, and then doing POST requests "manually"?

Wim Leers’s picture

Sorry! I did not mean https://www.drupal.org/project/subrequests.

I meant \Symfony\Component\HttpKernel\HttpKernelInterface::SUB_REQUEST. For examples in core, see \Drupal\Core\EventSubscriber\DefaultExceptionHtmlSubscriber::makeSubrequest() and \Drupal\comment\Controller\CommentController::commentPermalink().

That way you can talk to the local site's JSON API without doing HTTP requests: you'd skip the HTTP layer, the network layer, and just talk to Symfony directly.

e0ipso’s picture

Which incidentally is what the subrequests module uses.

Grimreaper’s picture

Thanks @Wim Leers and @e0ipso for your reply.

I didn't know it was possible, I will see the examples when working on that.

Thanks again!

Grimreaper’s picture

Version: 8.x-1.x-dev » 8.x-2.x-dev

Switching Entity share branch.

Grimreaper’s picture

Priority: Normal » Critical

  • Grimreaper authored 532ab5a on 8.x-2.x
    Issue #2939827 by Grimreaper: Rework pull form to not require entity...
Grimreaper’s picture

Hello,

Here is a poke I am trying to make to understand how subrequest works.

@Wim leers and/or @e0ipso: when going to admin/content/entity_share/pull, the code from the patch is execusted but I get a 415 status code from the subrequest and the following error trace.

The website encountered an unexpected error. Please try again later.
Recoverable fatal error: Argument 2 passed to Drupal\Core\Controller\TitleResolver::getTitle() must be an instance of Symfony\Component\Routing\Route, null given, called in /project/www/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php on line 193 and defined in Drupal\Core\Controller\TitleResolver->getTitle() (line 50 of core/lib/Drupal/Core/Controller/TitleResolver.php).
Drupal\Core\Controller\TitleResolver->getTitle(Object, NULL) (Line: 193)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}(Array) (Line: 233)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
call_user_func(Array, Object, 'kernel.view', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 156)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 665)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Would it be possible to have your advise please?

Because when looking at the examples from comment 19, the method and the parameters are retrieve from the current request. Here I am making a completely different request.

Also I tried to look at the subrequests module code, but I couldn't find something to help me inside.

Grimreaper’s picture

Priority: Critical » Major

Now that 8.x-2.0-alpha1 is out, Lowering priority to major. Top priority now is to have tests #2909022: Write tests.

Also posting about external API on #3032787-12: [META] Start creating the public PHP API of the JSON:API module

FatherShawn’s picture

I'm curious if you considered pushing the shared entity from server -> client via JSON:API rather than pulling it and having to deal with the denormalization within Entity Share? If it was considered, what obstacles lead to pull instead of push?

Grimreaper’s picture

Yes, pushing from server to client will be done in #2856715: Push form.

Historically, the pull had been implemented before the push to have the same architecture as the https://www.drupal.org/project/webfactory module. As Entity share had been created to avoid the architecture problems in the webfactory module.

And also because at the beginning of implementing Entity Share, the JSON:API was lacking some features, and some still currently, for example support for creating translations, so with a pull it can be handled.

Grimreaper’s picture

Version: 8.x-2.x-dev » 8.x-3.x-dev
joachim’s picture

The jsonapi_extras module provides a PHP API to JSONAPI: see the service Drupal\jsonapi_extras\EntityToJsonApi.
It does that by making a subrequest to core's JSONAPI route.