Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a uri attribute in field_module JSON:API resource objects #468

Open
jgaehring opened this issue Jan 4, 2022 · 6 comments
Open

Comments

@jgaehring
Copy link
Member

I think we talked about adding a uri attribute to the JSON object for each module that comes back from the api/field_module/field_module, but I guess it never got implemented.

Hey @jgaehring thanks for identifying this again. The TLDR is that we can't add computed fields to config entities, so adding "dynamic" properties (such as uri) into the field_module jsonapi resource is challenging.

This made me wonder about using the meta section of the resource, which led me to an open issue to support adding custom meta data: https://www.drupal.org/project/drupal/issues/3100732 Lots of nitty gritty there, but one specific comment here was of interest: https://www.drupal.org/project/drupal/issues/3100732#comment-13439744

JSON:API Hypermedia works by running an event dispatcher during normalization.

This reminded me of the JSON:API Hypermedia module which provides the ability to add custom links to the resource object - which is what we're trying to do here for field modules!! I think this could be a great (and easy!) solution!

Implementation is really quite simple - I added the following FieldModuleLinkProvider plugin:

<?php

namespace Drupal\farm_fieldkit\Plugin\jsonapi_hypermedia\LinkProvider;

use Drupal\Core\Access\AccessResult;
use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Url;
use Drupal\jsonapi_hypermedia\AccessRestrictedLink;
use Drupal\jsonapi_hypermedia\Plugin\LinkProviderBase;

/**
 * @JsonapiHypermediaLinkProvider(
 *   id = "farm_fieldkit.field_module.index",
 *   link_relation_type = "index",
 *   link_context = {
 *     "resource_object" = "field_module--field_module",
 *   }
 * )
 */
class FieldModuleLinkProvider extends LinkProviderBase {

  /**
   * {@inheritdoc}
   */
  public function getLink($context) {
    $field_module_id = $context->getField('drupal_internal__id');
    $url = Url::fromRoute('farm_fieldkit.field_module_js', ['field_module' => $field_module_id]);
    $cacheability = new CacheableMetadata();
    $access_result = AccessResult::allowed();
    return AccessRestrictedLink::createLink($access_result, $cacheability, $url, $this->getLinkRelationType());
  }

}

Which provides an index link for each field_module JSON:API resource:

"data": [
    {
      "type": "field_module--field_module",
      "id": "9cce513d-9b7d-44ff-bcaf-d7c8c0627dad",
      "links": {
        "describedby": {
          "href": "http://localhost/api/field_module/field_module/resource/schema"
        },
        "index": {
          "href": "http://localhost/fieldkit/js/test/index.js"
        },
        "self": {
          "href": "http://localhost/api/field_module/field_module/9cce513d-9b7d-44ff-bcaf-d7c8c0627dad"
        }
      },
      "attributes": {
        "langcode": "en",
        "status": true,
        "dependencies": {
          "enforced": {
            "module": [
              "farm_fieldkit_test"
            ]
          }
        },
        "drupal_internal__id": "test",
        "label": "Test module",
        "description": "Just a field module for testing.",
        "library": "farm_fieldkit_test/test_field_module"
      }
    }
  ],

You'll notice the describedby link is added here, too. This is actually provided by the jsonapi_schema module which already supports this jsonapi_hypermedia module!

SO my question for you @jgaehring describedby, index and self are all standardized link relations. I chose index because we are pointing to the field module "index.js", it just seemed natural. But is there a different link_relation_type you might prefer?

