Skip to content

Commit

Permalink
* [#998] fix(router): allow to have space when defining static args i…
Browse files Browse the repository at this point in the history 
…n routes

# Conflicts:
#	framework/src/play/mvc/Router.java
  • Loading branch information
@xael-fry
xael-fry committed Oct 25, 2016
1 parent b2685c0 commit ae92421
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 28 deletions.
44 changes: 18 additions & 26 deletions framework/src/play/mvc/Router.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;

import org.apache.commons.lang.StringUtils;
Expand All @@ -27,7 +28,6 @@
import play.utils.Utils;
import play.vfs.VirtualFile;

import java.util.concurrent.ConcurrentHashMap;
/**
* The router matches HTTP requests to action invocations
*/
Expand All @@ -36,8 +36,7 @@ public class Router {
static Pattern routePattern = new Pattern(
"^({method}GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD|WS|\\*)[(]?({headers}[^)]*)(\\))?\\s+({path}.*/[^\\s]*)\\s+({action}[^\\s(]+)({params}.+)?(\\s*)$");
/**
* Pattern used to locate a method override instruction in
* request.querystring
* Pattern used to locate a method override instruction in request.querystring
*/
static Pattern methodOverride = new Pattern("^.*x-http-method-override=({method}GET|PUT|POST|PATCH|DELETE).*$");
/**
Expand All @@ -49,8 +48,8 @@ public class Router {
* Parse the routes file. This is called at startup.
*
* @param prefix
* The prefix that the path of all routes in this route file
* start with. This prefix should not end with a '/' character.
* The prefix that the path of all routes in this route file start with. This prefix should not end with
* a '/' character.
*/
public static void load(String prefix) {
routes.clear();
Expand All @@ -62,16 +61,14 @@ public static void load(String prefix) {
}

/**
* This one can be called to add new route. Last added is first in the route
* list.
* This one can be called to add new route. Last added is first in the route list.
*/
public static void prependRoute(String method, String path, String action, String headers) {
prependRoute(method, path, action, null, headers);
}

/**
* This one can be called to add new route. Last added is first in the route
* list.
* This one can be called to add new route. Last added is first in the route list.
*/
public static void prependRoute(String method, String path, String action) {
prependRoute(method, path, action, null, null);
Expand Down Expand Up @@ -123,9 +120,8 @@ public static void addRoute(String method, String path, String action, String pa
}

/**
* This is used internally when reading the route file. The order the routes
* are added matters and we want the method to append the routes to the
* list.
* This is used internally when reading the route file. The order the routes are added matters and we want the
* method to append the routes to the list.
*/
public static void appendRoute(String method, String path, String action, String params, String headers, String sourceFile, int line) {
routes.add(getRoute(method, path, action, params, headers, sourceFile, line));
Expand Down Expand Up @@ -159,14 +155,13 @@ public static void prependRoute(String method, String path, String action, Strin
}

/**
* Parse a route file. If an action starts with <i>"plugin:name"</i>,
* replace that route by the ones declared in the plugin route file denoted
* by that <i>name</i>, if found.
* Parse a route file. If an action starts with <i>"plugin:name"</i>, replace that route by the ones declared in the
* plugin route file denoted by that <i>name</i>, if found.
*
* @param routeFile
* @param prefix
* The prefix that the path of all routes in this route file
* start with. This prefix should not end with a '/' character.
* The prefix that the path of all routes in this route file start with. This prefix should not end with
* a '/' character.
*/
static void parse(VirtualFile routeFile, String prefix) {
String fileAbsolutePath = routeFile.getRealFile().getAbsolutePath();
Expand Down Expand Up @@ -222,12 +217,11 @@ static void parse(String content, String prefix, String fileAbsolutePath) {
* In PROD mode and if the routes are already loaded, this does nothing.
* <p>
* <p>
* In DEV mode, this checks each routes file's "last modified" time to see
* if the routes need updated.
* In DEV mode, this checks each routes file's "last modified" time to see if the routes need updated.
*
* @param prefix
* The prefix that the path of all routes in this route file
* start with. This prefix should not end with a '/' character.
* The prefix that the path of all routes in this route file start with. This prefix should not end with
* a '/' character.
*/
public static void detectChanges(String prefix) {
if (Play.mode == Mode.PROD && lastLoading > 0) {
Expand Down Expand Up @@ -648,8 +642,7 @@ public static class ActionDefinition {
*/
public String action;
/**
* @todo - are these the required args in the routing file, or the query
* string in a request?
* @todo - are these the required args in the routing file, or the query string in a request?
*/
public Map<String, Object> args;

Expand Down Expand Up @@ -849,7 +842,7 @@ public void addParams(String params) {
}
params = params.substring(1, params.length() - 1);
for (String param : params.split(",")) {
Matcher matcher = paramPattern.matcher(param);
Matcher matcher = paramPattern.matcher(param.trim());
if (matcher.matches()) {
staticArgs.put(matcher.group(1), matcher.group(2));
} else {
Expand Down Expand Up @@ -897,8 +890,7 @@ public Map<String, String> matches(String method, String path, String accept) {
* @param method
* GET/POST/etc.
* @param path
* Part after domain and before query-string. Starts with a
* "/".
* Part after domain and before query-string. Starts with a "/".
* @param accept
* Format, e.g. html.
* @param domain
Expand Down
3 changes: 2 additions & 1 deletion samples-and-tests/just-test-cases/conf/routes
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ GET /re/{<[A-Z]{2,4}>re} Application.ok
GET /re/{<[0-9]%2F[0-9]>re}/rev Application.revRoute
GET /re/{<[a-z0-9]+%2F[a-z,0-9]+>re}/rev Application.revRoute
GET /re/{<[A-Za-z0-9\/%]+>re}/rev Application.revRoute
GET /re/urlWithArgumentInRoutesNoSpace Application.ressourceWithoutSpecialCharacters(appId:'param1',verId:'param2')
GET /re/urlWithArgumentInRoutesWithSpace Application.ressourceWithoutSpecialCharacters( appId:'param1', verId:'param2' )
GET /re/{appId}/{verId} Application.ressourceWithoutSpecialCharacters
GET /re/{<[a-z0-9]+[\/]??(%2F)??[a-z,0-9]+>appId}/{<[a-z0-9\/%F]+>verId} Application.ressourceWithSpecialCharacters


GET /{lucky}/doIt Application.showIt
GET /withQueryParam Application.withQueryParam

Expand Down
10 changes: 9 additions & 1 deletion samples-and-tests/just-test-cases/test/routing.test.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,15 @@
open('/re/api01%2F02/01%2F01')
assertTextPresent('OK[ressourceWithoutSpecialCharacters]: appId=api01/02 verId=01/01')
assertTextPresent('URL: /re/api01%2F02/01%2F01')


open('/re/urlWithArgumentInRoutesNoSpace')
assertTextPresent('OK[ressourceWithoutSpecialCharacters]: appId=param1 verId=param2')
assertTextPresent('URL: /re/urlWithArgumentInRoutesNoSpace')

open('/re/urlWithArgumentInRoutesWithSpace')
assertTextPresent('OK[ressourceWithoutSpecialCharacters]: appId=param1 verId=param2')
assertTextPresent('URL: /re/urlWithArgumentInRoutesNoSpace')

// [#1685] - show (reverse routing of param with '/' in it)
open('/re/1%2F2/rev')
assertTextPresent('OK[revRoute]: 1/2')
Expand Down

0 comments on commit ae92421

Please sign in to comment.