Skip to content

Commit

Permalink
Adds logic to verify that we are not inadvertently adding identical p…
Browse files Browse the repository at this point in the history
…attern/method routes that call different actions.

A developer may inadvertently declare the same url pattern and method on different REST actions. The result is that the
first one found in the handler's routes array will be used. This change checks for this illegal state at startup.

If you have duplicate declarations mapping to different REST actions this will error on first launch.
  • Loading branch information
kierankelleher committed Aug 16, 2012
1 parent d582461 commit 54ae9ed
Show file tree
Hide file tree
Showing 3 changed files with 290 additions and 0 deletions.
2 changes: 2 additions & 0 deletions Frameworks/EOF/ERRest/.classpath
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path="Sources"/>
<classpathentry kind="src" path="Tests"/>
<classpathentry exported="true" kind="con" path="WOFramework/ERExtensions"/>
<classpathentry exported="true" kind="con" path="WOFramework/ERJars"/>
<classpathentry exported="true" kind="con" path="WOFramework/JavaWebObjects"/>
Expand All @@ -10,6 +11,7 @@
<classpathentry exported="true" kind="con" path="WOFramework/JavaEOControl"/>
<classpathentry exported="true" kind="con" path="WOFramework/JavaJDBCAdaptor"/>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
<classpathentry kind="con" path="org.eclipse.jdt.junit.JUNIT_CONTAINER/4"/>
<classpathentry exported="true" kind="lib" path="Libraries/commons-logging-1.1.1.jar"/>
<classpathentry exported="true" kind="lib" path="Libraries/commons-collections-3.2.1.jar"/>
<classpathentry exported="true" kind="lib" path="Libraries/ezmorph-1.0.6.jar"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;

import org.apache.commons.lang.ObjectUtils;
import org.apache.log4j.Logger;

