From 0bd6657dd5990a44f27adc637dcd54b1a4950535 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 23 Jan 2013 16:18:40 +0100 Subject: [PATCH] #43 - Improved contract in ControllerEntityLinks. Fixed StackOverflowError in ControllerEntityLinks by delegating to the proper methods now. Clarified contract in EntityLinks interface. Added further unit tests to make sure the implementation behaves like the interface specifies. Fixes #43. --- .../springframework/hateoas/EntityLinks.java | 31 +++++++--- .../hateoas/core/ControllerEntityLinks.java | 19 ++++-- .../hateoas/core/DelegatingEntityLinks.java | 8 +-- .../core/ControllerEntityLinksUnitTest.java | 58 +++++++++++++++++-- .../core/DelegatingEntityLinksUnitTest.java | 4 +- 5 files changed, 94 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/EntityLinks.java b/src/main/java/org/springframework/hateoas/EntityLinks.java index 743f219e..f66ad069 100644 --- a/src/main/java/org/springframework/hateoas/EntityLinks.java +++ b/src/main/java/org/springframework/hateoas/EntityLinks.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2013 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. @@ -18,7 +18,10 @@ package org.springframework.hateoas; import org.springframework.plugin.core.Plugin; /** - * Accessor to links pointing to controllers backing an entity type. + * Accessor to links pointing to controllers backing an entity type. The {@link IllegalArgumentException} potentially + * thrown by the declared methods will only appear if the {@link #supports(Class)} method has returned {@literal false} + * and the method has been invoked anyway, i.e. if {@link #supports(Class)} returns {@literal true} it's safe to invoke + * the interface methods an the exception will never be thrown. * * @author Oliver Gierke */ @@ -30,6 +33,7 @@ public interface EntityLinks extends Plugin> { * * @param type the entity type to point to, must not be {@literal null}. * @return the {@link LinkBuilder} pointing to the collection resource. Will never be {@literal null}. + * @throws IllegalArgumentException in case the given type is unknown the entity links infrastructure. */ LinkBuilder linkFor(Class type); @@ -38,7 +42,8 @@ public interface EntityLinks extends Plugin> { * given parameters into the URI template the backing controller is mapped to. * * @param type the entity type to point to, must not be {@literal null}. - * @return the {@link LinkBuilder} pointing to the collection resource. Will never be {@literal null}. + * @return the {@link LinkBuilder} pointing to the collection resource. + * @throws IllegalArgumentException in case the given type is unknown the entity links infrastructure. */ LinkBuilder linkFor(Class type, Object... parameters); @@ -48,7 +53,9 @@ public interface EntityLinks extends Plugin> { * * @param type the entity type to point to, must not be {@literal null}. * @param id the id of the object of the handed type, {@link Identifiable}s will be unwrapped. - * @return + * @return the {@link LinkBuilder} pointing to the single resource identified by the given type and id. Will never be + * {@literal null}. + * @throws IllegalArgumentException in case the given type is unknown the entity links infrastructure. */ LinkBuilder linkForSingleResource(Class type, Object id); @@ -57,7 +64,8 @@ public interface EntityLinks extends Plugin> { * * @see #linkForSingleResource(Class, Object) * @param entity the entity type to point to, must not be {@literal null}. - * @return + * @return the {@link LinkBuilder} pointing the given entity. Will never be {@literal null}. + * @throws IllegalArgumentException in case the type of the given entity is unknown the entity links infrastructure. */ LinkBuilder linkForSingleResource(Identifiable entity); @@ -66,7 +74,9 @@ public interface EntityLinks extends Plugin> { * determined by the implementation class and should be defaulted to {@link Link#REL_SELF}. * * @param type the entity type to point to, must not be {@literal null}. - * @return + * @return the {@link Link} pointing to the collection resource exposed for the given entity. Will never be + * {@literal null}. + * @throws IllegalArgumentException in case the given type is unknown the entity links infrastructure. */ Link linkToCollectionResource(Class type); @@ -75,8 +85,10 @@ public interface EntityLinks extends Plugin> { * link will be determined by the implementation class and should be defaulted to {@link Link#REL_SELF}. * * @param type the entity type to point to, must not be {@literal null}. - * @param id - * @return + * @param id the identifier of the entity of the given type + * @return the {@link Link} pointing to the resource exposed for the entity with the given type and id. Will never be + * {@literal null}. + * @throws IllegalArgumentException in case the given type is unknown the entity links infrastructure. */ Link linkToSingleResource(Class type, Object id); @@ -85,7 +97,8 @@ public interface EntityLinks extends Plugin> { * determined by the implementation class and should be defaulted to {@link Link#REL_SELF}. * * @param entity the entity type to point to, must not be {@literal null}. - * @return + * @return the {@link Link} pointing to the resource exposed for the given entity. Will never be {@literal null}. + * @throws IllegalArgumentException in case the type of the given entity is unknown the entity links infrastructure. */ Link linkToSingleResource(Identifiable entity); } diff --git a/src/main/java/org/springframework/hateoas/core/ControllerEntityLinks.java b/src/main/java/org/springframework/hateoas/core/ControllerEntityLinks.java index c2326a2e..42d9771f 100644 --- a/src/main/java/org/springframework/hateoas/core/ControllerEntityLinks.java +++ b/src/main/java/org/springframework/hateoas/core/ControllerEntityLinks.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2013 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. @@ -95,9 +95,7 @@ public class ControllerEntityLinks extends AbstractEntityLinks { */ @Override public LinkBuilder linkFor(Class entity) { - - Class controllerClass = entityToController.get(entity); - return linkBuilderFactory.linkTo(controllerClass); + return linkFor(entity, new Object[0]); } /* @@ -106,7 +104,18 @@ public class ControllerEntityLinks extends AbstractEntityLinks { */ @Override public LinkBuilder linkFor(Class entity, Object... parameters) { - return linkFor(entity, parameters); + + Assert.notNull(entity); + + Class controllerType = entityToController.get(entity); + + if (controllerType == null) { + throw new IllegalArgumentException(String.format( + "Type %s is not managed by a Spring MVC controller. Make sure you have annotated your controller with %s!", + entity.getName(), ExposesResourceFor.class.getName())); + } + + return linkBuilderFactory.linkTo(controllerType, parameters); } /* diff --git a/src/main/java/org/springframework/hateoas/core/DelegatingEntityLinks.java b/src/main/java/org/springframework/hateoas/core/DelegatingEntityLinks.java index e21ef110..7422ee87 100644 --- a/src/main/java/org/springframework/hateoas/core/DelegatingEntityLinks.java +++ b/src/main/java/org/springframework/hateoas/core/DelegatingEntityLinks.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2013 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. @@ -88,8 +88,8 @@ public class DelegatingEntityLinks extends AbstractEntityLinks { } /** - * Returns the plugin for the given type or throws an {@link IllegalStateException} if no delegate {@link EntityLinks} - * can be found. + * Returns the plugin for the given type or throws an {@link IllegalArgumentException} if no delegate + * {@link EntityLinks} can be found. * * @param type must not be {@literal null}. * @return @@ -99,7 +99,7 @@ public class DelegatingEntityLinks extends AbstractEntityLinks { EntityLinks plugin = delegates.getPluginFor(type); if (plugin == null) { - throw new IllegalStateException(String.format( + throw new IllegalArgumentException(String.format( "Cannot determine link for %s! No EntityLinks instance found supporting the domain type!", type.getName())); } diff --git a/src/test/java/org/springframework/hateoas/core/ControllerEntityLinksUnitTest.java b/src/test/java/org/springframework/hateoas/core/ControllerEntityLinksUnitTest.java index 0dcf381d..5bdb4d04 100644 --- a/src/test/java/org/springframework/hateoas/core/ControllerEntityLinksUnitTest.java +++ b/src/test/java/org/springframework/hateoas/core/ControllerEntityLinksUnitTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2013 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. @@ -17,22 +17,25 @@ package org.springframework.hateoas.core; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; +import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; +import static org.springframework.hateoas.mvc.ControllerLinkBuilder.*; import java.util.Arrays; +import org.hamcrest.CoreMatchers; 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; import org.mockito.runners.MockitoJUnitRunner; import org.springframework.hateoas.EntityLinks; +import org.springframework.hateoas.ExposesResourceFor; import org.springframework.hateoas.LinkBuilder; import org.springframework.hateoas.LinkBuilderFactory; -import org.springframework.hateoas.ExposesResourceFor; import org.springframework.hateoas.TestUtils; -import org.springframework.hateoas.mvc.ControllerLinkBuilder; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.RequestMapping; @@ -60,7 +63,6 @@ public class ControllerEntityLinksUnitTest extends TestUtils { @Test(expected = IllegalArgumentException.class) public void rejectsNullControllerList() { - new ControllerEntityLinks(null, linkBuilderFactory); } @@ -76,14 +78,47 @@ public class ControllerEntityLinksUnitTest extends TestUtils { @SuppressWarnings("unchecked") public void registersControllerForEntity() { - when(linkBuilderFactory.linkTo(SampleController.class)).thenReturn( - ControllerLinkBuilder.linkTo(SampleController.class)); + when(linkBuilderFactory.linkTo(SampleController.class, new Object[0])).thenReturn(linkTo(SampleController.class)); EntityLinks links = new ControllerEntityLinks(Arrays.asList(SampleController.class), linkBuilderFactory); assertThat(links.supports(Person.class), is(true)); assertThat(links.linkFor(Person.class), is(notNullValue())); } + /** + * @see #43 + */ + @Test + @SuppressWarnings("unchecked") + public void returnsLinkBuilderForParameterizedController() { + + when(linkBuilderFactory.linkTo(eq(ControllerWithParameters.class), Mockito.any(Object[].class))).thenReturn( + linkTo(ControllerWithParameters.class, "1")); + + ControllerEntityLinks links = new ControllerEntityLinks(Arrays.asList(ControllerWithParameters.class), + linkBuilderFactory); + LinkBuilder builder = links.linkFor(Order.class, "1"); + + assertThat(builder.withSelfRel().getHref(), CoreMatchers.endsWith("/person/1")); + } + + @Test + @SuppressWarnings("unchecked") + public void rejectsUnmanagedEntity() { + + EntityLinks links = new ControllerEntityLinks( + Arrays.asList(SampleController.class, ControllerWithParameters.class), linkBuilderFactory); + + assertThat(links.supports(Person.class), is(true)); + assertThat(links.supports(Order.class), is(true)); + assertThat(links.supports(SampleController.class), is(false)); + + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage(SampleController.class.getName()); + thrown.expectMessage(ExposesResourceFor.class.getName()); + links.linkFor(SampleController.class); + } + @Controller @ExposesResourceFor(Person.class) @RequestMapping("/person") @@ -91,6 +126,13 @@ public class ControllerEntityLinksUnitTest extends TestUtils { } + @Controller + @ExposesResourceFor(Order.class) + @RequestMapping("/person/{id}") + static class ControllerWithParameters { + + } + static class InvalidController { } @@ -98,4 +140,8 @@ public class ControllerEntityLinksUnitTest extends TestUtils { static class Person { } + + static class Order { + + } } diff --git a/src/test/java/org/springframework/hateoas/core/DelegatingEntityLinksUnitTest.java b/src/test/java/org/springframework/hateoas/core/DelegatingEntityLinksUnitTest.java index ae20cb95..909d3afc 100644 --- a/src/test/java/org/springframework/hateoas/core/DelegatingEntityLinksUnitTest.java +++ b/src/test/java/org/springframework/hateoas/core/DelegatingEntityLinksUnitTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2013 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. @@ -63,7 +63,7 @@ public class DelegatingEntityLinksUnitTest extends TestUtils { @Test public void throwsExceptionForUnsupportedClass() { - exception.expect(IllegalStateException.class); + exception.expect(IllegalArgumentException.class); exception.expectMessage(String.class.getName()); EntityLinks links = new DelegatingEntityLinks(SimplePluginRegistry., EntityLinks> create());