We also have the ability to provide our own link relation type in a farm_fieldkit.link_relation_types.yml file (an example: https://git.drupalcode.org/project/jsonapi_hypermedia/-/blob/8.x-1.x/jsonapi_hypermedia.link_relation_types.yml). We could do this, and something like field_module might make sense, but after seeing the standardized list I'm curious if we can't just use one of those instead!

Originally posted by @paul121 in farmOS/field-kit#480 (comment)

@jgaehring
Copy link
Member Author

The TLDR is that we can't add computed fields to config entities, so adding "dynamic" properties (such as uri) into the field_module jsonapi resource is challenging.

Is there a hard and fast reason this needs to be a config entity to begin with? Or is there anyway this would not have to be a computed field?

SO my question for you @jgaehring describedby, index and self are all standardized link relations. I chose index because we are pointing to the field module "index.js", it just seemed natural. But is there a different link_relation_type you might prefer?

I wonder whether what we're doing can be properly described as a "link relation" if it is not a document but an executable script we're linking to. The JSON:API spec implies the resource represented by the link must be "data." The Hypermedia module seems intended for links equivalent to what could be represented by an HTML <link> tag's rel attribute, and those again are primarily intended for documents, unless it is rel="preload" along with as="script", but that seems like a stretch. It all feels a bit off to me, adding more complexity than should be required.

Again, it makes me wonder if this needs to be a config entity if it's going to impose so many restrictions. The fundamental requirements for a field module seem pretty simple to me:

  1. Serve a script at fixed location.
  2. Provide that location as JSON data from a known endpoint.

Is it possible to just start from those first principles and go from there?

@mstenta
Copy link
Member

mstenta commented Jan 4, 2022

Worth noting that we are essentially doing this already:

  1. Serve a script at fixed location.

It only changed this one time because we decided to re-namespace the module last minute. This won't happen again, so we could just put a stake in the ground right now and say "this won't EVER change again".

It would be nice if we could add the URI - but if that turns out to be a challenge it's still just a "nice to have" I think.

@mstenta
Copy link
Member

mstenta commented Jan 4, 2022

We can always figure out the URI in JSON in the future, and switch over to using that, while maintaining the old URI as a shortcut for BC. That's just to say I don't think this necessarily needs to block anything right now.

@jgaehring
Copy link
Member Author

while maintaining the old URI as a shortcut

Just to clarify, there's no URI in the current JSON response. Just this:

{
  "type": "field_module--field_module",
  "id": "42ff2d3b-3c98-4d0b-a3c5-7e54800a1137",
  "links": {
    "self": {
      "href": "https://test.farmos.dev/api/field_module/field_module/42ff2d3b-3c98-4d0b-a3c5-7e54800a1137"
    }
  },
  "attributes": {
    "langcode": "en",
    "status": true,
    "dependencies": {
      "enforced": {
        "module": [
          "farm_fieldkit_test"
        ]
      }
    },
    "drupal_internal__id": "test",
    "label": "Test module",
    "description": "Just a field module for testing.",
    "library": "farm_fieldkit_test/test_field_module"
  }
}

When all I really need is this:

{
  "type": "field_module--field_module",
  "id": "42ff2d3b-3c98-4d0b-a3c5-7e54800a1137",
  "attributes": {
    "uri": "fieldkit/js/test/index.js"
  }
}

That's a lot of parsing to ask the client to do for such a simple link that the server already has. Seems crazy to me this can't be added by the server.

@mstenta
Copy link
Member

mstenta commented Jan 4, 2022

Sorry yea I just meant the URI is fixed in code, so it will always follow the same pattern. It is not included in the API endpoints, correct.

What does Field Kit use the api/field_module/field_module endpoint for currently? Just to get a list of module machine names and labels?

The URI of the JS file always follows this pattern: /fieldkit/js/[module-machine-name]/index.js - so at the very least we can just hard code that in both places (farmOS and FK). Until we figure out an easy way to include it...

Is there a hard and fast reason this needs to be a config entity to begin with? Or is there anyway this would not have to be a computed field?

As config entities we get those API endpoints "for free" (among other tooling for discovery within farmOS). Otherwise we need to build all that ourselves. We certainly could do that, but it's not trivial, and not worth it right now if this is the only reason IMO.

@mstenta
Copy link
Member

mstenta commented Jan 4, 2022

FWIW here is where the JS path is defined:

path: 'fieldkit/js/{field_module}/index.js'

That is the routing YML file for the Field Kit module - just to show that it is quite "hard coded" to that path right now. It cannot be changed/different for individual Field Modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants