I'm not sure how/why this started happening, but when I turn on rest.module and hit http://8x.localhost:8082/entity/node/1 in my browser, I get:

<response>
<nid>
<value>1</value>
</nid>
<uuid>
<value>1d0b7f7c-18e6-4023-a700-bef36f2e00c3</value>
</uuid>
<vid>
<value>1</value>
</vid>
<type>
<value>article</value>
</type>
...

Awesome! Didn't even need to carefully craft some cURL request to send an Accept header!

However, if I hit a REST export view in my browser, I get:

Error
The website has encountered an error. Please try again later.
Error message
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).

This is decidedly un-user friendly. :)

Let's figure out how rest.module is doing its magic for entities and get it to do it for Views as well. :)

Files: 
CommentFileSizeAuthor
#32 drupal-1969870-32.patch2.75 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,561 pass(es).
[ View ]
#32 interdiff.txt902 bytesdawehner
#26 drupal-1969870-26.patch2.75 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,781 pass(es).
[ View ]
#26 interdiff.txt810 bytesdawehner
#25 drupal-1969870-25.patch2.78 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 56,553 pass(es).
[ View ]
#25 interdiff.txt836 bytesdawehner
#22 drupal-1969870-22.patch2.78 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 56,034 pass(es).
[ View ]
#22 interdiff.txt906 bytesdawehner
#18 1969870-18.patch2.78 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,781 pass(es).
[ View ]
#18 interdiff-1969870-18.txt3.04 KBdamiankloip
#15 1969870-15.patch2.44 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,724 pass(es).
[ View ]
#15 interdiff-1969870-15.txt2.08 KBdamiankloip
#13 drupal-1969870-13.patch2.45 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 55,949 pass(es), 1 fail(s), and 6 exception(s).
[ View ]
#13 interdiff.txt2.03 KBdawehner
#8 1969870-8.patch2.03 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,840 pass(es).
[ View ]
#8 interdiff-1969870-8.txt2.13 KBdamiankloip
#1 drupal-1969870-1.patch1.75 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 54,474 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.75 KB
PASSED: [[SimpleTest]]: [MySQL] 54,474 pass(es).
[ View ]

That's just a simple fix, read for more information below and how to fix it on the longrun.

The way the rest modules handles it is the following:

It registers one route per format, so for example entity/node/{node} for json, xml. As there is no special content-type requested, the acceptable mime-types are:

text/html, application/xhtml+xml, application/xml, */*
(@see MimeTypeMatcher::filter()
so basically all routes match. As far as I understand the routing system, the one who comes first then wins, which is (maybe not on purpose) xml.

Views could get such a default behavior when it would use the route system. Though there is currently a dependency tree:

Status:Needs review» Needs work
Issue tags:-VDC

The last submitted patch, drupal-1969870-1.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+VDC

#1: drupal-1969870-1.patch queued for re-testing.

You're right about XML not being on purpose. I think that we should probably make it default to JSON in both places, XML is serialization module's red headed stepchild.

Status:Needs review» Postponed

Ok, sounds then like this is postponed on those other issues for now, since we don't want to hard-code in red-headedness.

Title:REST export view should default to XMLREST export view should default to JSON

Let's leave it postponed, but I agree with lin - JSON should be the default format served.

I don't know but I think we should get this in first, because a) the other issues still need time (*hint* #1956896: Add role based access checker is RTBC)
and also this patch adds some kind of test, which is always helpful. Beside from this, it wouldn't block the other issue, as I guess we should not fix the needed behavior of this issue in #1800998: Use route system instead of hook_menu() in Views

Status:Postponed» Needs review
StatusFileSize
new2.13 KB
new2.03 KB
PASSED: [[SimpleTest]]: [MySQL] 55,840 pass(es).
[ View ]

Ok, let's just try and get this in then - Should be pretty simple.

I think this approach would be better; To set the contentType property on RestExport to 'json', and only override it if it's not html. Thoughts?

I'm wondering whether it should be json or hal_json by default.

Well, it's tricky, right? Because it should be whatever the primary serialization format is. Currently, D8 ships with both json-ld and hal+jason, but both are optional modules, separate from REST.

Should then logic then be:

If hal_json is enabled use hal_json
If json_ld is enabled use json_ld
else use json?

yes, that logic makes sense but even simpler since jsonld is poised for removal. #1935548: Remove JSON-LD module

StatusFileSize
new2.03 KB
new2.45 KB
FAILED: [[SimpleTest]]: [MySQL] 55,949 pass(es), 1 fail(s), and 6 exception(s).
[ View ]

Added this logic without taking care about json_ld.

Status:Needs review» Needs work

The last submitted patch, drupal-1969870-13.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.08 KB
new2.44 KB
PASSED: [[SimpleTest]]: [MySQL] 55,724 pass(es).
[ View ]

Just fixing it up. I'm not really a fan of having conditional module logic in the display plugin like this. Especially as hal will only work with entities aswell?

Great, Thank you. The code looks fine, so it's just about a comment of lin.

Damian is right, HAL will only work with entities.

I think we can simply default to JSON in this case. The only thing that HAL buys you over JSON is that it has link relations... and link relations are only important to you if you are a machine. Since this is about what gets shown in the browser, we're probably working with a human user, so the hypermedia special sauce isn't necessary.

StatusFileSize
new3.04 KB
new2.78 KB
PASSED: [[SimpleTest]]: [MySQL] 55,781 pass(es).
[ View ]

Thanks Lin. I was always of the mindset that JSON should be our default format, you explain things really well though :) Defaulting to hal just doesn't make sense.

This is a mix of previous patches (logic from #8 and test from #13), reverting hal_json back to json.

Status:Needs review» Reviewed & tested by the community

It's good to see that the logic is quite simple

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/rest/lib/Drupal/rest/Plugin/views/display/RestExport.phpundefined
@@ -78,7 +78,13 @@ public function initDisplay(ViewExecutable $view, array &$display, array &$optio
-    $this->setContentType($negotiation->getContentType($request));
+    $request_content_type = $negotiation->getContentType($request);
+    // Only set the requested content type if it's not 'html'. The default
+    // 'json' content will be used in this case.
+    if ($request_content_type !== 'html') {
+      $this->setContentType($request_content_type);
+    }
+
     $this->setMimeType($request->getMimeType($this->contentType));

Shouldn't this be $this->setContentType($this->contentType);

Or maybe the comment is just confusing...

Status:Needs work» Needs review
StatusFileSize
new906 bytes
new2.78 KB
PASSED: [[SimpleTest]]: [MySQL] 56,034 pass(es).
[ View ]

Shouldn't this be $this->setContentType($this->contentType);

I don't think it's needed to set it to json if it's already set onto json?

Maybe this small change helps to understand the comment?

Status:Needs review» Needs work

thanks @dawehner that comment is indeed clearer.

+++ b/core/modules/rest/lib/Drupal/rest/Tests/Views/StyleSerializerTest.phpundefined
@@ -120,12 +120,16 @@ public function testSerializerResponses() {
+    // Test that the default output will be JSON if html is requested.
+    $expected = $serializer->serialize($entities, 'json');
+    $actual = $this->drupalGet('test/serialize/entity');
+    $this->assertIdentical($actual, $expected, 'If hal module is enabled by default hal_json output is returned.');

I don't think the hal module has anything to do with this.

EDIT: duplicate comment

Status:Needs work» Needs review
StatusFileSize
new836 bytes
new2.78 KB
PASSED: [[SimpleTest]]: [MySQL] 56,553 pass(es).
[ View ]

Good catch.

StatusFileSize
new810 bytes
new2.75 KB
PASSED: [[SimpleTest]]: [MySQL] 55,781 pass(es).
[ View ]

As always alex spotted that there is no json module, let's get rid of the module comment.

Looks good to me. Alex?

+1

If Alex is happy with the new comment I think we're good to go again.

Status:Needs review» Reviewed & tested by the community

Oh

Status:Reviewed & tested by the community» Needs work

Ok I've now understood why this patch confuses me...

+++ b/core/modules/rest/lib/Drupal/rest/Plugin/views/display/RestExport.phpundefined
@@ -78,7 +78,13 @@ public function initDisplay(ViewExecutable $view, array &$display, array &$optio
+    $request_content_type = $negotiation->getContentType($request);
+    // Only determine the requested content type if it's not 'html'. The default
+    // 'json' content will be used else.
+    if ($request_content_type !== 'html') {
+      $this->setContentType($request_content_type);
+    }

To me this should be...

    $request_content_type = $negotiation->getContentType($request);
    // Only use the requested content type if it's not 'html'. If it is then default
    // to 'json' to aid debugging.
    if ($request_content_type !== 'html') {
      $this->setContentType($request_content_type);
    }

Status:Needs work» Needs review
StatusFileSize
new902 bytes
new2.75 KB
PASSED: [[SimpleTest]]: [MySQL] 55,561 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

Simple enough.

Doesn't this violate HTTP spec? If the agent asked for html, why should we return json?

Status:Reviewed & tested by the community» Fixed

Committed fc886f5 and pushed to 8.x. Thanks!

#34 to #35 was a crosspost; do we want to reopen the discussion?

Re #34, I don't object that this was committed, given that in practice, browsers include */* in their Accept header, but we don't yet have #1505080: [META] Content Negotiation for Accept Headers in place in order to handle that correctly. Perhaps after that issue though, we can revisit the implementation of what was committed here in order to not return json for a hypothetical agent that requested html (and nothing but html).

Since this issue is marked fixed and will therefore, fall off our radar, I also left a comment in #1505080-40: [META] Content Negotiation for Accept Headers and raised the priority of that issue.

@effulgentsia: so even if/when we do have this content negotiation, what do you propose to server when html is requested? serializer doesn't support html, or do you want to add an encoder for that? Throwing an error etc.. didn't fly with people before, so not sure.

Once both content negotiation and #1800998: Use route system instead of hook_menu() in Views are in, then when dynamically creating the route for a View that has only a REST export display, we'll need to do a $route->setRequirement('_format', $supported_serialization_formats);. If an agent asks for the URL without anything in the Accept header that matches a supported format, then the routing system will return a HTTP 415 (Unsupported Media Type) status code.

Throwing an error etc.. didn't fly with people before

That's because most browsers include */* in their Accept header, so they're capable of displaying JSON, and it's convenient for debugging to take advantage of that. But for agents that specifically ask for 'text/html' only, we need to honor that by returning a 415.

Automatically closed -- issue fixed for 2 weeks with no activity.