New examples for REST API in D8

You GO! Added @marvil07 to maintainers with full privs...

Looking forward to this. In fact; I would love to help out if I can.

Pretty please. :-)

Going to try to work on this during DrupalCon Pragu Lab Session on Wednesday,

Feel free to assign yourself back if you're still working on it.

I'll try to give this a go.

Just tested out the REST module, so I'll try to throw up what I've got while it's still fresh in my memory. Just to have a starting point.

OK, here is my first go.

A lot of commenting is still missing, but it should be a good leg up for some one who might want to do the more educational stuff.

I developed it in beta4.

Gah, wrong patch.

We'll try this one instead...

Hmm, better get this re-written for beta6. I see a few changes in the ConfigFormBase().

Thanks for the work, @tlyngej!

The patch applies (with some whitespace warnings), but I can't install the module. Here's what the log says:

[18-Mar-2015 12:13:31 Europe/Berlin] PHP Fatal error:  Call to private method Drupal\Core\Form\FormBase::container() from context 'Drupal\cache_example\Forms\CacheExampleForm' in /Users/paulmitchum/projects/drupal8/modules/examples/cache_example/src/Forms/CacheExampleForm.php on line 42
[18-Mar-2015 19:20:28 Europe/Berlin] Uncaught PHP Exception Drupal\Component\Plugin\Exception\PluginNotFoundException: "The "yes-no" plugin does not exist." at /Users/paulmitchum/projects/drupal8/core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php line 57
[18-Mar-2015 19:20:29 Europe/Berlin] PHP Fatal error:  Class Drupal\rest_example\Form\RestExampleClientSettings contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Drupal\Core\Form\ConfigFormBase::getEditableConfigNames) in /Users/paulmitchum/projects/drupal8/modules/examples/rest_example/src/Form/RestExampleClientSettings.php on line 20

Aside from that, a few more formal things:

1. --- /dev/null
   +++ b/rest_example/config/install/node.type.rest_example_test.yml
   
   --- /dev/null
   +++ b/rest_example/config/install/views.view.rest_service.yml
   
   --- /dev/null
   +++ b/rest_example/rest_example.links.menu.yml
   
   --- /dev/null
   +++ b/rest_example/rest_example.routing.yml
   

   For these YML files I'd really like to see some # comments as to what they're for, why they exist, what naming conventions are in use, why different routes were created... Generally make them more readable.

2. +++ b/rest_example/rest_example.links.menu.yml
   --- /dev/null
   +++ b/rest_example/rest_example.module
   

   rest_example.module really needs a @defgroup. See the examples module checklist: #2209627: [meta] Module Checklist for Examples We also need a reasonable explanation of what this module does, such as whether it provides a RESTful API, whether it consumes external APIs, etc.

3. +++ b/rest_example/rest_example.module
   @@ -0,0 +1,5 @@
   diff --git a/rest_example/rest_example.routing.yml b/rest_example/rest_example.routing.yml
   

   We also need a page that the user can read to find out what's going on. So if they go to /examples/rest_example they see some text explaining what the module does, how to use it, pointers about where they should go and look next within the code, etc.

Thanks for some really great feedback. I don't see that to often :-)

The problem about installing comes from some changes made to the configuration system, made post beta4, and I will look into that when ever I get a spare moment.

@Mile23

Got the issue fixed, but I have some issues with getting the authentication, when using ClientInterface::get() method.

Works with other methods, like post, but only for anonymous users when using get.

So what, specifically, are we trying to demo here? Is it adding a resource to our site? Or is it consuming other APIs?

It's a demo of how Drupal works as a REST server, and how to control entities using \GuzzleHttp\ClientInterface(). This example uses nodes as they works out of the box.

One site can be set as both client and server, or one can make two installations of the example module, and the "client" will be able to manage nodes on the "server" site. Lets pretend that the user has two installations.

The server part of the module comes with a View, that the client can request, that sends back (as JSON) a list of all nodes, of a specific type. From the response the client create a table, containing information about the node, including an edit and delete link (see image attachment Selection_006.png). The client can edit these nodes or delete them. The client can also create new nodes on the remote Drupal site.

I've also created a simple form, that allows one to set the remote site URL, along with username and password for authentication (see image attachment Selection_007.png).

