DATAREST-377 - UriStringDeserializer now falls back to null in case of an empty or null source.

UriStringDeserializer now explicitly checks the source JSON value to be null or empty and returns null right away. Previously it handed the value to UriTemplate without any further validation which caused assertion errors in case of an empty or null source.

We now also explicitly catch the IllegalArgumentException thrown from the UriTemplate to make sure that any failure creating a URI (usually from piping a JSON object into the serializer where a URI is expected) is rejected with an appropriate error message.

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/PersistentEntityJackson2Module.java

@@ -41,6 +41,7 @@ import org.springframework.hateoas.Resource;
 import org.springframework.hateoas.Resources;
 import org.springframework.hateoas.UriTemplate;
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 
 import com.fasterxml.jackson.annotation.JsonUnwrapped;
 import com.fasterxml.jackson.core.JsonGenerationException;
@@ -367,6 +368,7 @@ public class PersistentEntityJackson2Module extends SimpleModule {
 	static class UriStringDeserializer extends StdDeserializer<Object> {
 
 		private static final long serialVersionUID = -2175900204153350125L;
+		private static final String UNEXPECTED_VALUE = "Expected URI cause property %s points to the managed domain type!";
 
 		private final PersistentProperty<?> property;
 		private final UriToEntityConverter converter;
@@ -393,10 +395,20 @@ public class PersistentEntityJackson2Module extends SimpleModule {
 		@Override
 		public Object deserialize(JsonParser jp, DeserializationContext ctxt) throws IOException, JsonProcessingException {
 
-			URI uri = new UriTemplate(jp.getValueAsString()).expand();
-			TypeDescriptor typeDescriptor = TypeDescriptor.valueOf(property.getActualType());
+			String source = jp.getValueAsString();
 
-			return converter.convert(uri, URI_DESCRIPTOR, typeDescriptor);
+			if (!StringUtils.hasText(source)) {
+				return null;
+			}
+
+			try {
+				URI uri = new UriTemplate(source).expand();
+				TypeDescriptor typeDescriptor = TypeDescriptor.valueOf(property.getActualType());
+
+				return converter.convert(uri, URI_DESCRIPTOR, typeDescriptor);
+			} catch (IllegalArgumentException o_O) {
+				throw ctxt.weirdStringException(source, URI.class, String.format(UNEXPECTED_VALUE, property));
+			}
 		}
 	}
 

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/UriStringDeserializerUnitTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,13 +15,17 @@
  */
 package org.springframework.data.rest.webmvc.json;
 
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
 import static org.mockito.Matchers.*;
 import static org.mockito.Mockito.*;
 
 import java.net.URI;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.Mockito;
@@ -30,9 +34,12 @@ import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.rest.core.UriToEntityConverter;
 import org.springframework.data.rest.webmvc.json.PersistentEntityJackson2Module.UriStringDeserializer;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonMappingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 
 /**
  * Unit tests for {@link UriStringDeserializer}.
@@ -42,17 +49,24 @@ import com.fasterxml.jackson.databind.DeserializationContext;
 @RunWith(MockitoJUnitRunner.class)
 public class UriStringDeserializerUnitTests {
 
+	public @Rule ExpectedException exception = ExpectedException.none();
+
 	@Mock UriToEntityConverter converter;
 	@Mock PersistentProperty<?> property;
 
 	@Mock JsonParser parser;
-	@Mock DeserializationContext context;
+	DeserializationContext context;
 
 	UriStringDeserializer deserializer;
 
 	@Before
 	public void setUp() {
+
 		this.deserializer = new UriStringDeserializer(property, converter);
+
+		// Need to hack the context as there's virtually no way wo set up a combined parser and context easily
+		this.context = new ObjectMapper().getDeserializationContext();
+		ReflectionTestUtils.setField(context, "_parser", parser);
 	}
 
 	/**
@@ -71,15 +85,43 @@ public class UriStringDeserializerUnitTests {
 		assertConverterInvokedWithUri("/foo/32{?projection}", URI.create("/foo/32"));
 	}
 
+	/**
+	 * @see DATAREST-377
+	 */
+	@Test
+	public void returnsNullUriIfSourceIsEmptyOrNull() throws Exception {
+
+		assertThat(invokeConverterWith(""), is(nullValue()));
+		assertThat(invokeConverterWith(null), is(nullValue()));
+	}
+
+	/**
+	 * @see DATAREST-377
+	 */
+	@Test
+	public void rejectsNonUriValue() throws Exception {
+
+		exception.expect(JsonMappingException.class);
+		exception.expectMessage("managed domain type");
+
+		invokeConverterWith("{ \"foo\" : \"bar\" }");
+	}
+
 	@SuppressWarnings({ "unchecked", "rawtypes" })
-	private void assertConverterInvokedWithUri(String source, URI expected) throws Exception {
+	private Object invokeConverterWith(String source) throws Exception {
 
 		when(property.getActualType()).thenReturn((Class) Object.class);
 		when(parser.getValueAsString()).thenReturn(source);
+		when(parser.getText()).thenReturn(source);
 
-		deserializer.deserialize(parser, context);
+		return deserializer.deserialize(parser, context);
+	}
 
-		verify(converter)
-				.convert(eq(expected), Mockito.any(TypeDescriptor.class), eq(TypeDescriptor.valueOf(Object.class)));
+	private void assertConverterInvokedWithUri(String source, URI expected) throws Exception {
+
+		invokeConverterWith(source);
+
+		verify(converter).convert(eq(expected), Mockito.any(TypeDescriptor.class),
+				eq(TypeDescriptor.valueOf(Object.class)));
 	}
 }