Performance Refactoring Signed-off-by: Christian Amend <[email protected]>
Project: http://git-wip-us.apache.org/repos/asf/olingo-odata2/repo Commit: http://git-wip-us.apache.org/repos/asf/olingo-odata2/commit/1f76b210 Tree: http://git-wip-us.apache.org/repos/asf/olingo-odata2/tree/1f76b210 Diff: http://git-wip-us.apache.org/repos/asf/olingo-odata2/diff/1f76b210 Branch: refs/heads/master Commit: 1f76b2103d75fe8cd5077538bb84ad1b0bba28e3 Parents: b155bda Author: Christian Holzer <[email protected]> Authored: Tue Oct 7 14:42:41 2014 +0200 Committer: Christian Amend <[email protected]> Committed: Thu Oct 9 16:45:26 2014 +0200 ---------------------------------------------------------------------- .../olingo/odata2/core/batch/AcceptParser.java | 143 ++++++------------- .../odata2/core/batch/v2/BatchParser.java | 24 ++-- .../odata2/core/batch/v2/BatchParserCommon.java | 74 +++++++++- .../v2/BufferedReaderIncludingLineEndings.java | 10 +- .../olingo/odata2/core/batch/v2/Header.java | 2 +- .../core/batch/BatchRequestParserTest.java | 37 +++-- 6 files changed, 158 insertions(+), 132 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/1f76b210/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/AcceptParser.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/AcceptParser.java b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/AcceptParser.java index 48bfb1b..5c8811c 100644 --- a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/AcceptParser.java +++ b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/AcceptParser.java @@ -19,23 +19,19 @@ package org.apache.olingo.odata2.core.batch; import java.util.ArrayList; -import java.util.Comparator; -import java.util.Iterator; -import java.util.LinkedList; import java.util.List; -import java.util.Scanner; import java.util.TreeSet; -import java.util.regex.MatchResult; +import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.olingo.odata2.api.batch.BatchException; +import org.apache.olingo.odata2.api.exception.MessageReference; /** * */ public class AcceptParser { - private static final String COMMA = ","; private static final String BAD_REQUEST = "400"; private static final String ALL = "*"; private static final String REG_EX_QUALITY_FACTOR = "q=((?:1\\.0{0,3})|(?:0\\.[0-9]{0,2}[1-9]))"; @@ -54,42 +50,53 @@ public class AcceptParser { private List<String> acceptLanguageHeaderValues = new ArrayList<String>(); public List<String> parseAcceptHeaders() throws BatchException { - final String headerValue = concatenateHeaderLines(acceptHeaderValues); - final TreeSet<Accept> acceptTree = getAcceptTree(); + return parseQualifiedHeader(acceptHeaderValues, + REG_EX_ACCEPT_WITH_Q_FACTOR, + BatchException.INVALID_ACCEPT_HEADER); + } + + public List<String> parseAcceptableLanguages() throws BatchException { + return parseQualifiedHeader(acceptLanguageHeaderValues, + REG_EX_ACCEPT_LANGUAGES_WITH_Q_FACTOR, + BatchException.INVALID_ACCEPT_LANGUAGE_HEADER); + } + + private List<String> parseQualifiedHeader(List<String> headerValues, Pattern regEx, MessageReference exectionMessage) + throws BatchException { + final TreeSet<Accept> acceptTree = new TreeSet<AcceptParser.Accept>(); final List<String> acceptHeaders = new ArrayList<String>(); - final Scanner acceptHeaderScanner = new Scanner(headerValue); - - acceptHeaderScanner.useDelimiter(",\\s?"); - while (acceptHeaderScanner.hasNext()) { - if (acceptHeaderScanner.hasNext(REG_EX_ACCEPT_WITH_Q_FACTOR)) { - acceptHeaderScanner.next(REG_EX_ACCEPT_WITH_Q_FACTOR); - MatchResult result = acceptHeaderScanner.match(); - if (result.groupCount() == 2) { - String acceptHeaderValue = result.group(1); - double qualityFactor = result.group(2) != null ? Double.parseDouble(result.group(2)) : 1d; - qualityFactor = getQualityFactor(acceptHeaderValue, qualityFactor); - Accept acceptHeader = new Accept().setQuality(qualityFactor).setValue(acceptHeaderValue); + + for (final String headerValue : headerValues) { + final String[] acceptParts = headerValue.split(","); + + for (final String part : acceptParts) { + final Matcher matcher = regEx.matcher(part.trim()); + + if (matcher.matches() && matcher.groupCount() == 2) { + final Accept acceptHeader = getQualifiedHeader(matcher); acceptTree.add(acceptHeader); } else { - String header = acceptHeaderScanner.next(); - acceptHeaderScanner.close(); - throw new BatchException(BatchException.INVALID_ACCEPT_HEADER.addContent(header), BAD_REQUEST); + throw new BatchException(exectionMessage.addContent(part), BAD_REQUEST); } - } else { - String header = acceptHeaderScanner.next(); - acceptHeaderScanner.close(); - throw new BatchException(BatchException.INVALID_ACCEPT_HEADER.addContent(header), BAD_REQUEST); } } + for (Accept accept : acceptTree) { if (!acceptHeaders.contains(accept.getValue())) { acceptHeaders.add(accept.getValue()); } } - acceptHeaderScanner.close(); return acceptHeaders; } + private Accept getQualifiedHeader(final Matcher matcher) { + final String acceptHeaderValue = matcher.group(1); + double qualityFactor = matcher.group(2) != null ? Double.parseDouble(matcher.group(2)) : 1d; + qualityFactor = getQualityFactor(acceptHeaderValue, qualityFactor); + + return new Accept().setQuality(qualityFactor).setValue(acceptHeaderValue); + } + private double getQualityFactor(final String acceptHeaderValue, double qualityFactor) { int paramNumber = 0; double typeFactor = 0.0; @@ -115,57 +122,7 @@ public class AcceptParser { qualityFactor = qualityFactor + paramNumber * QUALITY_PARAM_FACTOR + typeFactor + subtypeFactor; return qualityFactor; } - - public List<String> parseAcceptableLanguages() throws BatchException { - final String headerValue = concatenateHeaderLines(acceptLanguageHeaderValues); - final List<String> acceptLanguages = new LinkedList<String>(); - final TreeSet<Accept> acceptTree = getAcceptTree(); - Scanner acceptLanguageScanner = new Scanner(headerValue); - acceptLanguageScanner.useDelimiter(",\\s?"); - - while (acceptLanguageScanner.hasNext()) { - if (acceptLanguageScanner.hasNext(REG_EX_ACCEPT_LANGUAGES_WITH_Q_FACTOR)) { - acceptLanguageScanner.next(REG_EX_ACCEPT_LANGUAGES_WITH_Q_FACTOR); - MatchResult result = acceptLanguageScanner.match(); - if (result.groupCount() == 2) { - String languagerange = result.group(1); - double qualityFactor = result.group(2) != null ? Double.parseDouble(result.group(2)) : 1d; - acceptTree.add(new Accept().setQuality(qualityFactor).setValue(languagerange)); - } else { - String acceptLanguage = acceptLanguageScanner.next(); - acceptLanguageScanner.close(); - throw new BatchException(BatchException.INVALID_ACCEPT_LANGUAGE_HEADER.addContent(acceptLanguage), - BAD_REQUEST); - } - } else { - String acceptLanguage = acceptLanguageScanner.next(); - acceptLanguageScanner.close(); - throw new BatchException(BatchException.INVALID_ACCEPT_LANGUAGE_HEADER.addContent(acceptLanguage), BAD_REQUEST); - } - } - for (Accept accept : acceptTree) { - if (!acceptLanguages.contains(accept.getValue())) { - acceptLanguages.add(accept.getValue()); - } - } - acceptLanguageScanner.close(); - return acceptLanguages; - } - - private String concatenateHeaderLines(final List<String> headerValues) { - final StringBuilder builder = new StringBuilder(); - final Iterator<String> iter = headerValues.iterator(); - - while (iter.hasNext()) { - builder.append(iter.next()); - if (iter.hasNext()) { - builder.append(COMMA); - } - } - - return builder.toString(); - } - + public void addAcceptHeaderValue(final String headerValue) { acceptHeaderValues.add(headerValue); } @@ -174,21 +131,7 @@ public class AcceptParser { acceptLanguageHeaderValues.add(headerValue); } - private TreeSet<Accept> getAcceptTree() { - TreeSet<Accept> treeSet = new TreeSet<Accept>(new Comparator<Accept>() { - @Override - public int compare(final Accept o1, final Accept o2) { - if (o1.getQuality() <= o2.getQuality()) { - return 1; - } else { - return -1; - } - } - }); - return treeSet; - } - - private static class Accept { + private static class Accept implements Comparable<Accept> { private double quality; private String value; @@ -201,14 +144,18 @@ public class AcceptParser { return this; } - public double getQuality() { - return quality; - } - public Accept setQuality(final double quality) { this.quality = quality; return this; } + @Override + public int compareTo(Accept o) { + if (quality <= o.quality) { + return 1; + } else { + return -1; + } + } } } http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/1f76b210/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java index 00a1f2a..76b5215 100644 --- a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java +++ b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParser.java @@ -20,7 +20,6 @@ package org.apache.olingo.odata2.core.batch.v2; import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; import java.util.LinkedList; import java.util.List; @@ -81,7 +80,7 @@ public class BatchParser { final String baseUri = getBaseUri(); final String boundary = BatchParserCommon.getBoundary(contentTypeMime, 1); final List<BatchParserResult> resultList = new LinkedList<BatchParserResult>(); - final List<List<Line>> bodyPartStrings = splitBodyParts(in, boundary); + final List<List<Line>> bodyPartStrings = BatchParserCommon.splitRequestByBoundary(in, boundary); for (List<Line> bodyPartString : bodyPartStrings) { BatchBodyPart bodyPart = new BatchBodyPart(bodyPartString, boundary, isStrict).parse(); @@ -90,16 +89,17 @@ public class BatchParser { return resultList; } - - private List<List<Line>> splitBodyParts(final InputStream in, final String boundary) - throws IOException, BatchException { - - final BufferedReaderIncludingLineEndings reader = new BufferedReaderIncludingLineEndings(new InputStreamReader(in)); - final List<Line> message = reader.toList(); - reader.close(); - - return BatchParserCommon.splitMessageByBoundary(message, boundary); - } + + //TODO remove +// private List<List<Line>> splitBodyParts(final InputStream in, final String boundary) +// throws IOException, BatchException { +// +// final BufferedReaderIncludingLineEndings reader = new BufferedReaderIncludingLineEndings(new InputStreamReader(in)); +// final List<Line> message = reader.toList(); +// reader.close(); +// +// return BatchParserCommon.splitMessageByBoundary(message, boundary); +// } private String getBaseUri() throws BatchException { String baseUri = ""; http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/1f76b210/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParserCommon.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParserCommon.java b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParserCommon.java index b028759..3592688 100644 --- a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParserCommon.java +++ b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BatchParserCommon.java @@ -19,7 +19,9 @@ package org.apache.olingo.odata2.core.batch.v2; import java.io.ByteArrayInputStream; +import java.io.IOException; import java.io.InputStream; +import java.io.InputStreamReader; import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; @@ -44,6 +46,9 @@ import org.apache.olingo.odata2.core.batch.v2.BufferedReaderIncludingLineEndings import org.apache.olingo.odata2.core.commons.Decoder; public class BatchParserCommon { + + private static final Pattern PATTERN_LAST_CRLF = Pattern.compile("(.*)(\r\n){1}( *)", Pattern.DOTALL); + private static final String REG_EX_BOUNDARY = "([a-zA-Z0-9_\\-\\.'\\+]{1,70})|\"([a-zA-Z0-9_\\-\\.'\\+\\s\\" + "(\\),/:=\\?]{1,69}[a-zA-Z0-9_\\-\\.'\\+\\(\\),/:=\\?])\""; // See RFC 2046 @@ -55,6 +60,7 @@ public class BatchParserCommon { public static final Pattern PATTERN_HEADER_LINE = Pattern.compile("([a-zA-Z\\-]+):\\s?(.*)\\s*"); public static final Pattern PATTERN_CONTENT_TYPE_APPLICATION_HTTP = Pattern.compile(REG_EX_APPLICATION_HTTP, Pattern.CASE_INSENSITIVE); + private static final Pattern PATTERN_RELATIVE_URI = Pattern.compile("([^/][^?]*)(\\?.*)?"); public static String trimLineListToLength(final List<Line> list, final int length) { final String message = stringListToString(list); @@ -86,19 +92,77 @@ public class BatchParserCommon { return new ByteArrayInputStream(message.getBytes()); } + + static List<List<Line>> splitRequestByBoundary(final InputStream in, final String boundary) + throws BatchException, IOException { + final List<List<Line>> messageParts = new LinkedList<List<Line>>(); + List<Line> currentPart = new ArrayList<Line>(); + boolean isEndReached = false; + //32ms + final String quotedBoundary = Pattern.quote(boundary); + final Pattern boundaryDelimiterPattern = Pattern.compile("--" + quotedBoundary + "--[\\s ]*"); + final Pattern boundaryPattern = Pattern.compile("--" + quotedBoundary + "[\\s ]*"); + + final BufferedReaderIncludingLineEndings reader = new BufferedReaderIncludingLineEndings(new InputStreamReader(in)); + String currentLine; + int lineNumber = 1; + + while((currentLine = reader.readLine()) != null) { + if (boundaryDelimiterPattern.matcher(currentLine.toString()).matches()) { + removeEndingCRLFFromList(currentPart); + messageParts.add(currentPart); + isEndReached = true; + } else if (boundaryPattern.matcher(currentLine.toString()).matches()) { + removeEndingCRLFFromList(currentPart); + messageParts.add(currentPart); + currentPart = new LinkedList<Line>(); + } else { + currentPart.add(new Line(currentLine, lineNumber)); + } + if (isEndReached) { + break; + } + + lineNumber++; + } + reader.close(); + + // Remove preamble + if (messageParts.size() > 0) { + messageParts.remove(0); + } else { + + throw new BatchException(BatchException.MISSING_BOUNDARY_DELIMITER.addContent(1)); + } + + if (!isEndReached) { + throw new BatchException(BatchException.MISSING_CLOSE_DELIMITER.addContent(1)); + } + + if (messageParts.size() == 0) { + throw new BatchException(BatchException.NO_MATCH_WITH_BOUNDARY_STRING.addContent(boundary).addContent(0)); + } + + return messageParts; + } + static List<List<Line>> splitMessageByBoundary(final List<Line> message, final String boundary) throws BatchException { final List<List<Line>> messageParts = new LinkedList<List<Line>>(); List<Line> currentPart = new ArrayList<Line>(); boolean isEndReached = false; - + + final String quotedBoundary = Pattern.quote(boundary); + final Pattern boundaryDelimiterPattern = Pattern.compile("--" + quotedBoundary + "--[\\s ]*"); + final Pattern boundaryPattern = Pattern.compile("--" + quotedBoundary + "[\\s ]*"); + for (Line currentLine : message) { - if (currentLine.toString().contains("--" + boundary + "--")) { + if (boundaryDelimiterPattern.matcher(currentLine.toString()).matches()) { removeEndingCRLFFromList(currentPart); messageParts.add(currentPart); isEndReached = true; - } else if (currentLine.toString().contains("--" + boundary)) { + } else if (boundaryPattern.matcher(currentLine.toString()).matches()) { removeEndingCRLFFromList(currentPart); messageParts.add(currentPart); currentPart = new LinkedList<Line>(); @@ -139,7 +203,7 @@ public class BatchParserCommon { } public static Line removeEndingCRLF(final Line line) { - Pattern pattern = Pattern.compile("(.*)(\r\n){1}( *)", Pattern.DOTALL); + Pattern pattern = PATTERN_LAST_CRLF; Matcher matcher = pattern.matcher(line.toString()); if (matcher.matches()) { @@ -268,7 +332,7 @@ public class BatchParserCommon { if (uriObject.isAbsolute()) { regexRequestUri = Pattern.compile(baseUri + "/([^/][^?]*)(\\?.*)?"); } else { - regexRequestUri = Pattern.compile("([^/][^?]*)(\\?.*)?"); + regexRequestUri = PATTERN_RELATIVE_URI; } http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/1f76b210/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BufferedReaderIncludingLineEndings.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BufferedReaderIncludingLineEndings.java b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BufferedReaderIncludingLineEndings.java index 295f4c6..7b81dc8 100644 --- a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BufferedReaderIncludingLineEndings.java +++ b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/BufferedReaderIncludingLineEndings.java @@ -27,7 +27,7 @@ public class BufferedReaderIncludingLineEndings extends Reader { private static final char CR = '\r'; private static final char LF = '\n'; private static final int EOF = -1; - private static final int BUFFER_SIZE = 1024; + private static final int BUFFER_SIZE = 8192; private Reader reader; private char[] buffer; private int offset = 0; @@ -105,7 +105,7 @@ public class BufferedReaderIncludingLineEndings extends Reader { } public String readLine() throws IOException { - if (isEOF()) { + if (limit == EOF) { return null; } @@ -113,7 +113,7 @@ public class BufferedReaderIncludingLineEndings extends Reader { boolean foundLineEnd = false; // EOF will be considered as line ending while (!foundLineEnd) { - if (isBufferReloadRequired()) { + if (limit == offset) { if (fillBuffer() == EOF) { foundLineEnd = true; } @@ -129,12 +129,12 @@ public class BufferedReaderIncludingLineEndings extends Reader { foundLineEnd = true; // Check next char. Consume \n if available - if (isBufferReloadRequired()) { + if (limit == offset) { fillBuffer(); } // Check if there is at least one character - if (!isEOF() && buffer[offset] == LF) { + if (limit != EOF && buffer[offset] == LF) { stringBuffer.append(LF); offset++; } http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/1f76b210/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/Header.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/Header.java b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/Header.java index c9daa6a..6c90e02 100644 --- a/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/Header.java +++ b/odata2-lib/odata-core/src/main/java/org/apache/olingo/odata2/core/batch/v2/Header.java @@ -87,7 +87,7 @@ public class Header implements Cloneable { public Map<String, List<String>> toMultiMap() { final Map<String, List<String>> singleMap = new HashMap<String, List<String>>(); - + for (final String key : headers.keySet()) { HeaderField field = headers.get(key); singleMap.put(field.getFieldName(), field.getValues()); http://git-wip-us.apache.org/repos/asf/olingo-odata2/blob/1f76b210/odata2-lib/odata-core/src/test/java/org/apache/olingo/odata2/core/batch/BatchRequestParserTest.java ---------------------------------------------------------------------- diff --git a/odata2-lib/odata-core/src/test/java/org/apache/olingo/odata2/core/batch/BatchRequestParserTest.java b/odata2-lib/odata-core/src/test/java/org/apache/olingo/odata2/core/batch/BatchRequestParserTest.java index 142d355..f9287b5 100644 --- a/odata2-lib/odata-core/src/test/java/org/apache/olingo/odata2/core/batch/BatchRequestParserTest.java +++ b/odata2-lib/odata-core/src/test/java/org/apache/olingo/odata2/core/batch/BatchRequestParserTest.java @@ -29,6 +29,7 @@ import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; import java.util.List; +import java.util.Random; import org.apache.olingo.odata2.api.batch.BatchException; import org.apache.olingo.odata2.api.batch.BatchRequestPart; @@ -895,7 +896,7 @@ public class BatchRequestParserTest { + "This is a preamble and must be ignored" + CRLF + CRLF + CRLF - + "----1242" + + "----1242" + CRLF + "--batch_8194-cf13-1f56" + CRLF + MIME_HEADERS + CRLF @@ -909,7 +910,7 @@ public class BatchRequestParserTest { + "This is a preamble and must be ignored" + CRLF + CRLF + CRLF - + "----1242" + + "----1242" + CRLF + "--changeset_f980-1cb6-94dd" + CRLF + MIME_HEADERS + "Content-Id: " + CONTENT_ID_REFERENCE + CRLF @@ -1071,20 +1072,34 @@ public class BatchRequestParserTest { assertEquals("{\"EmployeeName\":\"Peter Fall\"}", inputStreamToString(changeSetPart.getRequests().get(1).getBody())); } - + @Test public void testLargeBatch() throws BatchException, IOException { - String fileName = "/batchLarge.batch"; - InputStream in = ClassLoader.class.getResourceAsStream(fileName); - if (in == null) { - throw new IOException("Requested file '" + fileName + "' was not found."); + for (int j = 0; j < 200; j++) { + String fileName = "/batchLarge.batch"; + InputStream in = ClassLoader.class.getResourceAsStream(fileName); + if (in == null) { + throw new IOException("Requested file '" + fileName + "' was not found."); + } + + StringBuilder builder = new StringBuilder(); + Random rnd = new Random(); + for (int i = 0; i < 300; i++) { + builder.append((char) ('A' + rnd.nextInt('Z' - 'A'))); + } + + // String request = builder.toString() + CRLF + inputStreamToString(in); + String request = inputStreamToString(in).replace("Walldorf", builder.toString()); + in.close(); + InputStream requestStream = new ByteArrayInputStream(request.getBytes()); + + long start = System.currentTimeMillis(); + BatchParser parser = new BatchParser(contentType, batchProperties, true); + parser.parseBatchRequest(requestStream); + System.out.println(System.currentTimeMillis() - start); } - - BatchParser parser = new BatchParser(contentType, batchProperties, true); - parser.parseBatchRequest(in); } - private List<BatchRequestPart> parse(final String batch) throws BatchException { InputStream in = new ByteArrayInputStream(batch.getBytes()); BatchParser parser = new BatchParser(contentType, batchProperties, true);