import com.webobjects.appserver.WOAction;
Expand Down Expand Up @@ -309,6 +310,7 @@ public ERXRouteRequestHandler(NameFormat entityNameFormat) {
* the route to insert
*/
public void insertRoute(ERXRoute route) {
verifyRoute(route);
_routes.insertObjectAtIndex(route, 0);
}

Expand All @@ -322,6 +324,7 @@ public void addRoute(ERXRoute route) {
if (log.isDebugEnabled()) {
log.debug("adding route " + route);
}
verifyRoute(route);
_routes.addObject(route);
}

Expand Down Expand Up @@ -708,6 +711,57 @@ public ERXRoute routeForMethodAndPath(String method, String path, NSMutableDicti

return matchingRoute;
}

/**
* @param method
* @param pattern
* @return the first route matching <code>method</code> and <code>pattern</code>.
*/
protected ERXRoute routeForMethodAndPattern(ERXRoute.Method method, String urlPattern) {
for (ERXRoute route : _routes) {
if (route.method().equals(method) && route.routePattern().pattern().equals(urlPattern)) {
return route;
}
}
return null;
}

/**
* Checks for an existing route that matches the {@link ERXRoute.Method} and {@link ERXRoute#routePattern()} of <code>route</code> and
* yet has a different controller or action mapping.
* @param route
*/
protected void verifyRoute(ERXRoute route) {
ERXRoute duplicateRoute = routeForMethodAndPattern(route.method(), route.routePattern().pattern());
if (duplicateRoute != null) {
boolean isDifferentController = ObjectUtils.notEqual(duplicateRoute.controller(), route.controller());
boolean isDifferentAction = ObjectUtils.notEqual(duplicateRoute.action(), route.action());
if (isDifferentController || isDifferentAction) {
// We have a problem whereby two routes with same url pattern and http method map to different direct actions
StringBuilder message = new StringBuilder();
message.append("The route <");
message.append(route);
message.append("> conflicts with existing route <");
message.append(duplicateRoute);
message.append(">.");
if (isDifferentController) {
message.append(" The controller class <");
message.append(route.controller());
message.append("> is different to <");
message.append(duplicateRoute.controller());
message.append(">.");
}
if (isDifferentAction) {
message.append(" The action <");
message.append(route.action());
message.append("> is different to <");
message.append(duplicateRoute.action());
message.append(">.");
}
throw new IllegalStateException(message.toString());
}
}
}

/**
* Sets up the request userInfo for the given request for a request of the given method and path.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
package er.rest.routes;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import org.junit.Test;

import com.webobjects.appserver.WOActionResults;
import com.webobjects.appserver.WORequest;
import com.webobjects.foundation.NSArray;

import er.rest.routes.jsr311.DELETE;
import er.rest.routes.jsr311.GET;
import er.rest.routes.jsr311.POST;
import er.rest.routes.jsr311.PUT;
import er.rest.routes.jsr311.Path;
import er.rest.routes.jsr311.Paths;

public class ERXRouteRequestHandlerTest {

@Test
public void testRouteForMethodAndPattern() {
ERXRouteRequestHandler handler = new ERXRouteRequestHandler();
ERXRoute route1 = new ERXRoute("SomeEntity", "somepattern", ERXRoute.Method.Post);
handler.addRoute(route1);

// We create this object just to get the constructor-generated url pattern
ERXRoute route2 = new ERXRoute("SomeEntity", "somepattern");

ERXRoute route3 = handler.routeForMethodAndPattern(ERXRoute.Method.Post, route2.routePattern().pattern());
assertTrue(route3 != null);
assertTrue(route3 == route1);
}

// Checks error on verify conflict
@Test
public void testVerifyRouteConflict() {
ERXRouteRequestHandler handler = new ERXRouteRequestHandler();
ERXRoute route1 = new ERXRoute("SomeEntity", "somepattern");
handler.addRoute(route1);

ERXRoute route2 = new ERXRoute("SomeEntity", "somepattern", SomeController.class, "someAction");

try {
handler.verifyRoute(route2);
fail("Expected IllegalStateException");
}
catch (IllegalStateException e) {
System.out.println(e.getMessage());
}
}

// No error thrown when routes and controller/action are identical
@Test
public void testAddRouteSameNoConflict() {
ERXRouteRequestHandler handler = new ERXRouteRequestHandler();
ERXRoute route1 = new ERXRoute("SomeEntity", "somepattern", SomeController.class, "someAction");
System.out.println("route1 = " + route1);
handler.addRoute(route1);

ERXRoute route2 = new ERXRoute("SomeEntity", "somepattern", SomeController.class, "someAction");
System.out.println("route2 = " + route2);

handler.addRoute(route2);

// Two identical routes mapping to same controller and action exist.
assertEquals(handler.routes().count(), 2);
}

// Ensures declared methods are added correctly
@Test
public void testAddDeclaredMethods() {
ERXRouteRequestHandler handler = new ERXRouteRequestHandler();
handler.addDeclaredRoutes("SomeEntity", SomeController.class, true);
NSArray<ERXRoute> routes = handler.routes();
assertEquals(routes.count(), 6);

boolean didFindRoute1 = false;
boolean didFindRoute2 = false;
boolean didFindRoute3 = false;
boolean didFindRoute4 = false;
boolean didFindRouteXPut = false;
boolean didFindRouteXDefaultGet = false;
for (ERXRoute route : routes) {
System.out.println(route);
if (route.routePattern().pattern().contains("thing1")) {
assertEquals(route.method(), ERXRoute.Method.Get);
// Note that ERRest chops off trailing "Action"
assertEquals(route.action(), "some");
didFindRoute1 = true;
}
if (route.routePattern().pattern().contains("thing2")) {
assertEquals(route.method(), ERXRoute.Method.Post);
assertEquals(route.action(), "someAction2");
didFindRoute2 = true;
}
if (route.routePattern().pattern().contains("thing3")) {
assertEquals(route.method(), ERXRoute.Method.Put);
assertEquals(route.action(), "someAction3");
didFindRoute3 = true;
}
if (route.routePattern().pattern().contains("thing4")) {
assertEquals(route.method(), ERXRoute.Method.Delete);
assertEquals(route.action(), "someAction4");
didFindRoute4 = true;
}
if (route.routePattern().pattern().contains("thingX")) {
String routeAction = route.action();
if (routeAction.equals("someAction3")) {
assertEquals(route.method(), ERXRoute.Method.Put);
didFindRouteXPut = true;
} else if (routeAction.equals("someActionX")) {
assertEquals(route.method(), ERXRoute.Method.Get);
didFindRouteXDefaultGet = true;
}
}
}
assertTrue(didFindRoute1);
assertTrue(didFindRoute2);
assertTrue(didFindRoute3);
assertTrue(didFindRoute4);
assertTrue(didFindRouteXPut);
assertTrue(didFindRouteXDefaultGet);
}

// Throws error where actions have conflicting route declarations
@Test
public void testAddDeclaredMethodsWithConflicts() {
ERXRouteRequestHandler handler = new ERXRouteRequestHandler();
try {
handler.addDeclaredRoutes("FaultyEntity", FaultyController.class, true);
fail("Expected an IllegalStateException");
}
catch (IllegalStateException e) {
;
}
}

// Throws error where actions have conflicting route declarations
@Test
public void testAddDeclaredMethodsWithConflicts2() {
ERXRouteRequestHandler handler = new ERXRouteRequestHandler();
try {
handler.addDeclaredRoutes("FaultyEntity", FaultyController2.class, true);
fail("Expected an IllegalStateException");
}
catch (IllegalStateException e) {
;
}
}

// A dummy controller for testing with valid declarations
@SuppressWarnings("unused")
private static class SomeController extends ERXRouteController {
public SomeController(WORequest request) {
super(request);
}


@GET
@Path("/somethings/thing1")
public WOActionResults someAction() {
return null;
}

@POST
@Path("/somethings/thing2")
public WOActionResults someAction2() {
return null;
}

@PUT
@Paths({@Path("/somethings/thing3"), @Path("/somethings/thingX")})
public WOActionResults someAction3() {
return null;
}

@DELETE
@Path("/somethings/thing4")
public WOActionResults someAction4() {
return null;
}

// No http method declaration should default to @GET
@Path("/somethings/thingX")
public WOActionResults someActionX() {
return null;
}
}

// A dummy controller for testing
// This class deliberately has two duplicate declared route conflicts
@SuppressWarnings("unused")
private static class FaultyController extends ERXRouteController {
public FaultyController(WORequest request) {
super(request);
}

@GET
@Path("/somethings/thing1")
public WOActionResults someAction() {
return null;
}

// This will default to @GET and so should conflict with "someAction"
@Path("/somethings/thing1")
public WOActionResults someAction2() {
return null;
}
}

// A dummy controller for testing
// This class deliberately has a duplicate declared route conflict
@SuppressWarnings("unused")
private static class FaultyController2 extends ERXRouteController {
public FaultyController2(WORequest request) {
super(request);
}

@PUT
@Paths({@Path("/somethings/thing3"), @Path("/somethings/thingX")})
public WOActionResults someAction3() {
return null;
}

// This should conflict with the same method/path declaration on someAction3
@PUT
@Path("/somethings/thingX")
public WOActionResults someAction4() {
return null;
}
}
}

0 comments on commit 54ae9ed

Please sign in to comment.