#775 - Overhaul VndErrors to match spec. #776

gregturn · 2018-12-19T17:01:21Z

Handles:

single item error
multiple errors
nested errors

See https://github.com/blongden/vnd.error for details on the vnd.error spec.

Handles: * single item error * multiple errors * nested errors See https://github.com/blongden/vnd.error for details on the vnd.error spec.

schauder · 2019-01-04T12:35:22Z

src/main/java/org/springframework/hateoas/VndErrors.java

-	public VndErrors add(VndError error) {
-		this.vndErrors.add(error);
-		return this;
+	public VndErrors withMessage(String message) {


We could replace this with a Lombok @Witherannotation.

Same for other withers.

schauder · 2019-01-04T12:48:37Z

src/main/java/org/springframework/hateoas/VndErrors.java


 	/**
 	 * Creates a new {@link VndErrors} wrapper for the given {@link VndErrors}.
 	 * 
 	 * @param errors must not be {@literal null} or empty.
 	 */
 	@JsonCreator
-	public VndErrors(List<VndError> errors) {
+	public VndErrors(@JsonProperty("_embedded") List<VndError> errors, @JsonProperty("message") String message,


Both VndErrors constructors break the contract of Resources since they use the error attribute as content, but the Resources implementation doesn't know about those and uses its content attribute for the iterator.

Passing errors as an argument to super might fix this, although I'm not sure if this yields the correct JSON.

Alternatively one could change the Resources implementation to always use getContent

Testcase for demonstrating the issue:

VndErrors errors = new VndErrors(Arrays.asList(new VndError("one", "onePath", 24)), "message", 23, Collections.emptyList()); assertThat(errors).extracting(e -> e.getMessage()).containsExactlyInAnyOrder("one");

schauder · 2019-01-04T13:04:16Z

src/main/java/org/springframework/hateoas/VndErrors.java

 	 */
+	@JsonPropertyOrder({"message", "path", "logref"})
+	@Relation(collectionRelation = "errors")
+	@EqualsAndHashCode


Suggested change

@EqualsAndHashCode

@EqualsAndHashCode( callSuper=true)

We should call super in order to include the links in the comparison, especially since the Marshalling test relies on it.

schauder · 2019-01-04T13:18:22Z

src/main/java/org/springframework/hateoas/VndErrors.java

-	@Override
-	public String toString() {
-		return String.format("VndErrors[%s]", StringUtils.collectionToCommaDelimitedString(vndErrors));
+	public VndErrors withError(VndError error) {


I find this one weird for two reasons:

I would have expected it to replace the previous list of errors with a single element list containing the parameter.
I guess one can get used to either behavior, although I'd opt more for a name like addError.

What really throws me off though is the fact that it modifies the current instance and returns a new instance, which makes it behave fundamentally different to other withers except for withLink which has the same problem.

schauder · 2019-01-04T13:22:00Z

src/test/java/org/springframework/hateoas/VndErrorsMarshallingTest.java


-		jsonReference = readFile(new ClassPathResource("vnderror.json"));
-		json2Reference = readFile(new ClassPathResource("vnderror2.json"));
+		assertThat(mapper.readValue(json, VndError.class)).isEqualTo(error);


I wonder if we should use isEqualToIgnoringWhitespace in this and similar tests.

schauder · 2019-01-04T13:24:26Z

src/main/java/org/springframework/hateoas/VndErrors.java

-public class VndErrors implements Iterable<VndErrors.VndError> {
+@JsonPropertyOrder({"message", "logref", "total", "_links", "_embedded"})
+@JsonIgnoreProperties(ignoreUnknown = true)
+@EqualsAndHashCode


Although I currently don't see a way to exploid it we should call super for equals.

Suggested change

@EqualsAndHashCode

@EqualsAndHashCode(callSuper=true)

gregturn · 2019-01-15T18:25:35Z

@schauder Let's shelve this one in light of #786. Basically, this is me attempting to shove an error situation into one of the ResourceSupport types so that it will engage the serializers/deserializers.

Instead, we should have a clear API for error handling, and fit this into that instead.

This commit updates URLs to prefer the https protocol. Redirects are not followed to avoid accidentally expanding intentionally shortened URLs (i.e. if using a URL shortener). # Fixed URLs ## Fixed Success These URLs were switched to an https URL with a 2xx status. While the status was successful, your review is still recommended. * http://maven.apache.org/xsd/maven-4.0.0.xsd with 1 occurrences migrated to: https://maven.apache.org/xsd/maven-4.0.0.xsd ([https](https://maven.apache.org/xsd/maven-4.0.0.xsd) result 200). * http://www.apache.org/licenses/LICENSE-2.0 with 4 occurrences migrated to: https://www.apache.org/licenses/LICENSE-2.0 ([https](https://www.apache.org/licenses/LICENSE-2.0) result 200). * http://static.springframework.org/spring/docs/4.1.x/javadoc-api (301) with 1 occurrences migrated to: https://docs.spring.io/spring/docs/4.1.x/javadoc-api ([https](https://static.springframework.org/spring/docs/4.1.x/javadoc-api) result 301). * http://github.com/SpringSource/spring-hateoas with 1 occurrences migrated to: https://github.com/SpringSource/spring-hateoas ([https](https://github.com/SpringSource/spring-hateoas) result 301). * http://www.spring.io with 1 occurrences migrated to: https://www.spring.io ([https](https://www.spring.io) result 301). * http://docs.oracle.com/javase/6/docs/api with 1 occurrences migrated to: https://docs.oracle.com/javase/6/docs/api ([https](https://docs.oracle.com/javase/6/docs/api) result 302). * http://repo.spring.io/libs-snapshot with 2 occurrences migrated to: https://repo.spring.io/libs-snapshot ([https](https://repo.spring.io/libs-snapshot) result 302). # Ignored These URLs were intentionally ignored. * http://maven.apache.org/POM/4.0.0 with 2 occurrences * http://www.w3.org/2001/XMLSchema-instance with 1 occurrences

This commit updates URLs to prefer the https protocol. Redirects are not followed to avoid accidentally expanding intentionally shortened URLs (i.e. if using a URL shortener). # HTTP URLs that Could Not Be Fixed These URLs were unable to be fixed. Please review them to see if they can be manually resolved. * [ ] http://alps.io (200) with 1 occurrences could not be migrated: ([https](https://alps.io) result AnnotatedConnectException). * [ ] http://alps.io/spec/ (200) with 6 occurrences could not be migrated: ([https](https://alps.io/spec/) result AnnotatedConnectException). * [ ] http://amundsen.com/media-types/collection/examples/ (200) with 7 occurrences could not be migrated: ([https](https://amundsen.com/media-types/collection/examples/) result AnnotatedConnectException). * [ ] http://amundsen.com/media-types/collection/format/ (200) with 1 occurrences could not be migrated: ([https](https://amundsen.com/media-types/collection/format/) result AnnotatedConnectException). * [ ] http://stateless.co/hal_specification.html (200) with 2 occurrences could not be migrated: ([https](https://stateless.co/hal_specification.html) result SSLHandshakeException). * [ ] http://foo.com/bar (301) with 6 occurrences could not be migrated: ([https](https://foo.com/bar) result SSLHandshakeException). * [ ] http://www.csse.monash.edu.au/~damian/papers/HTML/Plurals.html (302) with 1 occurrences could not be migrated: ([https](https://www.csse.monash.edu.au/~damian/papers/HTML/Plurals.html) result SSLHandshakeException). * [ ] http://alps.io/ext/range (404) with 2 occurrences could not be migrated: ([https](https://alps.io/ext/range) result AnnotatedConnectException). # Fixed URLs ## Fixed But Review Recommended These URLs were fixed, but the https status was not OK. However, the https status was the same as the http request or http redirected to an https URL, so they were migrated. Your review is recommended. * [ ] http://tools.ietf.org/html/draft-kelly-json-hal (301) with 2 occurrences migrated to: https://tools.ietf.org/html/draft-kelly-json-hal ([https](https://tools.ietf.org/html/draft-kelly-json-hal) result ReadTimeoutException). * [ ] http://.../ (UnknownHostException) with 6 occurrences migrated to: https://.../ ([https](https://.../) result UnknownHostException). * [ ] http://examples.org/blogs/jdoe (UnknownHostException) with 5 occurrences migrated to: https://examples.org/blogs/jdoe ([https](https://examples.org/blogs/jdoe) result UnknownHostException). * [ ] http://examples.org/blogs/msmith (UnknownHostException) with 3 occurrences migrated to: https://examples.org/blogs/msmith ([https](https://examples.org/blogs/msmith) result UnknownHostException). * [ ] http://examples.org/blogs/rwilliams (UnknownHostException) with 3 occurrences migrated to: https://examples.org/blogs/rwilliams ([https](https://examples.org/blogs/rwilliams) result UnknownHostException). * [ ] http://examples.org/images/jdoe (UnknownHostException) with 5 occurrences migrated to: https://examples.org/images/jdoe ([https](https://examples.org/images/jdoe) result UnknownHostException). * [ ] http://examples.org/images/msmith (UnknownHostException) with 3 occurrences migrated to: https://examples.org/images/msmith ([https](https://examples.org/images/msmith) result UnknownHostException). * [ ] http://examples.org/images/rwilliams (UnknownHostException) with 3 occurrences migrated to: https://examples.org/images/rwilliams ([https](https://examples.org/images/rwilliams) result UnknownHostException). * [ ] http://path.to/describes (UnknownHostException) with 6 occurrences migrated to: https://path.to/describes ([https](https://path.to/describes) result UnknownHostException). * [ ] http://path.to/help (UnknownHostException) with 6 occurrences migrated to: https://path.to/help ([https](https://path.to/help) result UnknownHostException). * [ ] http://path.to/user/resource/1 (UnknownHostException) with 9 occurrences migrated to: https://path.to/user/resource/1 ([https](https://path.to/user/resource/1) result UnknownHostException). * [ ] http://acme.com/rels/foo-bar (404) with 2 occurrences migrated to: https://acme.com/rels/foo-bar ([https](https://acme.com/rels/foo-bar) result 404). * [ ] http://docs.spring.io/spring-hateoas/docs/current-SNAPSHOT/api/ (301) with 1 occurrences migrated to: https://docs.spring.io/spring-hateoas/docs/current-SNAPSHOT/api/ ([https](https://docs.spring.io/spring-hateoas/docs/current-SNAPSHOT/api/) result 404). * [ ] http://example.com/custom/deprecated (404) with 1 occurrences migrated to: https://example.com/custom/deprecated ([https](https://example.com/custom/deprecated) result 404). * [ ] http://example.com/customers/deprecated (404) with 2 occurrences migrated to: https://example.com/customers/deprecated ([https](https://example.com/customers/deprecated) result 404). * [ ] http://example.com/rels/ (404) with 2 occurrences migrated to: https://example.com/rels/ ([https](https://example.com/rels/) result 404). * [ ] http://example.com/rels/persons (404) with 1 occurrences migrated to: https://example.com/rels/persons ([https](https://example.com/rels/persons) result 404). * [ ] http://example.org/blogs/wchandry (404) with 2 occurrences migrated to: https://example.org/blogs/wchandry ([https](https://example.org/blogs/wchandry) result 404). * [ ] http://example.org/friends/ (404) with 13 occurrences migrated to: https://example.org/friends/ ([https](https://example.org/friends/) result 404). * [ ] http://example.org/friends/?queries (404) with 2 occurrences migrated to: https://example.org/friends/?queries ([https](https://example.org/friends/?queries) result 404). * [ ] http://example.org/friends/?template (404) with 2 occurrences migrated to: https://example.org/friends/?template ([https](https://example.org/friends/?template) result 404). * [ ] http://example.org/friends/jdoe (404) with 4 occurrences migrated to: https://example.org/friends/jdoe ([https](https://example.org/friends/jdoe) result 404). * [ ] http://example.org/friends/msmith (404) with 2 occurrences migrated to: https://example.org/friends/msmith ([https](https://example.org/friends/msmith) result 404). * [ ] http://example.org/friends/rss (404) with 5 occurrences migrated to: https://example.org/friends/rss ([https](https://example.org/friends/rss) result 404). * [ ] http://example.org/friends/rwilliams (404) with 2 occurrences migrated to: https://example.org/friends/rwilliams ([https](https://example.org/friends/rwilliams) result 404). * [ ] http://example.org/friends/search (404) with 2 occurrences migrated to: https://example.org/friends/search ([https](https://example.org/friends/search) result 404). * [ ] http://example.org/images/wchandry (404) with 2 occurrences migrated to: https://example.org/images/wchandry ([https](https://example.org/images/wchandry) result 404). * [ ] http://example.org/samples/full/doc.html (404) with 3 occurrences migrated to: https://example.org/samples/full/doc.html ([https](https://example.org/samples/full/doc.html) result 404). * [ ] http://www.example.com/rels/ (404) with 1 occurrences migrated to: https://www.example.com/rels/ ([https](https://www.example.com/rels/) result 404). ## Fixed Success These URLs were switched to an https URL with a 2xx status. While the status was successful, your review is still recommended. * [ ] http://docs.spring.io/spring-hateoas/docs/current/reference/html/ with 1 occurrences migrated to: https://docs.spring.io/spring-hateoas/docs/current/reference/html/ ([https](https://docs.spring.io/spring-hateoas/docs/current/reference/html/) result 200). * [ ] http://docs.spring.io/spring-hateoas/docs/current/reference/pdf/spring-hateoas-reference.pdf with 1 occurrences migrated to: https://docs.spring.io/spring-hateoas/docs/current/reference/pdf/spring-hateoas-reference.pdf ([https](https://docs.spring.io/spring-hateoas/docs/current/reference/pdf/spring-hateoas-reference.pdf) result 200). * [ ] http://en.wikipedia.org/wiki/HATEOAS with 2 occurrences migrated to: https://en.wikipedia.org/wiki/HATEOAS ([https](https://en.wikipedia.org/wiki/HATEOAS) result 200). * [ ] http://example.com?name=foo with 1 occurrences migrated to: https://example.com?name=foo ([https](https://example.com?name=foo) result 200). * [ ] http://mamund.site44.com/misc/hal-forms/ with 1 occurrences migrated to: https://mamund.site44.com/misc/hal-forms/ ([https](https://mamund.site44.com/misc/hal-forms/) result 200). * [ ] http://projects.spring.io/spring-hateoas/ with 2 occurrences migrated to: https://projects.spring.io/spring-hateoas/ ([https](https://projects.spring.io/spring-hateoas/) result 200). * [ ] http://tools.ietf.org/html/draft-kelly-json-hal-05 with 1 occurrences migrated to: https://tools.ietf.org/html/draft-kelly-json-hal-05 ([https](https://tools.ietf.org/html/draft-kelly-json-hal-05) result 200). * [ ] http://tools.ietf.org/html/rfc6570 with 1 occurrences migrated to: https://tools.ietf.org/html/rfc6570 ([https](https://tools.ietf.org/html/rfc6570) result 200). * [ ] http://tools.ietf.org/html/rfc7239 with 1 occurrences migrated to: https://tools.ietf.org/html/rfc7239 ([https](https://tools.ietf.org/html/rfc7239) result 200). * [ ] http://www.example.com with 2 occurrences migrated to: https://www.example.com ([https](https://www.example.com) result 200). * [ ] http://www.iana.org/assignments/link-relations/link-relations.xhtml with 2 occurrences migrated to: https://www.iana.org/assignments/link-relations/link-relations.xhtml ([https](https://www.iana.org/assignments/link-relations/link-relations.xhtml) result 200). * [ ] http://amazon.com with 4 occurrences migrated to: https://amazon.com ([https](https://amazon.com) result 301). * [ ] http://contributor-covenant.org with 1 occurrences migrated to: https://contributor-covenant.org ([https](https://contributor-covenant.org) result 301). * [ ] http://contributor-covenant.org/version/1/3/0/ with 1 occurrences migrated to: https://contributor-covenant.org/version/1/3/0/ ([https](https://contributor-covenant.org/version/1/3/0/) result 301). * [ ] http://www.w3.org/TR/curie with 1 occurrences migrated to: https://www.w3.org/TR/curie ([https](https://www.w3.org/TR/curie) result 301). * [ ] http://tools.ietf.org/html/rfc5988=section-4 with 1 occurrences migrated to: https://tools.ietf.org/html/rfc5988=section-4 ([https](https://tools.ietf.org/html/rfc5988=section-4) result 302). * [ ] http://www.springsource.org/download with 1 occurrences migrated to: https://www.springsource.org/download ([https](https://www.springsource.org/download) result 302). # Ignored These URLs were intentionally ignored. * http://barfoo:8888 with 1 occurrences * http://foobar with 1 occurrences * http://foobar:8088 with 1 occurrences * http://foobarhost/ with 1 occurrences * http://foobarhost:9090/ with 1 occurrences * http://localhost with 7 occurrences * http://localhost/ with 1 occurrences * http://localhost/employees with 21 occurrences * http://localhost/employees/0 with 8 occurrences * http://localhost/employees/1 with 2 occurrences * http://localhost/employees/2 with 7 occurrences * http://localhost/sample/1/foo with 1 occurrences * http://localhost/sample/2/bar with 1 occurrences * http://localhost/something/bar/foo with 1 occurrences * http://localhost:8080/api/ with 1 occurrences * http://localhost:8080/foo with 2 occurrences * http://localhost:8080/my/custom/location with 2 occurrences * http://localhost:8080/rels with 1 occurrences * http://localhost:8080/rels/ with 6 occurrences * http://localhost:8080/something with 4 occurrences * http://localhost:8080/test?page=0&filter=foo,bar with 2 occurrences * http://localhost:8080/your-app with 1 occurrences * http://localhost:8080/your-app/people with 1 occurrences * http://myhost/people with 4 occurrences * http://myhost/person/1 with 1 occurrences * http://myhost/person/1/orders with 1 occurrences * http://proxy1:1443 with 1 occurrences * http://somethingDifferent with 1 occurrences * http://www.w3.org/2005/Atom with 2 occurrences

This commit updates URLs to prefer the https protocol. Redirects are not followed to avoid accidentally expanding intentionally shortened URLs (i.e. if using a URL shortener). # Fixed URLs ## Fixed Success These URLs were switched to an https URL with a 2xx status. While the status was successful, your review is still recommended. * [ ] http://www.springframework.org/schema/beans/spring-beans.xsd with 1 occurrences migrated to: https://www.springframework.org/schema/beans/spring-beans.xsd ([https](https://www.springframework.org/schema/beans/spring-beans.xsd) result 200). * [ ] http://www.springframework.org/schema/context/spring-context.xsd with 1 occurrences migrated to: https://www.springframework.org/schema/context/spring-context.xsd ([https](https://www.springframework.org/schema/context/spring-context.xsd) result 200). # Ignored These URLs were intentionally ignored. * http://maven.apache.org/POM/4.0.0 with 2 occurrences * http://www.springframework.org/schema/beans with 2 occurrences * http://www.springframework.org/schema/context with 2 occurrences * http://www.w3.org/2001/XMLSchema-instance with 2 occurrences * http://www.w3.org/2005/Atom with 1 occurrences

This commit updates URLs to prefer the https protocol. Redirects are not followed to avoid accidentally expanding intentionally shortened URLs (i.e. if using a URL shortener). # Fixed URLs ## Fixed Success These URLs were switched to an https URL with a 2xx status. While the status was successful, your review is still recommended. * [ ] http://www.apache.org/licenses/ with 2 occurrences migrated to: https://www.apache.org/licenses/ ([https](https://www.apache.org/licenses/) result 200). * [ ] http://www.apache.org/licenses/LICENSE-2.0 with 195 occurrences migrated to: https://www.apache.org/licenses/LICENSE-2.0 ([https](https://www.apache.org/licenses/LICENSE-2.0) result 200).

gregturn added the process: in progress label Dec 19, 2018

gregturn requested review from odrotbohm and schauder December 19, 2018 17:02

#775 - Overhaul VndErrors to match spec.

9350acb

Handles: * single item error * multiple errors * nested errors See https://github.com/blongden/vnd.error for details on the vnd.error spec.

gregturn force-pushed the feature/vnd.error branch from 286f5b2 to 9350acb Compare December 19, 2018 17:03

gregturn self-assigned this Jan 2, 2019

gregturn added type: new feature specification labels Jan 2, 2019

schauder requested changes Jan 4, 2019

View reviewed changes

Spring Operator added 4 commits March 18, 2019 17:55

odrotbohm added type: enhancement and removed type: new feature labels Dec 3, 2019

gregturn closed this Dec 10, 2019

gregturn deleted the feature/vnd.error branch December 10, 2019 15:56

gregturn removed the process: in progress label Dec 10, 2019

gregturn added this to the 1.1.0.M1 milestone Dec 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#775 - Overhaul VndErrors to match spec. #776

#775 - Overhaul VndErrors to match spec. #776

gregturn commented Dec 19, 2018

schauder Jan 4, 2019

schauder Jan 4, 2019

schauder Jan 4, 2019

schauder Jan 4, 2019

schauder Jan 4, 2019

schauder Jan 4, 2019

schauder Jan 4, 2019

gregturn commented Jan 15, 2019

#775 - Overhaul VndErrors to match spec. #776

#775 - Overhaul VndErrors to match spec. #776

Conversation

gregturn commented Dec 19, 2018

schauder Jan 4, 2019

Choose a reason for hiding this comment

schauder Jan 4, 2019

Choose a reason for hiding this comment

schauder Jan 4, 2019

Choose a reason for hiding this comment

schauder Jan 4, 2019

Choose a reason for hiding this comment

schauder Jan 4, 2019

Choose a reason for hiding this comment

schauder Jan 4, 2019

Choose a reason for hiding this comment

schauder Jan 4, 2019

Choose a reason for hiding this comment

gregturn commented Jan 15, 2019