From 6d64cbd556030359e30c259ddc6144c43f2ae674 Mon Sep 17 00:00:00 2001 From: Michael Bolz Date: Fri, 7 Aug 2015 17:48:02 +0200 Subject: [PATCH] [OLINGO-659] Several fixes for issues from static code check --- .../olingo/commons/api/format/AcceptType.java | 14 +++--- .../commons/api/format/ContentType.java | 6 +-- .../commons/core/edm/FunctionMapKey.java | 34 +++------------ .../server/api/prefer/PreferencesApplied.java | 25 +++++------ .../BatchReferenceRewriter.java | 2 +- .../deserializer/batch/BatchBodyPart.java | 12 +++--- .../deserializer/batch/BatchLineReader.java | 28 ++++++------ .../server/core/uri/parser/UriDecoder.java | 43 +++---------------- .../server/core/uri/parser/RawUriTest.java | 16 ------- 9 files changed, 58 insertions(+), 122 deletions(-) diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java index caf52ba60..7c1057655 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/AcceptType.java @@ -164,8 +164,8 @@ public class AcceptType { public String toString() { StringBuilder result = new StringBuilder(); result.append(type).append('/').append(subtype); - for (final String key : parameters.keySet()) { - result.append(';').append(key).append('=').append(parameters.get(key)); + for (final Map.Entry entry : parameters.entrySet()) { + result.append(';').append(entry.getKey()).append('=').append(entry.getValue()); } return result.toString(); @@ -179,7 +179,7 @@ public class AcceptType { *
  • the subtype must be '*' or equal to the content-type's subtype,
    • *
  • all parameters must have the same value as in the content-type's parameter map.
    • *

    - * @param contentType + * @param contentType content type against which is matched * @return whether this accept type matches the given content type */ public boolean matches(final ContentType contentType) { @@ -196,10 +196,10 @@ public class AcceptType { return false; } Map compareParameters = contentType.getParameters(); - for (final String key : parameters.keySet()) { - if (compareParameters.containsKey(key) || TypeUtil.PARAMETER_Q.equalsIgnoreCase(key)) { - if (!parameters.get(key).equalsIgnoreCase(compareParameters.get(key)) - && !TypeUtil.PARAMETER_Q.equalsIgnoreCase(key)) { + for (final Map.Entry entry : parameters.entrySet()) { + if (compareParameters.containsKey(entry.getKey()) || TypeUtil.PARAMETER_Q.equalsIgnoreCase(entry.getKey())) { + String compare = compareParameters.get(entry.getKey()); + if (!entry.getValue().equalsIgnoreCase(compare) && !TypeUtil.PARAMETER_Q.equalsIgnoreCase(entry.getKey())) { return false; } } else { diff --git a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java index 2ff035d71..394e53428 100644 --- a/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java +++ b/lib/commons-api/src/main/java/org/apache/olingo/commons/api/format/ContentType.java @@ -303,9 +303,9 @@ public final class ContentType { sb.append(type).append(TypeUtil.TYPE_SUBTYPE_SEPARATOR).append(subtype); - for (String key : parameters.keySet()) { - sb.append(TypeUtil.PARAMETER_SEPARATOR).append(key) - .append(TypeUtil.PARAMETER_KEY_VALUE_SEPARATOR).append(parameters.get(key)); + for (Entry entry : parameters.entrySet()) { + sb.append(TypeUtil.PARAMETER_SEPARATOR).append(entry.getKey()) + .append(TypeUtil.PARAMETER_KEY_VALUE_SEPARATOR).append(entry.getValue()); } return sb.toString(); } diff --git a/lib/commons-core/src/main/java/org/apache/olingo/commons/core/edm/FunctionMapKey.java b/lib/commons-core/src/main/java/org/apache/olingo/commons/core/edm/FunctionMapKey.java index 0b37b0c3c..b2afdd01e 100644 --- a/lib/commons-core/src/main/java/org/apache/olingo/commons/core/edm/FunctionMapKey.java +++ b/lib/commons-core/src/main/java/org/apache/olingo/commons/core/edm/FunctionMapKey.java @@ -25,7 +25,7 @@ import java.util.List; import org.apache.olingo.commons.api.edm.EdmException; import org.apache.olingo.commons.api.edm.FullQualifiedName; -public class FunctionMapKey { +public final class FunctionMapKey { private final FullQualifiedName functionName; @@ -54,29 +54,11 @@ public class FunctionMapKey { @Override public int hashCode() { - String hash = functionName.toString(); - - if (bindingParameterTypeName != null) { - hash = hash + bindingParameterTypeName.toString(); - } else { - hash = hash + "typeNull"; - } - - if (isBindingParameterCollection != null) { - hash = hash + isBindingParameterCollection.toString(); - } else { - hash = hash + "collectionNull"; - } - - if (!parameterNames.isEmpty()) { - for (String name : parameterNames) { - hash = hash + name; - } - } else { - hash = hash + "parameterNamesEmpty"; - } - - return hash.hashCode(); + int result = functionName != null ? functionName.hashCode() : 0; + result = 31 * result + (bindingParameterTypeName != null ? bindingParameterTypeName.hashCode() : 0); + result = 31 * result + (isBindingParameterCollection != null ? isBindingParameterCollection.hashCode() : 0); + result = 31 * result + parameterNames.hashCode(); + return result; } @Override @@ -97,9 +79,7 @@ public class FunctionMapKey { || (isBindingParameterCollection != null && isBindingParameterCollection.equals(other.isBindingParameterCollection))) { - if (parameterNames == null && other.parameterNames == null) { - return true; - } else if (parameterNames.size() == other.parameterNames.size()) { + if (parameterNames.size() == other.parameterNames.size()) { for (String name : parameterNames) { if (!other.parameterNames.contains(name)) { return false; diff --git a/lib/server-api/src/main/java/org/apache/olingo/server/api/prefer/PreferencesApplied.java b/lib/server-api/src/main/java/org/apache/olingo/server/api/prefer/PreferencesApplied.java index f7a8146b7..028d3b1a7 100644 --- a/lib/server-api/src/main/java/org/apache/olingo/server/api/prefer/PreferencesApplied.java +++ b/lib/server-api/src/main/java/org/apache/olingo/server/api/prefer/PreferencesApplied.java @@ -50,23 +50,24 @@ public class PreferencesApplied { /** Returns a string representation that can be used as value of a Preference-Applied HTTP response header. */ public String toValueString() { StringBuilder result = new StringBuilder(); - for (final String name : applied.keySet()) { + for (final Map.Entry entry : applied.entrySet()) { if (result.length() > 0) { result.append(',').append(' '); } - result.append(name); - if (applied.get(name) != null) { - final boolean safe = ODataPreferenceNames.ALLOW_ENTITY_REFERENCES.toString().equals(name) - || ODataPreferenceNames.CALLBACK.toString().equals(name) - || ODataPreferenceNames.CONTINUE_ON_ERROR.toString().equals(name) - || ODataPreferenceNames.MAX_PAGE_SIZE.toString().equals(name) - || ODataPreferenceNames.TRACK_CHANGES.toString().equals(name) - || ODataPreferenceNames.RETURN.toString().equals(name) - || ODataPreferenceNames.RESPOND_ASYNC.toString().equals(name) - || ODataPreferenceNames.WAIT.toString().equals(name); + final String key = entry.getKey(); + result.append(key); + if (entry.getValue() != null) { + final boolean safe = ODataPreferenceNames.ALLOW_ENTITY_REFERENCES.toString().equals(key) + || ODataPreferenceNames.CALLBACK.toString().equals(key) + || ODataPreferenceNames.CONTINUE_ON_ERROR.toString().equals(key) + || ODataPreferenceNames.MAX_PAGE_SIZE.toString().equals(key) + || ODataPreferenceNames.TRACK_CHANGES.toString().equals(key) + || ODataPreferenceNames.RETURN.toString().equals(key) + || ODataPreferenceNames.RESPOND_ASYNC.toString().equals(key) + || ODataPreferenceNames.WAIT.toString().equals(key); result.append('=') .append(safe ? "" : '"') - .append(applied.get(name).replaceAll("\\\\|\"", "\\\\$0")) + .append(entry.getValue().replaceAll("\\\\|\"", "\\\\$0")) .append(safe ? "" : '"'); } } diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/batchhandler/referenceRewriting/BatchReferenceRewriter.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/batchhandler/referenceRewriting/BatchReferenceRewriter.java index b3ba074d0..31d201040 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/batchhandler/referenceRewriting/BatchReferenceRewriter.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/batchhandler/referenceRewriting/BatchReferenceRewriter.java @@ -98,7 +98,7 @@ public class BatchReferenceRewriter { } private String removeSlash(final String rawODataPath, final boolean first) { - final int indexOfSlash = rawODataPath.indexOf("/"); + final int indexOfSlash = rawODataPath.indexOf('/'); if (first) { return (indexOfSlash == 0) ? rawODataPath.substring(1) : rawODataPath; } else { diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchBodyPart.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchBodyPart.java index 3642f9a85..6c88da2ab 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchBodyPart.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchBodyPart.java @@ -26,9 +26,9 @@ import org.apache.olingo.commons.api.http.HttpHeader; import org.apache.olingo.server.api.deserializer.batch.BatchDeserializerException; public class BatchBodyPart implements BatchPart { - final private String boundary; - final private boolean isStrict; - List remainingMessage = new LinkedList(); + private final String boundary; + private final boolean isStrict; + private final List remainingMessage = new LinkedList(); private Header headers; private boolean isChangeSet; @@ -58,13 +58,13 @@ public class BatchBodyPart implements BatchPart { Integer.toString(headers.getLineNumber())); } - boolean isChangeSet = false; + boolean changeSet = false; for (String contentType : contentTypes) { if (isContentTypeMultiPartMixed(contentType)) { - isChangeSet = true; + changeSet = true; } } - return isChangeSet; + return changeSet; } private List consumeRequest(final List remainingMessage) diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchLineReader.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchLineReader.java index bd71d93b8..c0a1e206f 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchLineReader.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/batch/BatchLineReader.java @@ -96,8 +96,8 @@ public class BatchLineReader { private void updateCurrentCharset(String currentLine) { if(currentLine != null) { if(currentLine.startsWith(HttpHeader.CONTENT_TYPE)) { - currentLine = currentLine.substring(13, currentLine.length() - 2).trim(); - ContentType ct = ContentType.parse(currentLine); + String clValue = currentLine.substring(13, currentLine.length() - 2).trim(); + ContentType ct = ContentType.parse(clValue); if (ct != null) { String charsetString = ct.getParameter(ContentType.PARAMETER_CHARSET); if (charsetString != null) { @@ -133,7 +133,7 @@ public class BatchLineReader { return null; } - ByteBuffer buffer = ByteBuffer.allocate(BUFFER_SIZE); + ByteBuffer innerBuffer = ByteBuffer.allocate(BUFFER_SIZE); boolean foundLineEnd = false; // EOF will be considered as line ending while (!foundLineEnd) { @@ -146,13 +146,13 @@ public class BatchLineReader { if (!foundLineEnd) { byte currentChar = this.buffer[offset++]; - if(!buffer.hasRemaining()) { - buffer.flip(); - ByteBuffer tmp = ByteBuffer.allocate(buffer.limit() *2); - tmp.put(buffer); - buffer = tmp; + if(!innerBuffer.hasRemaining()) { + innerBuffer.flip(); + ByteBuffer tmp = ByteBuffer.allocate(innerBuffer.limit() *2); + tmp.put(innerBuffer); + innerBuffer = tmp; } - buffer.put(currentChar); + innerBuffer.put(currentChar); if (currentChar == LF) { foundLineEnd = true; @@ -167,21 +167,21 @@ public class BatchLineReader { // Check if there is at least one character if (limit != EOF && this.buffer[offset] == LF) { - buffer.put(LF); + innerBuffer.put(LF); offset++; } } } } - if(buffer.position() == 0) { + if(innerBuffer.position() == 0) { return null; } else { String currentLine; if(readState.isReadBody()) { - currentLine = new String(buffer.array(), 0, buffer.position(), getCurrentCharset()); + currentLine = new String(innerBuffer.array(), 0, innerBuffer.position(), getCurrentCharset()); } else { - currentLine = new String(buffer.array(), 0, buffer.position(), CS_ISO_8859_1); + currentLine = new String(innerBuffer.array(), 0, innerBuffer.position(), CS_ISO_8859_1); } updateCurrentCharset(currentLine); return currentLine; @@ -202,7 +202,7 @@ public class BatchLineReader { /** * Read state indicator (whether currently the body or header part is read). */ - private class ReadState { + private static class ReadState { private int state = 0; public void foundLinebreak() { diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriDecoder.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriDecoder.java index c49b98f8f..4649ac55a 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriDecoder.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriDecoder.java @@ -85,10 +85,13 @@ public class UriDecoder { return skipSegments > 0 ? list.subList(skipSegments, list.size()) : list; } - static List split(final String input, final char c) { - return split(input, c, false); - } - + /** + * Split the input string at given character and drop all empty elements. + * + * @param input string to split + * @param c character at which to split + * @return list of elements (can be empty) + */ static List splitSkipEmpty(final String input, final char c) { if(input.isEmpty() || input.length() == 1 && input.charAt(0) == c) { return Collections.emptyList(); @@ -113,38 +116,6 @@ public class UriDecoder { return list; } - static List split(final String input, final char c, boolean skipEmpty) { - if(skipEmpty && (input.isEmpty() || input.length() == 1 && input.charAt(0) == c)) { - return Collections.emptyList(); - } - - List list = new LinkedList(); - - int start = 0; - int end; - - while ((end = input.indexOf(c, start)) >= 0) { - if(skipEmpty) { - if(start != end) { - list.add(input.substring(start, end)); - } - } else { - list.add(input.substring(start, end)); - } - start = end + 1; - } - - if(skipEmpty) { - if(input.charAt(input.length()-1) != c) { - list.add(input.substring(start)); - } - } else { - list.add(input.substring(start)); - } - - return list; - } - public static String decode(final String encoded) throws UriParserSyntaxException { try { return Decoder.decode(encoded); diff --git a/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/parser/RawUriTest.java b/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/parser/RawUriTest.java index 927a37a53..c897400df 100644 --- a/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/parser/RawUriTest.java +++ b/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/parser/RawUriTest.java @@ -124,22 +124,6 @@ public class RawUriTest { @Test public void testSplit() { - assertEquals(Arrays.asList(""), UriDecoder.split("", '/')); - assertEquals(Arrays.asList("", ""), UriDecoder.split("/", '/')); - assertEquals(Arrays.asList("a"), UriDecoder.split("a", '/')); - assertEquals(Arrays.asList("a", ""), UriDecoder.split("a/", '/')); - assertEquals(Arrays.asList("", "a"), UriDecoder.split("/a", '/')); - assertEquals(Arrays.asList("a", "a"), UriDecoder.split("a/a", '/')); - assertEquals(Arrays.asList("", "a", "a"), UriDecoder.split("/a/a", '/')); - // with skip - assertTrue(UriDecoder.split("", '/', true).isEmpty()); - assertTrue(UriDecoder.split("/", '/', true).isEmpty()); - assertEquals(Arrays.asList("a"), UriDecoder.split("a", '/', true)); - assertEquals(Arrays.asList("a"), UriDecoder.split("a/", '/', true)); - assertEquals(Arrays.asList("a"), UriDecoder.split("/a", '/', true)); - assertEquals(Arrays.asList("a", "a"), UriDecoder.split("a/a", '/', true)); - assertEquals(Arrays.asList("a", "a"), UriDecoder.split("/a/a", '/', true)); - // with skip assertTrue(UriDecoder.splitSkipEmpty("", '/').isEmpty()); assertTrue(UriDecoder.splitSkipEmpty("/", '/').isEmpty()); assertEquals(Arrays.asList("a"), UriDecoder.splitSkipEmpty("a", '/'));