Fix code smells and minor bugs.

Signed-off-by: Achim Kraus <achim.kraus@bosch.io>

californium-core/src/main/java/org/eclipse/californium/core/coap/OptionSet.java

@@ -181,7 +181,7 @@ public void clear() {
 			uri_query_list.clear();
 		accept = null;
 		if (location_query_list != null)
-			location_path_list.clear();
+			location_query_list.clear();
 		proxy_uri = null;
 		proxy_scheme = null;
 		block1 = null;
@@ -384,14 +384,14 @@ public boolean containsETag(byte[] check) {
 	 * 
 	 * @param etag the ETag to add
 	 * @return this OptionSet for a fluent API.
+	 * @throws IllegalArgumentException if the etag is {@code null}
 	 */
 	public OptionSet addETag(byte[] etag) {
 		if (etag==null)
 			throw new IllegalArgumentException("ETag option must not be null");
-		// TODO: ProxyHttp uses ETags that are larger than 8 bytes (20).
-//		if (opaque.length < 1 || 8 < opaque.length)
-//			throw new IllegalArgumentException("ETag option's length must be between 1 and 8 inclusive but was "+opaque.length);
-		getETags().add(etag);
+		if (!containsETag(etag)) {
+			getETags().add(etag.clone());
+		}
 		return this;
 	}
 
@@ -402,7 +402,14 @@ public OptionSet addETag(byte[] etag) {
 	 * @return this OptionSet for a fluent API.
 	 */
 	public OptionSet removeETag(byte[] etag) {
-		getETags().remove(etag);
+		if (etag_list != null && etag != null && etag.length > 0) {
+			for (int index = 0; index < etag_list.size(); ++index) {
+				if (Arrays.equals(etag_list.get(index), etag)) {
+					etag_list.remove(index);
+					break;
+				}
+			}
+		}
 		return this;
 	}
 
@@ -1394,6 +1401,9 @@ public OptionSet removeOscore(){
 	/**
 	 * Checks if an arbitrary option is present.
 	 * 
+	 * Note: implementation uses {@link #asSortedList()} and is therefore not
+	 * recommended to be called too frequently.
+	 * 
 	 * @param number the option number
 	 * @return true if present
 	 */

californium-core/src/main/java/org/eclipse/californium/core/coap/Request.java

@@ -197,6 +197,8 @@ public class Request extends Message {
 	/** The current response for the request. */
 	private Response response;
 
+	private boolean ready;
+
 	/**
 	 * Request's scheme.
 	 * 
@@ -1066,7 +1068,7 @@ public Response waitForResponse(long timeout) throws InterruptedException {
 		long expiresNano = ClockUtil.nanoRealtime() + TimeUnit.MILLISECONDS.toNanos(timeout);
 		long leftTimeout = timeout;
 		synchronized (this) {
-			while (this.response == null && !isCanceled() && !isTimedOut() && !isRejected() && getSendError() == null) {
+			while (!ready && response == null) {
 				wait(leftTimeout);
 				// timeout expired?
 				if (timeout > 0) {
@@ -1096,6 +1098,7 @@ public void setTimedOut(boolean timedOut) {
 		super.setTimedOut(timedOut);
 		if (timedOut) {
 			synchronized (this) {
+				ready = true;
 				notifyAll();
 			}
 		}
@@ -1112,6 +1115,7 @@ public void setCanceled(boolean canceled) {
 		super.setCanceled(canceled);
 		if (canceled) {
 			synchronized (this) {
+				ready = true;
 				notifyAll();
 			}
 		}
@@ -1122,6 +1126,7 @@ public void setRejected(boolean rejected) {
 		super.setRejected(rejected);
 		if (rejected) {
 			synchronized (this) {
+				ready = true;
 				notifyAll();
 			}
 		}
@@ -1132,6 +1137,7 @@ public void setSendError(Throwable sendError) {
 		super.setSendError(sendError);
 		if (sendError != null) {
 			synchronized (this) {
+				ready = true;
 				notifyAll();
 			}
 		}
@@ -1165,6 +1171,7 @@ public void setOnResponseError(Throwable cause) {
 			}
 
 			synchronized (this) {
+				ready = true;
 				notifyAll();
 			}
 		}

californium-core/src/main/java/org/eclipse/californium/core/network/CoapEndpoint.java

@@ -867,7 +867,7 @@ public void sendRequest(final Request request) {
 				return;
 			}
 		} else if (0 < multicastBaseMid && request.getMID() >= multicastBaseMid) {
-			LOGGER.warn("{}request has mid {}, which is in the MULTICAST_MID range [{}-65535]", tag, destinationAddress,
+			LOGGER.warn("{}request to {} has mid {}, which is in the MULTICAST_MID range [{}-65535]", tag, destinationAddress,
 					request.getMID(), multicastBaseMid);
 			request.setSendError(
 					new IllegalArgumentException("unicast mid is in multicast range [" + multicastBaseMid + "-65535]"));
@@ -1101,9 +1101,7 @@ public void sendResponse(Exchange exchange, Response response) {
 
 			// MessageInterceptor might have canceled
 			if (response.isCanceled() || response.getSendError() != null) {
-				if (null != exchange) {
-					exchange.executeComplete();
-				}
+				exchange.executeComplete();
 			} else {
 				RawData data = serializer.serializeResponse(response,
 						new ExchangeCallback<Response>(exchange, response) {

californium-core/src/main/java/org/eclipse/californium/core/network/InMemoryMessageExchangeStore.java

@@ -486,7 +486,7 @@ private <K> void dumpExchanges(int logMaxExchanges, Set<Entry<K, Exchange>> exch
 			Request origin = exchange.getRequest();
 			Request current = exchange.getCurrentRequest();
 			String pending = exchange.getRetransmissionHandle() == null ? "" : "/pending";
-			if (origin != current && !origin.getToken().equals(current.getToken())) {
+			if (origin != null && origin != current && !origin.getToken().equals(current.getToken())) {
 				HEALTH_LOGGER.debug("  {}, {}, retransmission {}{}, org {}, {}, {}", exchangeEntry.getKey(),
 						exchange, exchange.getFailedTransmissionCount(), pending, origin.getToken(),
 						current, exchange.getCurrentResponse());

californium-core/src/main/java/org/eclipse/californium/core/network/InMemoryMessageIdProvider.java

@@ -88,7 +88,7 @@ public InMemoryMessageIdProvider(final NetworkConfig config) {
 			throw new NullPointerException("Config must not be null");
 		}
 		String textualMode = null;
-		TrackerMode mode = TrackerMode.GROUPED;
+		TrackerMode mode;
 		try {
 			textualMode = config.getString(NetworkConfig.Keys.MID_TRACKER);
 			mode = TrackerMode.valueOf(textualMode);

californium-core/src/main/java/org/eclipse/californium/core/network/config/NetworkConfig.java

@@ -575,6 +575,10 @@ public String getString(final String key, final String defaultValue) {
 	 */
 	public Integer getOptInteger(final String key) {
 		return getNumberValue(new PropertyParser<Integer>() {
+			@Override
+			public String getTypeName() {
+				return "Integer";
+			}
 
 			@Override
 			public Integer parseValue(String value) {
@@ -593,6 +597,10 @@ public Integer parseValue(String value) {
 	 */
 	public Long getOptLong(final String key) {
 		return getNumberValue(new PropertyParser<Long>() {
+			@Override
+			public String getTypeName() {
+				return "Long";
+			}
 
 			@Override
 			public Long parseValue(String value) {
@@ -623,6 +631,10 @@ public int getInt(final String key) {
 	 */
 	public int getInt(final String key, final int defaultValue) {
 		return getNumberValue(new PropertyParser<Integer>() {
+			@Override
+			public String getTypeName() {
+				return "int";
+			}
 
 			@Override
 			public Integer parseValue(String value) {
@@ -653,6 +665,10 @@ public long getLong(final String key) {
 	 */
 	public long getLong(final String key, final long defaultValue) {
 		return getNumberValue(new PropertyParser<Long>() {
+			@Override
+			public String getTypeName() {
+				return "long";
+			}
 
 			@Override
 			public Long parseValue(String value) {
@@ -684,6 +700,10 @@ public float getFloat(final String key) {
 	 */
 	public float getFloat(final String key, final float defaultValue) {
 		return getNumberValue(new PropertyParser<Float>() {
+			@Override
+			public String getTypeName() {
+				return "float";
+			}
 
 			@Override
 			public Float parseValue(String value) {
@@ -715,6 +735,10 @@ public double getDouble(final String key) {
 	 */
 	public double getDouble(final String key, final double defaultValue) {
 		return getNumberValue(new PropertyParser<Double>() {
+			@Override
+			public String getTypeName() {
+				return "double";
+			}
 
 			@Override
 			public Double parseValue(String value) {
@@ -726,17 +750,16 @@ public Double parseValue(String value) {
 	private <T> T getNumberValue(final PropertyParser<T> parser, final String key, final T defaultValue) {
 		T result = defaultValue;
 		String value = properties.getProperty(key);
-		if (value != null && !value.isEmpty()) {
-			try {
-				result = parser.parseValue(value);
-			} catch (NumberFormatException e) {
-				LOGGER.warn("value for key [{}] is not a {0}, returning default value", key, defaultValue.getClass());
-			}
-		} else if (value == null) {
+		if (value == null) {
 			LOGGER.debug("key [{}] is undefined, returning default value", key);
-		} else {
+		} else if (value.isEmpty()) {
 			LOGGER.debug("key [{}] is empty, returning default value", key);
 		}
+		try {
+			result = parser.parseValue(value);
+		} catch (NumberFormatException e) {
+			LOGGER.warn("value for key [{}] is not a {}, returning default value", key, parser.getTypeName());
+		}
 		return result;
 	}
 
@@ -775,7 +798,7 @@ public boolean getBoolean(String key) {
 	}
 
 	private interface PropertyParser<T> {
-
+		String getTypeName();
 		T parseValue(String value);
 	}
 

californium-core/src/test/java/org/eclipse/californium/core/test/ObserveTest.java

@@ -175,13 +175,13 @@ public void testObserveLifecycle() throws Exception {
 		// ensure relations are established
 		assertNotNull("Client received no response", resp1);
 		assertTrue(resp1.getOptions().hasObserve());
-		assertTrue(resourceX.getObserverCount() == 1);
+		assertEquals(1, resourceX.getObserverCount());
 		assertEquals(resp1.getPayloadString(), resourceX.currentResponse);
 
 		Response resp2 = requestB.waitForResponse(1000);
 		assertNotNull("Client received no response", resp2);
 		assertTrue(resp2.getOptions().hasObserve());
-		assertTrue(resourceY.getObserverCount() == 1);
+		assertEquals(1, resourceY.getObserverCount());
 		assertEquals(resp2.getPayloadString(), resourceY.currentResponse);
 
 		System.out.println(System.lineSeparator() + "Observe relation established, resource changes");
@@ -194,7 +194,7 @@ public void testObserveLifecycle() throws Exception {
 		// (which will go lost, see ClientMessageInterceptor)
 
 		// wait for the server to timeout, see ClientMessageInterceptor.
-		waitforit.await(1000, TimeUnit.MILLISECONDS);
+		assertTrue(waitforit.await(1000, TimeUnit.MILLISECONDS));
 
 		Thread.sleep(500);
 
@@ -203,9 +203,9 @@ public void testObserveLifecycle() throws Exception {
 		// - request B to resource Y
 
 		// check that relations to resource X AND Y have been canceled
-		assertTrue(resourceX.getObserverCount() == 0);
-		assertTrue(resourceY.getObserverCount() == 0);
-		
+		assertEquals(0, resourceX.getObserverCount());
+		assertEquals(0, resourceY.getObserverCount());
+
 		observations.setStoreException(new ObservationStoreException("test"));
 
 		Request requestC = Request.newGet();

californium-core/src/test/java/org/eclipse/californium/core/test/SmallServerClientTest.java

@@ -96,7 +96,10 @@ private CoapServer createSimpleServer() {
 			public void deliverRequest(Exchange exchange) {
 				System.out.println("server received request");
 				exchange.sendAccept();
-				try { Thread.sleep(500); } catch (Exception e) {}
+				try { 
+					Thread.sleep(500);
+				} catch (InterruptedException e) {
+				}
 				Response response = new Response(ResponseCode.CONTENT);
 				response.setConfirmable(false);
 				response.setPayload(SERVER_RESPONSE);

californium-osgi/src/main/java/org/eclipse/californium/osgi/ManagedServer.java

@@ -216,9 +216,11 @@ public void stop() {
 	@Override
 	public Resource addingService(ServiceReference<Resource> reference) {
 		Resource resource = context.getService(reference);
-		LOGGER.debug("Adding resource [{}]", resource.getName());
 		if (resource != null) {
+			LOGGER.debug("Adding resource [{}]", resource.getName());
 			managedServer.add(resource);
+		} else {
+			LOGGER.debug("Failed adding resource for [{}], not available!", reference);
 		}
 		return resource;
 	}

californium-proxy2/src/main/java/org/eclipse/californium/proxy2/MappingProperties.java

@@ -176,7 +176,11 @@ public boolean getBool(String key) {
 
 	public void load(String fileName) throws IOException {
 		InputStream in = new FileInputStream(fileName);
-		load(in);
+		try {
+			load(in);
+		} finally {
+			in.close();
+		}
 	}
 
 	public void set(String key, int value) {
@@ -193,7 +197,11 @@ public void set(String key, boolean value) {
 
 	public void store(String fileName) throws IOException {
 		OutputStream out = new FileOutputStream(fileName);
-		store(out, HEADER);
+		try {
+			store(out, HEADER);
+		} finally {
+			out.close();
+		}
 	}
 
 	private void init() {

cf-utils/cf-cli/src/main/java/org/eclipse/californium/cli/ClientBaseConfig.java

@@ -124,11 +124,11 @@ public void defaults() {
 		// allow quick hostname as argument
 		String scheme = CoAP.getSchemeFromUri(uri);
 		if (scheme == null) {
-			if (authenticationModes != null && !authenticationModes.isEmpty()) {
+			if (authenticationModes.isEmpty()) {
+				uri = CoAP.COAP_URI_SCHEME + "://" + uri;
+			} else {
 				uri = CoAP.COAP_SECURE_URI_SCHEME + "://" + uri;
 				secure = true;
-			} else {
-				uri = CoAP.COAP_URI_SCHEME + "://" + uri;
 			}
 		} else {
 			secure = CoAP.isSecureScheme(scheme);

demo-apps/cf-benchmark/src/main/java/org/eclipse/californium/benchmark/WorkQueueExecutor.java

@@ -87,13 +87,13 @@ public <V> ScheduledFuture<V> schedule(Callable<V> callable, long delay, TimeUni
 	@Override
 	public ScheduledFuture<?> scheduleAtFixedRate(Runnable command,
 			long initialDelay, long period, TimeUnit unit) {
-		return scheduleAtFixedRate(command, initialDelay, period, unit);
+		return timerService.scheduleAtFixedRate(command, initialDelay, period, unit);
 	}
 
 	@Override
 	public ScheduledFuture<?> scheduleWithFixedDelay(Runnable command,
 			long initialDelay, long delay, TimeUnit unit) {
-		return scheduleWithFixedDelay(command, initialDelay, delay, unit);
+		return timerService.scheduleWithFixedDelay(command, initialDelay, delay, unit);
 	}
 
 	@Override

demo-apps/cf-extplugtest-client/src/main/java/org/eclipse/californium/extplugtests/BenchmarkClient.java

@@ -1031,7 +1031,7 @@ public static void main(String[] args) throws InterruptedException, IOException
 				!config.stop ? "none-stop " : "", secure ? "secure " : "", overallRequests, proxyMessage, uri);
 
 		if (config.reverse != null && overallReverseResponses > 0) {
-			if (config.reverse.min == config.reverse.max) {
+			if (config.reverse.min.equals(config.reverse.max)) {
 				System.out.format("Expect %d notifies, interval %d [ms]%n", overallReverseResponses,
 						config.reverse.min);
 			} else {