I don't think that consuming API's outside of the Drupal universe would suite this module. That sounds more like a Guzzle/ClientInterface example module to me.

Was that specific enough?

Shoot, thought I had it.

Turned out that there was a few more API changes in the beta10.

But I think that I'll have something ready soon. Hopefully today.

Right, another patch, that seems to be compatible with the beta10 release.

Fingers crossed!

Here is a patch that works with the latest git version of Drupal8.

The String::checkPlain() has changed to SafeMarkup::checkPlain().

Re-running the test to make sure everything is still working with the latest code Core.

Lovely.

Let's re-write that patch then...

If you check the 'view' link in the test results box, you'll see a list of the failing tests. In this case, they were unrelated to your patch.

I just fixed those today. :-)

So I'm restarting your test.

Looks like a good start, but there are a few things missing... :-)

First, check out the checklist for examples modules: #2209627: [meta] Module Checklist for Examples

This module doesn't have any tests at all. It needs, at minimum, to test for error-free 200s for all the menus it registers.

For instance, I installed it and went to examples/rest_client_actions, but I got an 'unexpected error' message. There needs to be a test that verifies such things don't happen.

Some specific things:

1. +++ b/rest_example/src/Form/RestExampleClientDelete.php
   @@ -0,0 +1,76 @@
   +  public function buildForm(array $form, FormStateInterface $form_state, $id = NULL) {
   +    $form = parent::buildForm($form, $form_state);
   +
   +    if (!is_null($id) && !is_numeric($id)) {
   +      return new \Symfony\Component\HttpFoundation\Response('The ID passed in the URL is not an integer', 500);
   +    }
   +    $form_state->set('node_id', $id);
   +
   +    return $form;
   +  }
   

   It's nice that you're doing due diligence on the input, but that's what FormInterface::validateForm() is for. :-)

   I think this needs to change in the other forms as well.

2. +++ b/rest_example/src/RestExampleClientCalls.php
   @@ -0,0 +1,148 @@
   +  public function __construct(\GuzzleHttp\ClientInterface $client) {
   

   We want a use statement with the FQDN and then just reference the easy class name in the rest of the file. So in this case:

   use \GuzzleHttp\ClientInterface;
   
   __construct(ClientInterface $client) { ....
   

   There are a bunch of these throughout the module.

Thanks for the review Mile23.

A few questions:

1) What do you think of the level of documentation? This is my first contribution to the example module (as far as I remember) and I'm not sure what would be a reasonable.

2) With "We want a use statement", who is "we" (this modules maintainers? Drupal community? Is there a general guideline?) and why do we not want the FQDN? Normally, when I use FQDN, it's because I only access it once, so creating a use statement would only add more code.

Ohh, and at https://www.drupal.org/node/2209627 you mention a top level README. Is this for the content that currently reside in my .module file? None of the other modules has this, so I couldn't just do what ever they do :-)

Here's a new version of the module.

I created use statements and are now using aliases instead.
I created a test class to test the declared menus.
I created FormInterface::validateForm() in my form classes and performs the test of missing variables there. Though, I did leave the test for a value in the create/update form, as I need it when I build the form.

But I did have some issues though that I was hoping that I could get some help with.

1) I could not run my test, as Drupal failed on a View configuration file (views.view.rest_service). The View is exported through the config interface, and I couldn't figure out what went wrong in that process.

2) I wanted to create a unit test for the RestExampleClientCalls class, but I couldn't figure out how to do it with prerequisites the the system has (I need to know the username, password and URL, from the setting form) so I skipped that for now.

Lets try with a file that contains something this time :-)

Used an isset() on the result of a function call.

Fixed it in this patch.

2) With "We want a use statement", who is "we" (this modules maintainers? Drupal community? Is there a general guideline?) and why do we not want the FQDN? Normally, when I use FQDN, it's because I only access it once, so creating a use statement would only add more code.

The Royal "We."

Kidding! It's a coding standard: https://www.drupal.org/node/1353118

You can set up various IDEs to help you identify coding standards errors, usually involving the php_cs tool: https://www.drupal.org/node/147789

If you use NetBeans, then you might want this: https://www.drupal.org/sandbox/mile23/2197899

Also, setting to Needs Review so the testbot can do it's thing. Feel free to do this whenever you submit a patch.

