Skip to content

Commit

Permalink
Serve HTTP 415 responses for unsupported media types
Browse files Browse the repository at this point in the history 
Don't fall back to JSON if unsupported format was requested

See #91
  • Loading branch information
@fsteeg
fsteeg committed Jun 22, 2018
1 parent 81b7f48 commit 0e2dd40
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 21 deletions.
15 changes: 8 additions & 7 deletions app/controllers/Accept.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ private Accept() {

enum Format {
JSON_LINES("jsonl", "application/x-jsonlines"), //
JSON_LD("json(.+)?", "application/json", "application/ld+json"), //
JSON_LD("json(:.+)?", "application/json", "application/ld+json"), //
HTML("html", "text/html"), //
RDF_XML("rdf", "application/rdf+xml", "application/xml", "text/xml"), //
N_TRIPLE("nt", "application/n-triples", "text/plain"), //
Expand All @@ -46,12 +46,13 @@ public static Format formatFor(String formatParam, Collection<MediaRange> accept
for (Format format : Format.values())
if (formatParam != null && formatParam.matches(format.queryParamString))
return format;
for (MediaRange mediaRange : acceptedTypes)
for (Format format : Format.values())
for (String mimeType : format.types)
if (mediaRange.accepts(mimeType))
return format;
return Format.JSON_LD;
if (formatParam == null || formatParam.isEmpty())
for (MediaRange mediaRange : acceptedTypes)
for (Format format : Format.values())
for (String mimeType : format.types)
if (mediaRange.accepts(mimeType))
return format;
return (formatParam == null || formatParam.isEmpty()) && acceptedTypes.isEmpty() ? Format.JSON_LD : null;
}

}
10 changes: 10 additions & 0 deletions app/controllers/HomeController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ public Result authority(String id, String format) {
return notFound("Not found: " + id);
}
Format responseFormat = Accept.formatFor(format, request().acceptedTypes());
if (responseFormat == null || responseFormat == Accept.Format.JSON_LINES
|| format != null && format.contains(":")) {
return unsupportedMediaType(String.format("Unsupported for single resource: format=%s, accept=%s", format,
request().acceptedTypes()));
}
try {
switch (responseFormat) {
case HTML: {
Expand Down Expand Up @@ -250,6 +255,11 @@ public Result gnd(String id) {

public Result search(String q, String filter, int from, int size, String format) {
Format responseFormat = Accept.formatFor(format, request().acceptedTypes());
if (responseFormat == null || Stream.of(RdfFormat.values()).map(RdfFormat::getParam)
.anyMatch(f -> f.equals(responseFormat.queryParamString))) {
return unsupportedMediaType(
String.format("Unsupported for search: format=%s, accept=%s", format, request().acceptedTypes()));
}
String queryString = (q == null || q.isEmpty()) ? "*" : q;
SearchResponse response = index.query(queryString, filter, from, size);
response().setHeader("Access-Control-Allow-Origin", "*");
Expand Down
4 changes: 4 additions & 0 deletions app/models/RdfConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ public String getName() {
return name;
}

public String getParam() {
return queryParamString;
}

public static RdfFormat of(String format) {
for (RdfFormat f : values()) {
if (f.queryParamString.equals(format)) {
Expand Down
7 changes: 4 additions & 3 deletions test/controllers/AcceptIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ public static Collection<Object[]> data() {
{ fakeRequest(GET, "/gnd/search?q=*"), /*->*/ "application/json" },
{ fakeRequest(GET, "/gnd/search?q=*&format="), /*->*/ "application/json" },
{ fakeRequest(GET, "/gnd/search?q=*&format=json"), /*->*/ "application/json" },
{ fakeRequest(GET, "/gnd/search?q=*&format=whatever"), /*->*/ "application/json" },
{ fakeRequest(GET, "/gnd/search?q=*").header("Accept", "text/plain"), /*->*/ "application/json" },
{ fakeRequest(GET, "/gnd/search?q=*&format=whatever"), /*->*/ "text/plain" },
{ fakeRequest(GET, "/gnd/search?q=*").header("Accept", "text/plain"), /*->*/ "text/plain" },
// search, bulk format: JSON lines
{ fakeRequest(GET, "/gnd/search?q=*").header("Accept", "application/x-jsonlines"), /*->*/ "application/x-jsonlines" },
{ fakeRequest(GET, "/gnd/search?format=jsonl"), /*->*/ "application/x-jsonlines" },
Expand All @@ -57,7 +57,8 @@ public static Collection<Object[]> data() {
{ fakeRequest(GET, "/gnd/118820591"), /*->*/ "application/json" },
{ fakeRequest(GET, "/gnd/118820591?format="), /*->*/ "application/json" },
{ fakeRequest(GET, "/gnd/118820591?format=json"), /*->*/ "application/json" },
{ fakeRequest(GET, "/gnd/118820591?format=whatever"), /*->*/ "application/json" },
{ fakeRequest(GET, "/gnd/118820591?format=whatever"), /*->*/ "text/plain" },
{ fakeRequest(GET, "/gnd/118820591?format=whatever").header("Accept", "text/html"), /*->*/ "text/plain" },
{ fakeRequest(GET, "/gnd/118820591").header("Accept", "text/plain"), /*->*/ "application/n-triples" },
// get, other formats as query param:
{ fakeRequest(GET, "/gnd/118820591?format=html"), /*->*/ "text/html" },
Expand Down
33 changes: 23 additions & 10 deletions test/controllers/AcceptUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@

import static org.hamcrest.CoreMatchers.startsWith;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import static play.test.Helpers.fakeRequest;

import java.util.Arrays;
import java.util.Collection;
import java.util.List;

import org.hamcrest.CoreMatchers;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

import controllers.Accept.Format;
import play.api.http.MediaRange;
import play.mvc.Http.RequestBuilder;

Expand All @@ -31,20 +34,19 @@ public class AcceptUnitTest {
@Parameters
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] {
// neither supported header nor supported format given, return
// default:
// neither header nor format given, return default:
{ fakeRequest(), null, /*->*/ "json" }, //
{ fakeRequest(), "", /*->*/ "json" }, //
{ fakeRequest(), "xml", /*->*/ "json" }, //
{ fakeRequest().header("Accept", ""), null, /*->*/ "json" }, //
{ fakeRequest().header("Accept", "application/pdf"), null, /*->*/ "json" },
// no header, just format parameter:
{ fakeRequest(), "html", /*->*/ "html" }, //
{ fakeRequest(), "json", /*->*/ "json" }, //
{ fakeRequest(), "json:preferredName", /*->*/ "json(.+)?" }, //
{ fakeRequest(), "json:preferredName", /*->*/ "json(:.+)?" }, //
{ fakeRequest(), "json:suggest", /*->*/ "json(:.+)?" }, //
{ fakeRequest(), "ttl", /*->*/ "ttl" }, //
{ fakeRequest(), "nt", /*->*/ "nt" }, //
{ fakeRequest(), "jsonl", /*->*/ "jsonl" },
{ fakeRequest(), "jsonl", /*->*/ "jsonl" }, //
{ fakeRequest(), "jsonfoo", /*->*/ null },
// supported content types, no format parameter given:
{ fakeRequest().header("Accept", "text/html"), null, /*->*/ "html" },
{ fakeRequest().header("Accept", "application/json"), null, /*->*/ "json" },
Expand All @@ -57,6 +59,10 @@ public static Collection<Object[]> data() {
{ fakeRequest().header("Accept", "application/rdf+xml"), null, /*->*/ "rdf" },
{ fakeRequest().header("Accept", "text/xml"), null, /*->*/ "rdf" },
{ fakeRequest().header("Accept", "application/x-jsonlines"), null, /*->*/ "jsonl" },
// unsupported content types:
{ fakeRequest(), "xml", /*->*/ null }, //
{ fakeRequest().header("Accept", "application/pdf"), null, /*->*/ null },
{ fakeRequest().header("Accept", "application/pdf"), "pdf", /*->*/ null },
// we pick the preferred content type:
{ fakeRequest().header("Accept", "text/html,application/json"), null, /*->*/"html" },
{ fakeRequest().header("Accept", "application/json,text/html"), null, /*->*/ "json" },
Expand All @@ -78,10 +84,17 @@ public AcceptUnitTest(RequestBuilder request, String givenFormat, String expecte
@Test
public void test() {
List<MediaRange> acceptedTypes = fakeRequest.build().acceptedTypes();
String description = String.format("resulting format for passedFormat=%s, acceptedTypes=%s", passedFormat,
acceptedTypes);
assertThat(description, Accept.formatFor(passedFormat, acceptedTypes).queryParamString,
startsWith(expectedFormat));
Format format = Accept.formatFor(passedFormat, acceptedTypes);
if (expectedFormat == null) {
assertThat(format, CoreMatchers.nullValue());
} else if (format != null) {
String actual = format.queryParamString;
String description = String.format("resulting format for passedFormat=%s, processd to %s, acceptedTypes=%s",
passedFormat, actual, acceptedTypes);
assertThat(description, actual, startsWith(expectedFormat));
} else {
fail("Format is null");
}
}

}
16 changes: 15 additions & 1 deletion test/controllers/HomeControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,24 @@ public class HomeControllerTest extends IndexTest {
@Parameters(name = "{0} -> {1}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] { //
// index
{ routes.HomeController.index().toString(), Status.OK }, //
// search
{ routes.HomeController.search("*", "", 0, 10, "json").toString(), Status.OK },
{ routes.HomeController.search("*", "", 0, 10, "jsonl").toString(), Status.OK },
{ routes.HomeController.search("*", "", 0, 10, "json:suggest").toString(), Status.OK },
{ routes.HomeController.search("*", "", 0, 10, "jsonfoo").toString(), Status.UNSUPPORTED_MEDIA_TYPE },
{ routes.HomeController.search("*", "", 0, 10, "ttl").toString(), Status.UNSUPPORTED_MEDIA_TYPE },
{ routes.HomeController.search("*", "", 0, 10, "rdf").toString(), Status.UNSUPPORTED_MEDIA_TYPE },
{ routes.HomeController.search("*", "", 0, 10, "nt").toString(), Status.UNSUPPORTED_MEDIA_TYPE },
// authority
{ routes.HomeController.authority("4791358-7", "json").toString(), Status.OK },
{ routes.HomeController.authority("abc", "json").toString(), Status.NOT_FOUND },
{ routes.HomeController.authority("1090750048", "json").toString(), Status.MOVED_PERMANENTLY } });
{ routes.HomeController.authority("1090750048", "json").toString(), Status.MOVED_PERMANENTLY },
{ routes.HomeController.authority("4791358-7", "jsonl").toString(), Status.UNSUPPORTED_MEDIA_TYPE },
{ routes.HomeController.authority("4791358-7", "jsonfoo").toString(), Status.UNSUPPORTED_MEDIA_TYPE },
{ routes.HomeController.authority("4791358-7", "json:suggest").toString(),
Status.UNSUPPORTED_MEDIA_TYPE } });
}

public HomeControllerTest(String route, int status) {
Expand Down

0 comments on commit 0e2dd40

Please sign in to comment.