Simplify and Optimize RestController Slightly (#45419) (#45485)

* Simplify the path iterator to generate less garbage
* `dispatchRequest` always terminates, adjust code accordingly
This commit is contained in:
Armin Braun 2019-08-13 10:43:30 +02:00 committed by GitHub
parent 488673f636
commit 00e4fba2fb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 53 deletions

View File

@ -19,12 +19,9 @@
package org.elasticsearch.common.path; package org.elasticsearch.common.path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.EnumSet; import java.util.EnumSet;
import java.util.HashMap; import java.util.HashMap;
import java.util.Iterator; import java.util.Iterator;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.NoSuchElementException; import java.util.NoSuchElementException;
import java.util.function.BiFunction; import java.util.function.BiFunction;
@ -352,40 +349,22 @@ public class PathTrie<T> {
* of parameters. * of parameters.
*/ */
public Iterator<T> retrieveAll(String path, Supplier<Map<String, String>> paramSupplier) { public Iterator<T> retrieveAll(String path, Supplier<Map<String, String>> paramSupplier) {
return new PathTrieIterator<>(this, path, paramSupplier); return new Iterator<T>() {
}
private static class PathTrieIterator<T> implements Iterator<T> { private int mode;
private final List<TrieMatchingMode> modes;
private final Supplier<Map<String, String>> paramSupplier;
private final PathTrie<T> trie;
private final String path;
PathTrieIterator(PathTrie<T> trie, String path, Supplier<Map<String, String>> paramSupplier) {
this.path = path;
this.trie = trie;
this.paramSupplier = paramSupplier;
this.modes = new ArrayList<>(Arrays.asList(TrieMatchingMode.EXPLICIT_NODES_ONLY,
TrieMatchingMode.WILDCARD_ROOT_NODES_ALLOWED,
TrieMatchingMode.WILDCARD_LEAF_NODES_ALLOWED,
TrieMatchingMode.WILDCARD_NODES_ALLOWED));
assert TrieMatchingMode.values().length == 4 : "missing trie matching mode";
}
@Override @Override
public boolean hasNext() { public boolean hasNext() {
return modes.isEmpty() == false; return mode < TrieMatchingMode.values().length;
} }
@Override @Override
public T next() { public T next() {
if (modes.isEmpty()) { if (hasNext() == false) {
throw new NoSuchElementException("called next() without validating hasNext()! no more modes available"); throw new NoSuchElementException("called next() without validating hasNext()! no more modes available");
} }
TrieMatchingMode mode = modes.remove(0); return retrieve(path, paramSupplier.get(), TrieMatchingMode.values()[mode++]);
Map<String, String> params = paramSupplier.get(); }
return trie.retrieve(path, params, mode); };
}
} }
} }

View File

@ -48,7 +48,6 @@ import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier; import java.util.function.Supplier;
@ -153,9 +152,8 @@ public class RestController implements HttpServerTransport.Dispatcher {
usageService.addRestHandler((BaseRestHandler) handler); usageService.addRestHandler((BaseRestHandler) handler);
} }
final RestHandler maybeWrappedHandler = handlerWrapper.apply(handler); final RestHandler maybeWrappedHandler = handlerWrapper.apply(handler);
handlers.insertOrUpdate(path, new MethodHandlers(path, maybeWrappedHandler, method), (mHandlers, newMHandler) -> { handlers.insertOrUpdate(path, new MethodHandlers(path, maybeWrappedHandler, method),
return mHandlers.addMethods(maybeWrappedHandler, method); (mHandlers, newMHandler) -> mHandlers.addMethods(maybeWrappedHandler, method));
});
} }
@Override @Override
@ -198,21 +196,18 @@ public class RestController implements HttpServerTransport.Dispatcher {
} }
} }
/** private void dispatchRequest(RestRequest request, RestChannel channel, RestHandler handler) throws Exception {
* Dispatch the request, if possible, returning true if a response was sent or false otherwise.
*/
private boolean dispatchRequest(RestRequest request, RestChannel channel, RestHandler handler) throws Exception {
final int contentLength = request.content().length(); final int contentLength = request.content().length();
if (contentLength > 0) { if (contentLength > 0) {
final XContentType xContentType = request.getXContentType(); final XContentType xContentType = request.getXContentType();
if (xContentType == null) { if (xContentType == null) {
sendContentTypeErrorMessage(request.getAllHeaderValues("Content-Type"), channel); sendContentTypeErrorMessage(request.getAllHeaderValues("Content-Type"), channel);
return true; return;
} }
if (handler.supportsContentStream() && xContentType != XContentType.JSON && xContentType != XContentType.SMILE) { if (handler.supportsContentStream() && xContentType != XContentType.JSON && xContentType != XContentType.SMILE) {
channel.sendResponse(BytesRestResponse.createSimpleErrorResponse(channel, RestStatus.NOT_ACCEPTABLE, channel.sendResponse(BytesRestResponse.createSimpleErrorResponse(channel, RestStatus.NOT_ACCEPTABLE,
"Content-Type [" + xContentType + "] does not support stream parsing. Use JSON or SMILE instead")); "Content-Type [" + xContentType + "] does not support stream parsing. Use JSON or SMILE instead"));
return true; return;
} }
} }
RestChannel responseChannel = channel; RestChannel responseChannel = channel;
@ -228,7 +223,6 @@ public class RestController implements HttpServerTransport.Dispatcher {
} catch (Exception e) { } catch (Exception e) {
responseChannel.sendResponse(new BytesRestResponse(responseChannel, e)); responseChannel.sendResponse(new BytesRestResponse(responseChannel, e));
} }
return true;
} }
private boolean handleNoHandlerFound(String rawPath, RestRequest.Method method, String uri, RestChannel channel) { private boolean handleNoHandlerFound(String rawPath, RestRequest.Method method, String uri, RestChannel channel) {
@ -297,7 +291,8 @@ public class RestController implements HttpServerTransport.Dispatcher {
if (handleNoHandlerFound(rawPath, requestMethod, uri, channel)) { if (handleNoHandlerFound(rawPath, requestMethod, uri, channel)) {
return; return;
} }
} else if (dispatchRequest(request, channel, handler)) { } else {
dispatchRequest(request, channel, handler);
return; return;
} }
} }
@ -402,8 +397,11 @@ public class RestController implements HttpServerTransport.Dispatcher {
private Set<RestRequest.Method> getValidHandlerMethodSet(String rawPath) { private Set<RestRequest.Method> getValidHandlerMethodSet(String rawPath) {
Set<RestRequest.Method> validMethods = new HashSet<>(); Set<RestRequest.Method> validMethods = new HashSet<>();
Iterator<MethodHandlers> allHandlers = getAllHandlers(null, rawPath); Iterator<MethodHandlers> allHandlers = getAllHandlers(null, rawPath);
for (Iterator<MethodHandlers> it = allHandlers; it.hasNext(); ) { while (allHandlers.hasNext()) {
Optional.ofNullable(it.next()).map(mh -> validMethods.addAll(mh.getValidMethods())); final MethodHandlers methodHandlers = allHandlers.next();
if (methodHandlers != null) {
validMethods.addAll(methodHandlers.getValidMethods());
}
} }
return validMethods; return validMethods;
} }