Right there in the first bullet point: "must not use their fully-qualified name inside the code". Don't know why I missed that.

I've been using phpcs with NetBeans for years, but not actually not for D8 projects, as the standard available was VERY broken a while back. Not even Drupal 8 Core was capable of following it. But maybe I should try turning it on again, see if it has gotten better.

Yeah, I forgot the set the issue to "Need review". I was in eager to get to bed, so I forgot :-)

As expected, the test failed because of the View, that I exported from the Configuration Manager, has some discrepancies with the ConfigSchemaChecker. I'll try to get someone on IRC to help me out with that.

Just following along ..this is going to be a much needed example topic.

so thanks for working on it. :)

Just picking up a few coding standard errors. nothing substantial.

Excelent Martin. Thanks a lot for the changes.

I'm just looking at RestExampleClientCalls in this change.

1) I want to harmonize the return value of the family of calls

RestExampleClientCalls::create()/update()/delete()

If the configuration setting rest_example.settings:server_url is not defined.
then RestExampleClientCalls::remoteUrl is undefined.

Uncomfortably before this patch the return type of create/update/delete changed to void/null from Response object

Instead I have change this to a 500 Response object saying in effect "I haven't been configured yet."

I've not found the right message text yet, so I've gone with something simple

"The remote URL has not been setup."

2) Assigning a constant to RestExampleClientCalls::clientHeaders can be done outside the constructor in a slightly simpler way.

3) I am now injecting the config_factory clearing the hidden dependancy on Drupal.php.

4) The class now passes a coding standard review as defined by phpcs --standard=Drupal
So I ve added lots of annotation, camel cased state variables, and fixed some indentation issues.

Using the corrections from #39 I've created a new patch.

Hopefully the exported View in this can pass the test. :-S

Trying a new patch that fixes the errors the CI complained about.

It looks like there's a branch blocker. Don't sweat the fails that are unrelated to your module.

Digging through the test fails I see this

Declaration of Drupal\field_example\Plugin\Field\FieldFormatter\SimpleTextFormatter::viewElements() must be compatible with Drupal\Core\Field\FormatterInterface::viewElements(Drupal\Core\Field\FieldItemListInterface $items, $langcode)

this is the change record we must use to fix the problem

https://www.drupal.org/node/2575729

in short, we need to add an extra $langcode parameter to SimpleTextFormatter::viewElements()

I would do this myself ... but I am busy today sorry.

Yah, I'm working on it. This isn't the place discuss, however. :-)

White space errors but still applied. Couldn't install because of php error. Error below ...

[Sat Jan 30 11:43:54.605647 2016] [fcgid:warn] [pid 18064:tid 2973798400] [client 127.0.0.1:53291] mod_fcgid: stderr: Uncaught PHP Exception Drupal\\Core\\Config\\UnsupportedDataTypeConfigException: "Invalid data type in config views.view.rest_service: A YAML file cannot contain tabs as indentation at line 30 (near "\ttype: perm")." at /Users/imioloj/Downloads/drupal/core/lib/Drupal/Core/Config/FileStorage.php line 103, referer: http://drupal.dd:8083/admin/modules/list/confirm

@jerryimiolo I just tested with Drupal 8.0.3 on my own machine and on simplytest.me with version 8.0.4-dev. Installation went fine bot times.

What version of Drupal are you using?

Some review:

It needs to modify examples.module so it has a toolbar link to whichever is the 'main' page.

There should be more explanation on the pages as to what to do and how to recover from a problem.

If you don't set a remote host and then try to create a node, there is no error even though no node is created.

I tried to set my local host as the remote and then create a node, and I got this error: Uncaught PHP Exception GuzzleHttp\Exception\ClientException: "Client error: 405" at /Users/paul/pj2/drupal/vendor/guzzlehttp/guzzle/src/Middleware.php line 69 Note that this is the same Drupal instance as client and server, and I don't know if that matters, but it seems that'd be an obvious use case for an example.

Also there are some @todos in the code that might be stale given the age of this patch.

I also had no issues with installation just now on simplytest.me, which is using Drupal 8.0.7-dev, fwiw. Not a full review, but ideally this report is of some use for now with more to come from me.

whups - links for REST examples both worked on SimplyTest.me instance until I entered client connection info

