Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support verb suffix for HTTP endpoint #5613

Merged
merged 19 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,15 @@ private ParameterizedPathMapping(String prefix, String pathPattern) {
skeletonJoiner.add(token);
continue;
}
// The paramName should not include colons.
// TODO: Implement param options expressed like `{bar:type=int,range=[0,10]}`.
minwoox marked this conversation as resolved.
Show resolved Hide resolved
final String colonRemovedParamName = removeColonAndFollowing(paramName);
final boolean captureRestPathMatching = isCaptureRestPathMatching(token);
final int paramNameIdx = paramNames.indexOf(paramName);
final int paramNameIdx = paramNames.indexOf(colonRemovedParamName);
if (paramNameIdx < 0) {
// If the given token appeared first time, add it to the set and
// replace it with a capturing group expression in regex.
paramNames.add(paramName);
paramNames.add(colonRemovedParamName);
if (captureRestPathMatching) {
patternJoiner.add("(.*)");
} else {
Expand All @@ -159,7 +162,7 @@ private ParameterizedPathMapping(String prefix, String pathPattern) {
patternJoiner.add("\\" + (paramNameIdx + 1));
}

normalizedPatternJoiner.add((captureRestPathMatching ? ":*" : ':') + paramName);
normalizedPatternJoiner.add((captureRestPathMatching ? ":*" : ':') + colonRemovedParamName);
skeletonJoiner.add(captureRestPathMatching ? "*" : ":");
}

Expand Down Expand Up @@ -198,6 +201,17 @@ private static String paramName(String token) {
return null;
}

/**
* Removes the portion of the specified string following a colon (:).
*/
private static String removeColonAndFollowing(String paramName) {
final int index = paramName.indexOf(':');
if (index == -1) {
return paramName;
}
return paramName.substring(0, index);
}

/**
* Return true if path parameter contains capture the rest path pattern
* ({@code "{*foo}"}" or {@code ":*foo"}).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -95,7 +96,7 @@ public final class RouteBuilder {
* @throws IllegalArgumentException if the specified path pattern is invalid
*/
public RouteBuilder path(String pathPattern) {
return pathMapping(getPathMapping(pathPattern));
return pathMapping(getPathMapping(pathPattern, RouteBuilder::getPathMapping));
}

/**
Expand Down Expand Up @@ -165,7 +166,7 @@ RouteBuilder pathMapping(PathMapping pathMapping) {
* Sets the {@link Route} to match the specified exact path.
*/
public RouteBuilder exact(String exactPath) {
return pathMapping(new ExactPathMapping(exactPath));
return pathMapping(new ExactPathMapping(requireNonNull(exactPath, "exactPath")));
}

/**
Expand Down Expand Up @@ -216,7 +217,9 @@ public RouteBuilder glob(String glob) {
}

private RouteBuilder glob(String glob, int numGroupsToSkip) {
return pathMapping(globPathMapping(requireNonNull(glob, "glob"), numGroupsToSkip));
requireNonNull(glob, "glob");
return pathMapping(
getPathMapping(glob, pathPattern -> globPathMapping(pathPattern, numGroupsToSkip)));
}

/**
Expand Down Expand Up @@ -546,6 +549,26 @@ private static PathMapping getPathMapping(String pathPattern) {
return new ParameterizedPathMapping(pathPattern);
}

private static PathMapping getPathMapping(String pathPattern,
Function<String, PathMapping> basePathMappingMapper) {
requireNonNull(pathPattern, "pathPattern");
if (pathPattern.startsWith(EXACT) ||
pathPattern.startsWith(PREFIX) ||
pathPattern.startsWith(REGEX) ||
pathPattern.startsWith(GLOB)) {
return basePathMappingMapper.apply(pathPattern);
}

// Parameterized, glob or no prefix.
final String verb = VerbSuffixPathMapping.findVerb(pathPattern);
if (verb == null) {
return basePathMappingMapper.apply(pathPattern);
}
final String basePathPattern = pathPattern.substring(0, pathPattern.length() - verb.length() - 1);
final PathMapping basePathMapping = basePathMappingMapper.apply(basePathPattern);
return new VerbSuffixPathMapping(basePathMapping, verb);
}

static PathMapping prefixPathMapping(String prefix, boolean stripPrefix) {
if ("/".equals(prefix)) {
// Every path starts with '/'.
Expand Down
23 changes: 17 additions & 6 deletions core/src/main/java/com/linecorp/armeria/server/RoutingTrie.java
Original file line number Diff line number Diff line change
Expand Up @@ -223,16 +223,27 @@ private Node<V> checkNode(Node<V> node, String path, int begin, boolean exact, I
break;
case PARAMETER:
// Consume characters until the delimiter '/' as a path variable.
final int delim = path.indexOf('/', begin);
if (delim < 0) {
// No more delimiter.
return node;
final int delimSlash = path.indexOf('/', begin);

if (delimSlash < 0) {
final int lastColon = path.lastIndexOf(':');
// lastIndexOf performs backward search
if (lastColon < begin) {
// No more delimiter.
return node;
} else {
final Node<V> verb = node.children.get(':');
ikhoon marked this conversation as resolved.
Show resolved Hide resolved
final String pathVerbPart = path.substring(lastColon);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Micro optimization) What do you think of implementing a method such as findVerb to find a verb part from path? findVerb will scan path from the last character up to begin to find :.

String verbPart = findVerb(path, begin);
if (verbPart == null) {
  return node;
} else {
   final Node<V> verb = node.children.get(':');
   ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use the fineVerb method defined in VerbSuffixPathMapping because the verb found by the method has to be last segment of a path 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regular expression may consume more CPU cycles which is inefficient in finding an exact character.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your advice! Implemented the findVerb function.


return verb != null && verb.path.equals(pathVerbPart) ? verb : node;
}
}
if (path.length() == delim + 1) {

if (path.length() == delimSlash + 1) {
final Node<V> trailingSlashNode = node.children.get('/');
return trailingSlashNode != null ? trailingSlashNode : node;
}
next.value = delim;
next.value = delimSlash;
break;
default:
throw new Error("Should not reach here");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ RoutingTrieBuilder<V> add(String path, V value, boolean hasHighPrecedence) {
checkArgument(path.indexOf(KEY_CATCH_ALL) < 0,
"A path should not contain %s: %s",
Integer.toHexString(KEY_CATCH_ALL), path);

routes.add(new Entry<>(path, value, hasHighPrecedence));
return this;
}
Expand Down Expand Up @@ -137,8 +136,10 @@ private void addRoute(NodeBuilder<V> node, Entry<V> entry) {

// Count the number of characters having the same prefix.
int same = 0;
while (same < max && p.charAt(same) == path.charAt(same)) {
final PathReader pathReader = new PathReader(path);
while (same < max && pathReader.isSameChar(p.charAt(same))) {
same++;
pathReader.moveIndex();
minwoox marked this conversation as resolved.
Show resolved Hide resolved
}

// We need to split the current node into two in order to ensure that this node has the
Expand All @@ -156,31 +157,15 @@ private void addRoute(NodeBuilder<V> node, Entry<V> entry) {

// We need to find a child to be able to consume the next character of the path, or need to
// make a new sub trie to manage remaining part of the path.
final char nextChar = convertKey(path.charAt(same));
final NodeBuilder<V> next = current.child(nextChar);
final NodeBuilder<V> next = current.child(pathReader.peekAsKey());
if (next == null) {
// Insert node.
insertChild(current, path.substring(same), entry.value, entry.hasHighPrecedence);
insertChild(current, pathReader.remaining(), entry.value, entry.hasHighPrecedence);
return;
}

current = next;
path = path.substring(same);
}
}

/**
* Converts the given character to the key of the children map.
* This is only used while building a {@link RoutingTrie}.
*/
static char convertKey(char key) {
switch (key) {
case ':':
return KEY_PARAMETER;
case '*':
return KEY_CATCH_ALL;
default:
return key;
path = pathReader.remaining();
}
}

Expand All @@ -189,24 +174,37 @@ static char convertKey(char key) {
*/
private NodeBuilder<V> insertChild(@Nullable NodeBuilder<V> node,
String path, V value, boolean highPrecedence) {
int pathStart = 0;
final int max = path.length();
final PathReader pathReader = new PathReader(path);
StringBuilder exactPath = null;

while (pathReader.hasNext()) {
// All escaped characters will be treated as an exact path.
if (pathReader.isEscaped()) {
if (exactPath == null) {
exactPath = new StringBuilder();
}
exactPath.append(pathReader.read());
continue;
}

for (int i = 0; i < max; i++) {
final char c = path.charAt(i);
final char c = pathReader.read();
// Find the prefix until the first wildcard (':' or '*')
if (c != '*' && c != ':') {
if (exactPath == null) {
exactPath = new StringBuilder();
}
exactPath.append(c);
continue;
}
if (c == '*' && i + 1 < max) {

if (c == '*' && pathReader.hasNext()) {
throw new IllegalStateException("Catch-all should be the last in the path: " + path);
}

if (i > pathStart) {
node = asChild(new NodeBuilder<>(NodeType.EXACT, node, path.substring(pathStart, i)));
if (exactPath != null) {
node = asChild(new NodeBuilder<>(NodeType.EXACT, node, exactPath.toString()));
exactPath = null;
}
// Skip this '*' or ':' character.
pathStart = i + 1;

if (c == '*') {
node = asChild(new NodeBuilder<>(NodeType.CATCH_ALL, node, "*"));
Expand All @@ -216,8 +214,8 @@ private NodeBuilder<V> insertChild(@Nullable NodeBuilder<V> node,
}

// Make a new child node with the remaining characters of the path.
if (pathStart < max) {
node = asChild(new NodeBuilder<>(NodeType.EXACT, node, path.substring(pathStart)));
if (exactPath != null) {
node = asChild(new NodeBuilder<>(NodeType.EXACT, node, exactPath.toString()));
}
// Attach the value to the last node.
assert node != null;
Expand Down Expand Up @@ -277,6 +275,17 @@ NodeBuilder<V> child(char key) {
return children == null ? null : children.get(key);
}

char extractKey() {
switch (type) {
case PARAMETER:
return KEY_PARAMETER;
case CATCH_ALL:
return KEY_CATCH_ALL;
default:
return path.charAt(0);
}
}

/**
* Attaches a given {@code value} to the value list. If the list is not empty
* the {@code value} is added, and sorted by the given {@link Comparator}.
Expand Down Expand Up @@ -309,7 +318,7 @@ void addValue(V value, boolean highPrecedence, @Nullable Comparator<V> comparato
NodeBuilder<V> addChild(NodeBuilder<V> child) {
requireNonNull(child, "child");

final char key = convertKey(child.path.charAt(0));
final char key = child.extractKey();
if (children == null) {
children = new Char2ObjectOpenHashMap<>();
}
Expand Down Expand Up @@ -418,4 +427,63 @@ Node<V> build() {
return new Node<>(type, path, children, parameterChild, catchAllChild, values);
}
}

private static class PathReader {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice abstraction

private final String path;
private final int length;
private int index;

PathReader(String path) {
this.path = path;
length = path.length();
}

char peekAsKey() {
final char c = path.charAt(index);
if (isEscapeChar(c)) {
return path.charAt(index + 1);
}
switch (c) {
case ':':
return KEY_PARAMETER;
case '*':
return KEY_CATCH_ALL;
default:
return c;
}
}

char read() {
final char c = path.charAt(index++);
if (isEscapeChar(c)) {
return path.charAt(index++);
} else {
return c;
}
}

boolean isSameChar(char c) {
return path.charAt(index) == c;
}

int moveIndex() {
return ++index;
}

boolean hasNext() {
return index < length;
}

boolean isEscaped() {
return isEscapeChar(path.charAt(index));
}

String remaining() {
return path.substring(index);
}

private boolean isEscapeChar(char ch) {
minwoox marked this conversation as resolved.
Show resolved Hide resolved
return ch == '\\';
}
}
}