Allow specifying `meta` data on JSON:API objects

Created on 12 December 2019, almost 5 years ago
Updated 13 January 2023, almost 2 years ago

From the JSON:API spec: https://jsonapi.org/format/#document-meta

Where specified, a meta member can be used to include non-standard meta-information. The value of each meta member MUST be an object (a “meta object”).

Any members MAY be specified within meta objects.

Handling of metadata in the JSON:API normalizers already exists.

RelationshipNormalizer

    return CacheableNormalization::aggregate([
      'data' => $this->serializer->normalize($object->getData(), $format, $context),
      'links' => $this->serializer->normalize($object->getLinks(), $format, $context)->omitIfEmpty(),
      'meta' => CacheableNormalization::permanent($object->getMeta())->omitIfEmpty(),
    ]);

ResourceIdentifierNormalizer

    if ($object->getMeta()) {
      $normalization['meta'] = $this->serializer->normalize($object->getMeta(), $format, $context);
    }

The problem is there's no way to add meta information to these objects. See ResourceObjectNormalizer::serializeField

      if ($field instanceof EntityReferenceFieldItemListInterface) {
        // Build the relationship object based on the entity reference and
        // normalize that object instead.
        assert(!empty($context['resource_object']) && $context['resource_object'] instanceof ResourceObject);
        $resource_object = $context['resource_object'];
        $relationship = Relationship::createFromEntityReferenceField($resource_object, $field);
        $normalized_field = $this->serializer->normalize($relationship, $format, $context);
      }

createFromEntityReferenceField takes a meta and links parameter. But there's no way to add information.

This is useful for building a headless application on Drupal to provide context about the information.

Feature request
Status

Needs work

Version

11.0 🔥

Component
JSON API 

Last updated about 16 hours ago

Created by

🇺🇸United States mglaman WI, USA