then when I visit /examples/rest_client_actions I get the following cryptic error "The website encountered an unexpected error. Please try again later."

no issues with the usual suspect links that seem often to throw that error (e.g. the modules page); just /examples/rest_client_actions

Another version (finally).

• There now is a link in the Examples toolbar menu.
• There now is a dedicated 'Getting started' page.
• An exception is thrown if the the HTTP response code doesn't have the right value, when querying the endpoint service.
• @todo's has been handled.

I can now see that I wrote this before I knew how services in Drupal worked.

I should re-factor this patch to use dependency injection properly.

1. +++ b/rest_example/rest_example.info.yml
   @@ -0,0 +1,11 @@
   +  - rest
   +  - basic_auth
   +  - hal
   +  - views
   +  - examples
   

   Quick look on patch:

   dependencies should be in format {project_name}:{module}

2. +++ b/rest_example/rest_example.links.action.yml
   @@ -0,0 +1,5 @@
   \ No newline at end of file
   

   A pretty new line :)

3. +++ b/rest_example/src/Controller/RestExampleClientController.php
   @@ -0,0 +1,57 @@
   +    $config_factory = \Drupal::configFactory();
   ...
   +    $client = new RestExampleClientCalls(\Drupal::httpClient(), $config_factory);
   
   +++ b/rest_example/src/Controller/RestExampleHelpController.php
   @@ -0,0 +1,79 @@
   +        '\Drupal\rest_example\Controller\RestExampleClientController::indexAction(): Builds the list of nodes on the remote server.',
   

   As you said, DI would be better !

Re-rolling and updating the patch.

Added description template, made RestExampleClientCalls a service and few more adjustments.

The examples/rest-client-actions path is giving me this:

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException</em>: You have requested a non-existent service &quot;messenger&quot;. in <em class="placeholder">Drupal\Component\DependencyInjection\Container-&gt;get()</em> (line <em class="placeholder">151</em> of <em class="placeholder">core/lib/Drupal/Component/DependencyInjection/Container.php</em>). <pre class="backtrace">Drupal\rest_example\Controller\RestExampleClientController::create(Object) (Line: 28)

The messenger service is 8.5.x+ https://www.drupal.org/node/2774931

We want to target core 8.4.x, since that's the current stable release.

Reviewing:

1. +++ b/rest_example/rest_example.links.menu.yml
   @@ -0,0 +1,22 @@
   +rest_examples.help:
   ...
   +rest_example.settings:
   ...
   +rest_example.actions:
   

   Settings and actions should be children of help, and help should be expanded: TRUE.

   Help should be titled 'REST Example' and the others should be called 'Client settings' and 'Client actions'.

2. +++ b/rest_example/rest_example.routing.yml
   @@ -0,0 +1,55 @@
   +    _access: 'TRUE'
   ...
   +    _access: 'TRUE'
   ...
   +    _access: 'TRUE'
   ...
   +    _access: 'TRUE'
   ...
   +    _access: 'TRUE'
   ...
   +    _access: 'TRUE'
   

   Should be _permission: 'access content' unless there's some reason they need different permissions. And if they need different permissions, then that should be explained in comments. We should use some kind of content admin permission for the routes that can change content.

+++ b/rest_example/config/install/node.type.rest_example_test.yml
@@ -0,0 +1,19 @@
+# Imports a new node type that we use for testing REST. The content that we
+# create, read, update and delete will be of this type.

This content type persists after the module is uninstalled.

So like in node_type_example we should add:

dependencies:
  enforced:
    module:
      - rest_example

Rerolled the patch.

Setting to NR to trigger a testbot run.

Bringing this back.... I think we might need some actual work on this but I am blank on the implementation please review and suggest some changes...

coding standards

The default branch is 3.x-dev now, which is Drupal 9 compatible.

Setting to NW as testing did not pass.

FYI, A VUE.js based REST API example.

Moving to new examples meta

Replacing core version with core_version_requirement in info.yml

Patch from #86 looks good, and testbot is happy. Thanks everyone for the tremendous effort over 7 years!

+++ b/modules/rest_example/rest_example.info.yml
@@ -0,0 +1,11 @@
+  - views:views

BTW, here should be drupal:views which failed testing on HEAD. Fixing in #3160118: Fix broken testing on HEAD