Add a REST export display plugin and serializer integration.

+++ b/lib/Drupal/views/Plugin/views/display/Json.phpundefined
@@ -0,0 +1,239 @@
+  public function executeHookMenu($callbacks) {

Is there a reason not to extend page, this is quite a lot of duplicated code ... There is no problem with doing that, for example Feed is doing this as well.

+++ b/lib/Drupal/views/Plugin/views/display/Json.phpundefined
@@ -0,0 +1,239 @@
+    return new JsonResponse($this->view->result);

The one problem we have with this approach is that flapi fields aren't part of the actual result, but _entity will be, so it could be that this value is quite polluted with unwanted stuff.

+++ b/lib/Drupal/views/Tests/Plugin/DisplayJsonTest.phpundefined
@@ -0,0 +1,57 @@
+    $actual_json = $this->drupalGet('test/json');
+    $this->assertResponse(200);

Can we actually test for the mimetype of the http result?

Neat patch. Will need a reroll against core.

Attached one patch with reroll.

tag since related

+++ b/core/modules/system/lib/Drupal/system/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,169 @@
+    $options['style'] = array(
+      'contains' => array(
+        'type' => array('default' => 'json'),
+        'options' => array('default' => array()),
+      ),
+    );
+    $options['row'] = array(
+      'contains' => array(
+        'type' => array('default' => 'data_entity'),
+        'options' => array('default' => array()),
+      ),
+    );
...
+dpm($options);

What about just using $options['style]['contains']['type']['default'] = 'json'; This would look much simpler.

+++ b/core/modules/system/lib/Drupal/system/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,169 @@
+    unset($options['hide_admin_links'], $options['analyze-theme']);

Maybe we should document that.

We went through the patch together and improved it.

Remove the group form from the style settings.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,129 @@
+ * Definition of Drupal\views\Plugin\views\display\Data.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataEntityRow.phpundefined
@@ -0,0 +1,41 @@
+ * Definition of Drupal\views\Plugin\views\row\DataEntityRow.

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayJsonTest.phpundefined
@@ -0,0 +1,68 @@
+ * Definition of Drupal\views\Tests\Plugin\DisplayJsonTest.

Should be Contains

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,52 @@
+  public function render($row) {
+    foreach ($this->view->field as $id => $field) {
+      $output[$field->field_alias] = $row->{$field->field_alias};
+    }
+
+    return $output;

This should initialize $output just in case.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,81 @@
+  public function getFieldKeyAlias($id) {

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/Json.phpundefined
@@ -0,0 +1,73 @@
+  public function init(ViewExecutable $view, &$display, $options = NULL) {

I didn't notice earlier, but this is missing a docblock.

This looks really cool overall.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+  public function buildOptionsForm(&$form, &$form_state) {

Just for consistency it should call parent::

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+      $options = $this->options;

Nitpick alarm: Why not use $this->options directly? It's just used in two places so this should be fine.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+      $output[$this->getFieldKeyAlias($id)] = $row->{$field->field_alias};

I hope it is okay to call a function per field and per row. In theory this mapping could be precalculated but yeah that's not so important.

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayJsonTest.phpundefined
@@ -0,0 +1,125 @@
+class DisplayJsonTest extends PluginTestBase {

It seems to be for me that it might make sense to split up the tests for the displayJson test from testing for example the fields row plugin. What do you think about that? We could though still reuse the test view.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+  public function buildOptionsForm(&$form, &$form_state) {

Just for consistency it should call parent::

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+      $options = $this->options;

Nitpick alarm: Why not use $this->options directly? It's just used in two places so this should be fine.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.phpundefined
@@ -0,0 +1,90 @@
+      $output[$this->getFieldKeyAlias($id)] = $row->{$field->field_alias};

I hope it is okay to call a function per field and per row. In theory this mapping could be precalculated but yeah that's not so important.

#24: 1819760-24.patch queued for re-testing.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,136 @@
+   *
+   * @var bool
...
+   *
+   * @var bool
...
+   *
+   * @var bool
...
+   *
+   * @var bool

It's a bit nitpicking but i don't think it makes sense to document the type here, as it is already part of the parent.

YAY! This looks AWESOME!

Default the style plugin to JSON on a "Data" display. Right now it keeps the old default (?) or at any rate the radio is not auto-selected.

When submitting the style form to make it JSON, then I received a blank confirm form.

And after submitting the form, saw:

Notice: Undefined index: style_options in Drupal\views\Plugin\views\display\DisplayPluginBase->submitOptionsForm() (line 2286 ofcore/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php).

However! After that, it actually looks to indeed be outputting JSON. :D

Also, let's make sure to get linclark's sign-off on this, and make sure this isn't going to harm the JSON-LD work.

Fixed the UX issue that you don't have a proper style/row style selected by default.

The test configuration have to be updated as well.

Overlap with REST

It is a great idea to respond different depending on the request type. I'm not really sure about the connection
between the configuration of a view and a json result. Do you want, for example, always the full entities or maybe just certain fields (like the patch is providing). I'm also not sure, how to handle aggregated data, but let's discuss that.

I'm not sure about this; as we are dealing with raw row data (and we have to) for the row plugins, rather than anything rendered, So I think we would maybe have a need to use separate views. Otherwise you will get alot of junk in your output :) Also, we need to make this work with fields too, so a similar principle applies there.

Maybe we could provide a sane default but provide a way to override it, if there is more custom handling needed.

Overlap with serialization
Since the plugin is coupled to the format, it directly creates a JsonResponse object. It would be possible to decouple this and make it so that the data plugin could respond to any format if it used the Serializer service instead. This would depend on #1814864: Provide way to register serialization classes being committed and also require creating a normalizer that creates the expected JSON array structure (which shouldn't be too hard).

So in fact the patch currently provides a ultra-light serializer (not sure about the terminology)
which allows to output an array structure (provided by the row definition (for example the full entity or just certain fields))
with a certain format (currently there is just one provided for json but more could be added). The title of the issue got updated
to make it a bit clearer.

Recommendation
I expect a plugin will eventually be unnecessary if the REST work is completed. However, we may want to put this in as a stopgap and re-evaluate once Klaus is further with REST routing. Since Serializer is pretty close to getting committed, we probably do want to move towards using that instead of coupling the plugin to a format, though.

This makes totally sense, replace json with serialized will automatically work.

Lin pointed me here. I think there's two separate and valid use cases here, which parallel what's done in HTML.

1) Entity-based. Each row is a full entity. For this case, the JSON-LD serialization work that Lin is doing should be the format that gets used. That neatly solves the entire "collection" listing problem in REST, because "The answer is Views, what's the question?" is still valid and correct. I don't think it needs any configurability beyond which serializer to use, and should be all tricked out and hypermedia-ified (possibly in follow-ups).

2) Field-based. Each row is some combination of fields, like a fielded view in HTML. This likely would not use JSON-LD at all, and would be just a one-off JSON structure that you'd have to code to directly. (That's what we used for the DrupalCon mobile app, actually, way back in late-model Drupal 6, with the Views Datasource module.) I'm OK with this one not being JSON-LD-ified-semantic-yummy given the complexity involved.

Note that while there will usually be a path overlap between different mime type views, that's not guaranteed. That means it will need to be possible to generate a URI that is only capable of outputting JSON-LD data, not HTML.

I haven't dug into the changes for Views in D8 yet, so I defer to those who have on actual implementation.

Wonderful idea! I would be very happy about this patch gets in as it will abandon services_views module.

We miss validation on the aliases to make sure we don't have same alias for two fields so in output they won't override each other.

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.phpundefined
@@ -864,9 +866,9 @@ public function getFormBucket(ViewUI $view, $type, $display) {
-        'title' => $rearrange_text,
-        'href' => $rearrange_url,
...
+        'title' => t('Rearrange'),
+        'href' => "admin/structure/views/nojs/rearrange/{$view->get('name')}/{$display['id']}/$type",

Shouldn't we still have a different rearrange_text/url for filters?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/Serialize.phpundefined
@@ -0,0 +1,102 @@
+    // @todo get the serializer formats properly. How??
+    $formats = array('json', 'jsonld');

Do we still need the selection, as it is not used at all with the serializer.

PHP is so WTF. What do you think about the sort of cleaner fix in this patch?

  /**
+   * Overrides Drupal\views\Plugin\views\style\StylePluginBase::usesFields().
+   */
+  public function usesFields() {
+    if (!empty($this->row_plugin)) {
+      return $this->row_plugin->usesFields();
+    }

Any reason for an override of the logic? Basically the default usesFields method should work here as well. I see your problem. The style plugin should not define to use fields, when it just leverages row plugins.

This patch fixes some smaller things like a missing defineOptions entry, wrong variable names etc. or misnamed files for tests etc.
The interdiff is pretty useless, sorry.

There are remaining test failures, but it seems to be that first node has to be converted to entityNG

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/Serialize.phpundefined
@@ -0,0 +1,62 @@
+    $serializer = drupal_container()->get('serializer');
+    $request = drupal_container()->get('request');
+    $negotiation = drupal_container()->get('content_negotiation');

Just realized that we don't want to get all this objects on each row but store it on the object.

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/ViewEditFormController.phpundefined
@@ -817,8 +817,14 @@ public function getFormBucket(ViewUI $view, $type, $display) {
+    $rearrange_url = "admin/structure/views/nojs/rearrange/{$view->get('name')}/{$display['id']}/$type";
+    $rearrange_text = t('Rearrange');
+    $class = 'icon compact rearrange';

@@ -847,10 +854,7 @@ public function getFormBucket(ViewUI $view, $type, $display) {
-      default:
-        $rearrange_url = "admin/structure/views/nojs/rearrange/{$view->get('name')}/{$display['id']}/$type";
-        $rearrange_text = t('Rearrange');

This change is part of another patch so remove it here.

Might be of interest here: #1678572: Add support for a raw text formatter

In short : it's a request to add a "raw, unfiltered text" formatter in core for text fields, primary use case being views-based data exports.
We "won't fix"ed it on the grounds that:
- the use case is very non-80%, while offering this formatter on "Manage display" for every text field is a handing a loaded gun to all users
- D8 will have better options for data export (that was referring to JSON-LD entity serializtion, which doesn't rely on formatters)

My understanding is that this thread invalidates the second item ?
At any rate, feedback welcome over there :-)

seems to work ok for field rows, obviously not yet with entities

(from #63

Is there a blocker for integrating with entities? Do we think thats a follow-up?

Is there a blocker for integrating with entities? Do we think thats a follow-up?

The problem is that entities would have to support entityNG to be able to test it. We could use the entity_test entity, though the patch to add views integration for that didn't got any review: #1839624: Add a views integration for entity_test.module

Thanks for quick reroll. I tested this a lot and have some UI feedback and a possible bug report.

To test, I enabled entity_test.module and created a few entity_test entities. Then I created a new View with a Data display and a path and then requested that path with Accept: application/json and Accept: application/ld+json headers.

1. When requesting with Accept: application/ld+json, I get
   

   Fatal error: Call to a member function uri() on a non-object in /Users/mweitzman/htd/d8x/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php on line 68
   

   . Is there a different header I should be using? We should add this to the Test, IMO

2. In Views UI, we should be able to simplify a bit:
   1. Remove Exposed Form
   2. Remove CSS Class
   3. The Show: item has a choice of Entity or Fields. When using Fields, I get no way to add/remove Fields. This may be a limitation of the entity_test views integration?
3. Once Views rendering is cleaned up, we should remove Header/Footer/Pager as well since they are not applicable to Data Display.

Thanks for testing, highly welcomed!

The Show: item has a choice of Entity or Fields. When using Fields, I get no way to add/remove Fields. This may be a limitation of the entity_test views integration?

For some odd reason the fixes in #49 and #51 didn't went into the actual current patch. (patched that in again)
This automatically also hides the header/footer/empty text.

Remove Exposed Form
Remove CSS Class

Yeah I totally agree that they don't make sense here! Removed them.

Fatal error: Call to a member function uri() on a non-object in /Users/mweitzman/htd/d8x/core/modules/jsonld/lib/Drupal/jsonld/JsonldEntityWrapper.php on line 68

No idea how this could have happened. If I add a new request in the test it for sure returns the expected data.

Thanks for making those changes and for adding testing for JSONLD. We're RTBC now.

#77: drupal-1819760-77.patch queued for re-testing.

Verified to still work. Reminder that this only works for entityNG entities. For now, you have to enable entity_test module and create using entity-test/add in order to see this in action.

I have spotted some code duplication.

We have defining $content_type in both display and row plugins.

Display plugin:

+  public function getContentType() {
+    if (!isset($this->contentType)) {
+      // Return the content type based on the request object.
+      $negotiation = drupal_container()->get('content_negotiation');
+      $request = drupal_container()->get('request');
+      $content_type = $negotiation->getContentType($request);
+
+      $this->setContentType($request->getMimeType($content_type));
+    }
+
+    return $this->contentType;
+  }

Row plugin:

+    $content_type = $this->negotiation->getContentType($this->request);
+    $this->displayHandler->setContentType($this->request->getMimeType($content_type));

Can't we in row plugin just call $content_type = $this->displayHandler->getContentType();? Then we would not need protected $request and $negotiation variables in row plugin and initializing them in init() method.

Sorry for my terminology. Glad you understood what I meant.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/Data.phpundefined
@@ -0,0 +1,219 @@
+
+    $this->contentType = $negotiation->getContentType($request);
+    $this->mimeType = $request->getMimeType($this->contentType);

Lets use $this->setContentType() and $this->setMimeType() here instead of assigning properties directly.

So let's get it back to RTBC.

So, the description of this functionality sounds absolutely amazing. In practice, however, I wasn't able to make it work. I go into these patches blind to emulate how a new user would behave, so I'm sorry if I repeat anything said above.

The first thing I tried was to do a Content view, add a Data display to it, and have it output fields. This caused me to lose my theme (maintenance page), with the errors:

Notice: Undefined property: stdClass::$unknown in Drupal\views\Plugin\views\row\DataFieldRow->render() (line 107 of core/modules/views/lib/Drupal/views/Plugin/views/row/DataFieldRow.php).
• Symfony\Component\Serializer\Exception\UnexpectedValueException: Serialization for the format html is not supported in Symfony\Component\Serializer\Serializer->serialize() (line 78 of /Users/webchick/Sites/8.x/core/vendor/symfony/serializer/Symfony/Component/Serializer/Serializer.php).

I was informed by Damian that this is because the display is set up to expect "EntityNG" entities, and currently in core the only one we have is Entity Test, at least until/unless #1778178: Convert comments to the new Entity Field API is in.

PROBLEM I think we should add some more robust error handling here; either by preventing you from adding a Data display altogether on an unsupported base table, or else at least not dying outright.

I tried manually enabling entity_test module, and had to do some digging to figure out how to add test entities (entity-test/add in case you are wondering :P) so I could test it.

This didn't give me a notice, but it did still give me the HTML error above.

Also, I feel like there must be a place where I can select JSON instead of HTML, but I can't seem to figure out where it is. :(

So.. I'm stuck. Marking needs work for the error handling, and I'm going to need pretty explicit test instructions.

While I'm at it, here are a few small string tweaks I noticed:

Under "Data: Style options":

[ ] Force using fields
If neither the row nor the style plugin supports fields, this field allows to enable them, so you can for example use groupby.


Should be "group by"

I also am not sure I understand what that means. What will be the consequences if I click this, despite choosing "Entity" under "Show"?

Then if I switch "Entity" to "Fields" I get:

"FIELD ID ALIASES
Rename views default field ID's in the output data.
id"

First, ID's should be IDs.

Secondly, "id" should be "Id" (I think? It's weird to have field labels that aren't capitalized)

Lastly, why would I want to do this? :) Can you add some help text here to explain?

===

UPDATE! I talked to Damian about this in IRC and he informed me of a couple of things:

1) I had a wild misunderstanding of what this patch did; I read "Data" and thought "CSV export", "JSON", etc. when in reality this is setting up a web services endpoint (which explains the errors above, since I was accessing it with just a browser, not a carefully crafted request).

PROBLEM Can we rename this display then to something more explicit like "Web services endpoint" so it sounds scary and people don't use it unless that's what they actually want?

2) The reason the path is giving those errors is because that's the default thing Symfony does when you pass it things it doesn't support.

PROBLEM I don't think we can get away with this, because unlike normal web service endpoints, whose URLs will be a MENU_CALLBACK type and never show up in the UI anywhere, these paths are displayed smack in your face on the views listing page and *of course* you're going to click them because that's how you test all of your other views. Suggestion is to replace those ugly PHP errors with something nice like:

"HTML method not supported. Please access this URL with Accept-header: application/json."

...I think that was everything! Whew. This still seems very cool, but it does need a bit more work.

Also, given the advanced nature of this, I wonder if this plugin ought to be exposed by the JSON-LD module, rather than Views itself. I think most "weekend hacker" types would be totally lost and end up as befuddled as me. :)

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/RestExport.phpundefined
@@ -13,20 +13,20 @@
+ *   id = "rest_export",
...
+class RestExport extends PathPluginBase {

You know this was written in a way to support more then just rest webservices ... should we just don't tell people that this will also provide a theoretical possibility for CSV support, which is THE format for a lot of people just using excel? With this we end up again with multiple contrib and custom solutions again :(

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/WSEndpoint.phpundefined
@@ -216,4 +217,14 @@ public function render() {
+   * The DisplayPluginBase preview method assumes we will be returning a render
+   * array. The data plugin will already return the serialized string.

Sure, the symfony serializer framework don't have an idea about drupal render arrays, but should maybe the drupal layer use simple render arrays? Not sure whether this is required for caching.

I think REST export is a decent name because it encompasses CVS as well as JSON response. Basically, this plugin returns your results in whatever format you requested in your HTTP Accept header (assuming the site supports your format). Thats's REST. I think we are RTBC.

+1

If I have not committed this in 24 hours, please ping me incessantly about it in IRC. :P

Really sorry that this fell off my radar last week. :(

+1 from me, too.

I opened #1891202: REST Export Views format should be configurable as follow-up.

WOOHOO!

ok I gave this one more spin, everything looks great. :) Here's the command I used to test:

curl -H "Accept: application/json" -H "Content-Type: application/json" -X GET http://8x.localhost:8082/resty

Since this feature was RTBC before BAP, I committed and pushed to 8.x. YAY!