Live updates comments and jobs are added and updated live.
  • API addition

    Enhances an existing API or introduces a new subsystem. Depending on the size and impact, possibly backportable to earlier major versions.

  • Needs documentation

    A documentation change is requested elsewhere. For Drupal core (and possibly other projects), once the change has been committed, this status should be recorded in a change record node.

  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • needs profiling

    It may affect performance, and thus requires in-depth technical reviews and profiling.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • last update over 1 year ago
    Custom Commands Failed
  • last update over 1 year ago
    Custom Commands Failed
  • last update over 1 year ago
    29,485 pass, 2 fail
  • Status changed to Needs review over 1 year ago
  • 🇳🇱Netherlands bbrala Netherlands

    I've done some updates and improved the testing. Hopefully it is enough like this.

    Mostly a question open if we should return CacheableNormalisation in the events 'getMeta' method.

    Another possible open question is if the current context testing is enough or we need a more complex case.

  • 🇳🇱Netherlands bbrala Netherlands
  • 🇳🇱Netherlands bbrala Netherlands

    Also from #61:

    1. This requires updating jsonapi.api.php, tagging Needs documentation for that.
    2. .
    3. ) few ms, see #64
  • 🇳🇱Netherlands bbrala Netherlands

    Added Wim for reviewing

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Removed patches as there's an MR
    Adding issue credits, and removing credits for simple re-rolls/rebases
    Updating issue summary and tags
    Makes the job of reviewers easier ❤️

  • Status changed to Needs work over 1 year ago
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Left a review

  • last update over 1 year ago
    29,790 pass, 16 fail
  • last update over 1 year ago
    Custom Commands Failed
  • Pipeline finished with Failed
    11 months ago
    Total: 164s
    #66812
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Trying to get this moving forward again 😄

    Merged in 11.x, addressed phpcs violations, left a review, and … there's still @larowlan's review to address.

    (Trying to only touch trivial things to make things easier but keep myself distant enough to be able to RTBC this!)

  • Pipeline finished with Failed
    11 months ago
    Total: 565s
    #66825
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    The tests now confirm the failure reported by @ptmkenney on the MR 👍 The failure also happens in kernel tests, which means it should be an easy fix 😄

  • 🇺🇸United States jrockowitz Brooklyn, NY

    Sorry to add a little noise to this ticket. I wanted to create a patch to test this via composer patches.

  • Pipeline finished with Failed
    7 months ago
    Total: 183s
    #144745
  • Status changed to Needs review 7 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    Ok, i've tried to adjust to the comments on the MR. Only not sure about my context test, think it tests the context switch, but perhaps its not nested enough?

    Rest seems to be adressed now, or at least explained ;)

  • Status changed to Needs work 7 months ago
  • 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.

  • 🇯🇵Japan ptmkenny

    The change record draft needs to be updated because MetaDataEvents was removed. I would update it myself but I don't understand all the new changes.

  • 🇳🇱Netherlands bbrala Netherlands

    Updated the changerecord to reflect the removal of the MetaDataEvents class. Fixed codestyle issues.

    I'll wait for ci ;)

  • Pipeline finished with Failed
    7 months ago
    Total: 163s
    #146136
  • Pipeline finished with Failed
    7 months ago
    Total: 172s
    #146145
  • Status changed to Needs review 7 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    Fixed spellcheck by rebasing (wonder if we should automate that...), tests are running.

    Possible todos (from #61):

    This MR introduces event dispatching in various performance-critical pieces of JSON:API. I'm fine with that costing something for sites that choose to implement this (although I would like to see numbers). But did we measure that there's no significant overhead for the >90% of sites that don't use this?

    As per #64, it adds about 1k extra events (on 16k currently) and adds a few ms uncached, which i would say is fine. Forgot to remove the tag there, sorry.

  • Pipeline finished with Failed
    7 months ago
    Total: 1042s
    #146154
  • Status changed to Needs work 7 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    Tests failed, needs work.

  • Pipeline finished with Failed
    7 months ago
    Total: 985s
    #150965
  • Pipeline finished with Failed
    7 months ago
    Total: 961s
    #150979
  • Pipeline finished with Success
    7 months ago
    Total: 987s
    #150999
  • Status changed to Needs review 7 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    Fixed event dispatching, also fixed the fact the code tried to add cachability data to non-cachable reponses. Added an extra check in the testing module so we get not warning on a non existant array key.

    Ready for review.

  • Pipeline finished with Success
    7 months ago
    Total: 1079s
    #151086
  • 🇯🇵Japan ptmkenny

    I don't understand enough about the internals of JSON:API to do a code review, but I can do an end user review because I have a mobile app that uses JSON:API as a backend. I checked out the latest version of this MR and ran my app's automated tests; everything passed. I then poked around in the app for a few minutes and the metadata is added correctly as I expected. I use the meta info to return lots of extra details about user entities, so this is critical functionality for my app.

    Just one comment on the change record. I would suggest adding comments to the functions as per the coding standards so that it's more clear that addPaymentInfoToResourceObjectMeta() is an arbitrary name for a custom method, whereas getSubscribedEvents() is the event subscriber method.

    Something like:

      /**
        * {@inheritDoc}
        */ 
      public static function getSubscribedEvents() {
        return [
          CollectResourceObjectMetaEvent::class => 'addPaymentInfoToResourceObjectMeta'
        ];
      }
    
      /**
        * Custom method to addd payment info to the order object.
        *
        * @param \Drupal\jsonapi\Events\CollectResourceObjectMetaEvent $event
        *   The event to add meta resources for.
        *
        * @return void
        */ 
      public function addPaymentInfoToResourceObjectMeta(CollectResourceObjectMetaEvent $event) {
        if ($event->getResourceObject()->getTypeName() !== 'order--order') {
          return;
        }
    
        $event->setMeta('payment_iframe_url', \Drupal::service('custom_payment.payment_data')->getIframeUrl());
      }
    
  • 🇳🇱Netherlands bbrala Netherlands

    Thank you, i've updated the changerecord. And thank you for the real world testing :)

  • Pipeline finished with Success
    7 months ago
    Total: 1046s
    #151159
  • Pipeline finished with Failed
    7 months ago
    Total: 3162s
    #151667
  • Pipeline finished with Failed
    7 months ago
    Total: 203s
    #151696
  • Pipeline finished with Failed
    7 months ago
    Total: 176s
    #151703
  • Pipeline finished with Success
    7 months ago
    Total: 1050s
    #151722
  • Status changed to Needs work 7 months ago
  • 🇺🇸United States smustgrave

    Sorry to do it, but can we have two MRs now

    10.3.x with the deprecation
    11.x with the items removed or required

    Thanks.

  • 🇺🇸United States bradjones1 Digital Nomad Life

    bradjones1 changed the visibility of the branch 3100732-rebase to hidden.

  • Merge request !78093100732 for 11.0 with deprecations removed. → (Open) created by bradjones1
  • 🇺🇸United States bradjones1 Digital Nomad Life

    Bjorn's MR will need its target changed but added one against 11.x that can stay that way, with deprecations removed.

  • Pipeline finished with Canceled
    7 months ago
    Total: 625s
    #159152
  • 🇺🇸United States bradjones1 Digital Nomad Life

    Updating IS covering last "remaining tasks." Profiling was cleared up in #82 and I think it's enough to add @Event to the new events for API docs generation.

    By way of cross-promotion, API docs are really hard to generate and maintain. Meta for better docs at #3441158: [Meta, Plan] Move official Drupal docs into repository, host from static site CI artifact .

  • Pipeline finished with Success
    7 months ago
    Total: 996s
    #159157
  • Pipeline finished with Success
    7 months ago
    Total: 1085s
    #159156
  • 🇳🇱Netherlands bbrala Netherlands

    Did the dance, unfortunately a true rebase onto the other branch did not work, so force pushed the diff. Setting to NR, pipeline is pending.

  • Status changed to Needs review 7 months ago
  • Pipeline finished with Canceled
    7 months ago
    Total: 182s
    #159655
  • Pipeline finished with Failed
    7 months ago
    Total: 4968s
    #159660
  • Status changed to Needs work 7 months ago
  • 🇺🇸United States smustgrave

    Not sure if something in gitlab got stuck? But seeing a 10.3.x test failure in jsonApi

  • Pipeline finished with Failed
    7 months ago
    Total: 6044s
    #159662
  • 🇺🇸United States bradjones1 Digital Nomad Life

    I dunno about stuck but it looked like it could be a random, but just retried and same outcome. https://git.drupalcode.org/issue/drupal-3100732/-/pipelines/159676/test_...

    I'll try to take a look today to help keep this moving.

  • 🇳🇱Netherlands bbrala Netherlands

    There was an issue with user login which broken 10.3 head. Which after also has a few other things fail. It might be something there. Too tired to check today though.

  • Pipeline finished with Failed
    7 months ago
    Total: 630s
    #160060
  • Pipeline finished with Success
    7 months ago
    Total: 511s
    #160510
  • 🇳🇱Netherlands bbrala Netherlands

    I've fixed the pipeline by using drupalResetSession in the test, which logs out the user through code.

    It is kinda weird though, that loggin out didn't work through the interface, but that is isolated from this issue.

  • Pipeline finished with Failed
    7 months ago
    Total: 538s
    #160529
  • Status changed to Needs review 7 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    Did a quick fix for the logOut issue bases on the changes here: 🐛 User logout is vulnerable to CSRF Fixed . There where changes in how we logout, we need those changes in this test also.

    I tried removing the mink session reset, but that was needed, so put it back in.

  • Pipeline finished with Success
    7 months ago
    Total: 765s
    #160549
  • Status changed to Needs work 7 months ago
  • 🇺🇸United States smustgrave

    Thanks for fixing! Left some small comments looking good!

  • Pipeline finished with Canceled
    7 months ago
    Total: 42s
    #160876
  • Status changed to Needs review 7 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    11.x is updated, ill push the same changes to the 10.3 mr (excluding the BC removal)

  • Pipeline finished with Failed
    7 months ago
    #160877
  • 🇳🇱Netherlands bbrala Netherlands

    Update 10.3 mr also, all done <3

  • Pipeline finished with Success
    7 months ago
    Total: 994s
    #160881
  • Pipeline finished with Success
    7 months ago
    Total: 1419s
    #160884
  • Status changed to RTBC 7 months ago
  • 🇺🇸United States smustgrave

    Feedback appears to be addressed

    Reran the kernel test and it was random failure.

  • Status changed to Needs work 6 months ago
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Hi Folks, unfortunately we've missed the window for 11.0.x and 10.3.x now.
    I think we need to re-do this 10.4.x as the 'from' in the deprecation notice and 12.0.x for removal.
    The only consolation is at least then there's only one MR.

    Thanks for all the effort here.

  • 🇳🇱Netherlands bbrala Netherlands

    bbrala changed the visibility of the branch 3100732-11.x to hidden.

  • Status changed to RTBC 6 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    Simple rebase, nothing changed but the deprecation message. Updated the CR and target for MR. Think those are small anough changes for a direct RTBC

  • Pipeline finished with Failed
    6 months ago
    Total: 182s
    #176804
  • Pipeline finished with Failed
    6 months ago
    Total: 171s
    #176807
  • Pipeline finished with Success
    6 months ago
    Total: 558s
    #177003
  • Status changed to Needs work 6 months ago
  • 🇳🇿New Zealand quietone

    I read the issue summary and the MR. I skimmed the comments and didn't see any unanswered questions, although like I said I only skimmed (It is noisy here and hard to concentrate).

    I left comments in the MR about the comments. I also think that this should not use $rootUser. I did not do a code review.

    Setting to NW.

  • Pipeline finished with Canceled
    6 months ago
    Total: 112s
    #183940
  • Pipeline finished with Canceled
    6 months ago
    #183942
  • Pipeline finished with Canceled
    6 months ago
    #183943
  • Pipeline finished with Canceled
    6 months ago
    Total: 8s
    #183947
  • Pipeline finished with Canceled
    6 months ago
    Total: 525s
    #183949
  • Pipeline finished with Canceled
    6 months ago
    #183957
  • Pipeline finished with Canceled
    6 months ago
    Total: 65s
    #183958
  • Pipeline finished with Canceled
    6 months ago
    #183962
  • Pipeline finished with Canceled
    6 months ago
    #183964
  • Pipeline finished with Canceled
    6 months ago
    #183965
  • Pipeline finished with Canceled
    6 months ago
    #183966
  • Pipeline finished with Canceled
    6 months ago
    #183967
  • Pipeline finished with Success
    6 months ago
    #183970
  • Pipeline finished with Success
    5 months ago
    Total: 540s
    #210438
  • Pipeline finished with Failed
    5 months ago
    Total: 170s
    #210526
  • Pipeline finished with Failed
    5 months ago
    Total: 595s
    #210536
  • Pipeline finished with Failed
    5 months ago
    #210552
  • Pipeline finished with Failed
    5 months ago
    Total: 180s
    #210620
  • Pipeline finished with Failed
    5 months ago
    Total: 541s
    #210632
  • Pipeline finished with Failed
    5 months ago
    Total: 572s
    #210685
  • Pipeline finished with Success
    5 months ago
    Total: 631s
    #210702
  • Status changed to Needs review 5 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    Resolved all feedback.

    1. Add an admin user and check for an admin role in the testing
    2. Improve some comments to help developers use this feature.
    3. Update tests.

    All good I think :)

  • Status changed to Needs work 4 months ago
  • 🇺🇸United States smustgrave

    Can the MR 7828 be updated for 11.x please

  • Pipeline finished with Canceled
    4 months ago
    Total: 61s
    #226289
  • Pipeline finished with Failed
    4 months ago
    Total: 1872s
    #226290
  • Pipeline finished with Failed
    4 months ago
    #226321
  • Pipeline finished with Canceled
    4 months ago
    #226324
  • Pipeline finished with Canceled
    4 months ago
    Total: 100s
    #226358
  • Pipeline finished with Failed
    4 months ago
    Total: 450s
    #226360
  • Pipeline finished with Success
    4 months ago
    Total: 448s
    #226377
  • Status changed to Needs review 4 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    Rebased onto 11.x and fixed a small issue with a missing return type.

  • Status changed to RTBC 4 months ago
  • 🇺🇸United States smustgrave

    MR for 11.x is showing as all green

    All threads appear to have been addressed

    CR reads well to me and even better has examples (love those)

    Running the test-only feature gets https://git.drupalcode.org/issue/drupal-3100732/-/jobs/2141544 showing the coverage

    Code review wise nothing seems to be off that hasn't been fixed already
    Deprecation message appears correct.
    Everything appears to be typehinted.

    Believe this one is good to go.

  • Status changed to Needs work about 1 month ago
  • 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.

  • Pipeline finished with Success
    about 1 month ago
    Total: 636s
    #304692
  • 🇯🇵Japan ptmkenny

    In #113, needs-review-queue-bot only reported a missing strict types declaration. I added this and am setting back to RTBC since it is an extremely minor change and all tests are passing.

  • 🇺🇸United States bradjones1 Digital Nomad Life

    Rebasing.

  • Pipeline finished with Failed
    14 days ago
    Total: 764s
    #327297
  • 🇳🇿New Zealand jonathan_hunt

    Typo at L43 core/modules/jsonapi/tests/modules/jsonapi_test_meta_events/src/EventSubscriber/MetaEventSubscriber.php, also L87, "// Only continue if the recourse type is enabled.", presumably recourse should be resource.
    https://git.drupalcode.org/project/drupal/-/merge_requests/7828/diffs#3e...

Production build 0.71.5 2024