You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by jf...@apache.org on 2018/11/23 10:26:30 UTC
[incubator-plc4x] branch alias-registry-for-opm updated: [OPM]
Refactoring, added comments from Sebastian.
This is an automated email from the ASF dual-hosted git repository.
jfeinauer pushed a commit to branch alias-registry-for-opm
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git
The following commit(s) were added to refs/heads/alias-registry-for-opm by this push:
new 9cf886a [OPM] Refactoring, added comments from Sebastian.
9cf886a is described below
commit 9cf886aca651a78b6390b343684669a2b5c16e63
Author: Julian Feinauer <j....@pragmaticminds.de>
AuthorDate: Fri Nov 23 11:26:22 2018 +0100
[OPM] Refactoring, added comments from Sebastian.
---
.../apache/plc4x/java/mock/PlcMockConnection.java | 14 +++++---------
.../org/apache/plc4x/java/mock/PlcMockDriver.java | 2 +-
.../java/org/apache/plc4x/java/opm/OpmUtils.java | 22 +++++++++++++---------
.../plc4x/java/opm/PlcEntityInterceptor.java | 6 ++++--
.../apache/plc4x/java/opm/SimpleAliasRegistry.java | 3 ++-
.../plc4x/java/opm/PlcEntityInterceptorTest.java | 6 +++++-
plc4j/utils/opm/src/test/resources/logback.xml | 2 +-
7 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/plc4j/protocols/test/src/main/java/org/apache/plc4x/java/mock/PlcMockConnection.java b/plc4j/protocols/test/src/main/java/org/apache/plc4x/java/mock/PlcMockConnection.java
index cdd3448..9130d7f 100644
--- a/plc4j/protocols/test/src/main/java/org/apache/plc4x/java/mock/PlcMockConnection.java
+++ b/plc4j/protocols/test/src/main/java/org/apache/plc4x/java/mock/PlcMockConnection.java
@@ -21,7 +21,6 @@ package org.apache.plc4x.java.mock;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.plc4x.java.api.PlcConnection;
import org.apache.plc4x.java.api.authentication.PlcAuthentication;
-import org.apache.plc4x.java.api.exceptions.PlcConnectionException;
import org.apache.plc4x.java.api.exceptions.PlcUnsupportedOperationException;
import org.apache.plc4x.java.api.messages.*;
import org.apache.plc4x.java.api.metadata.PlcConnectionMetadata;
@@ -41,16 +40,13 @@ import java.util.stream.Collectors;
public class PlcMockConnection implements PlcConnection, PlcReader {
- private static final Logger logger = LoggerFactory.getLogger(PlcMockConnection.class);
+ private static final Logger LOGGER = LoggerFactory.getLogger(PlcMockConnection.class);
- private final String name;
private final PlcAuthentication authentication;
- private boolean isConnected = false;
private MockDevice device;
- PlcMockConnection(String name, PlcAuthentication authentication) {
- this.name = name;
+ PlcMockConnection(PlcAuthentication authentication) {
this.authentication = authentication;
}
@@ -59,7 +55,7 @@ public class PlcMockConnection implements PlcConnection, PlcReader {
}
public void setDevice(MockDevice device) {
- logger.info("Set Mock Devie on Mock Connection " + this + " with device " + device);
+ LOGGER.info("Set Mock Devie on Mock Connection {} with device {}", this, device);
this.device = device;
}
@@ -75,7 +71,7 @@ public class PlcMockConnection implements PlcConnection, PlcReader {
@Override
public void close() {
- logger.info("Closing MockConnection with device " + device);
+ LOGGER.info("Closing MockConnection with device {}", device);
}
@Override
@@ -109,7 +105,7 @@ public class PlcMockConnection implements PlcConnection, PlcReader {
@Override
public PlcReadResponse get() {
- logger.debug("Sending read request to MockDevice");
+ LOGGER.debug("Sending read request to MockDevice");
Map<String, Pair<PlcResponseCode, BaseDefaultFieldItem>> response = readRequest.getFieldNames().stream()
.collect(Collectors.toMap(
Function.identity(),
diff --git a/plc4j/protocols/test/src/main/java/org/apache/plc4x/java/mock/PlcMockDriver.java b/plc4j/protocols/test/src/main/java/org/apache/plc4x/java/mock/PlcMockDriver.java
index 1b293ae..0a2b1c0 100644
--- a/plc4j/protocols/test/src/main/java/org/apache/plc4x/java/mock/PlcMockDriver.java
+++ b/plc4j/protocols/test/src/main/java/org/apache/plc4x/java/mock/PlcMockDriver.java
@@ -56,7 +56,7 @@ public class PlcMockDriver implements PlcDriver {
if (deviceName.isEmpty()) {
throw new PlcConnectionException("Invalid URL: no device name given.");
}
- return connectionMap.computeIfAbsent(deviceName, name -> new PlcMockConnection(name, authentication));
+ return connectionMap.computeIfAbsent(deviceName, name -> new PlcMockConnection(authentication));
}
}
diff --git a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/OpmUtils.java b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/OpmUtils.java
index 61dfc7c..4ff4526 100644
--- a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/OpmUtils.java
+++ b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/OpmUtils.java
@@ -19,6 +19,8 @@
package org.apache.plc4x.java.opm;
+import org.apache.commons.lang3.Validate;
+
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -49,20 +51,20 @@ final class OpmUtils {
}
static String getOrResolveAddress(AliasRegistry registry, String addressString) {
- if (!OpmUtils.isValidExpression(addressString)) {
+ if (!isValidExpression(addressString)) {
throw new IllegalArgumentException("Invalid Syntax, either use field address (no starting $) " +
"or an alias with Syntax ${xxx}. But given was '" + addressString + "'");
}
- if (OpmUtils.isAlias(addressString)) {
- String alias = OpmUtils.getAlias(addressString);
- if (registry.canResolve(alias)) {
- return registry.resolve(alias);
- } else {
- throw new IllegalArgumentException("Unable to resolve Alias '" + alias + "' in Schema Registry");
- }
- } else {
+ if (!isAlias(addressString)) {
return addressString;
}
+ String alias = getAlias(addressString);
+ if (registry.canResolve(alias)) {
+ return registry.resolve(alias);
+ } else {
+ throw new IllegalArgumentException("Unable to resolve Alias '" + alias + "' in Schema Registry");
+ }
+
}
/**
@@ -70,10 +72,12 @@ final class OpmUtils {
* either an Address or an alias ${xxx}.
*/
static boolean isValidExpression(String s) {
+ Validate.notNull(s);
return !s.startsWith("$") || pattern.matcher(s).matches();
}
static boolean isAlias(String s) {
+ Validate.notNull(s);
return s.startsWith("$") && pattern.matcher(s).matches();
}
diff --git a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityInterceptor.java b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityInterceptor.java
index 9f99378..7671563 100644
--- a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityInterceptor.java
+++ b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityInterceptor.java
@@ -24,6 +24,7 @@ import org.apache.commons.configuration2.Configuration;
import org.apache.commons.configuration2.SystemConfiguration;
import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Validate;
import org.apache.plc4x.java.PlcDriverManager;
import org.apache.plc4x.java.api.PlcConnection;
import org.apache.plc4x.java.api.exceptions.PlcConnectionException;
@@ -150,7 +151,7 @@ public class PlcEntityInterceptor {
static void refetchAllFields(Object proxy, PlcDriverManager driverManager, String address, AliasRegistry registry, Map<String, Instant> lastFetched) throws OPMException {
// Don't log o here as this would cause a second request against a plc so don't touch it, or if you log be aware of that
Class<?> entityClass = proxy.getClass().getSuperclass();
- LOGGER.trace("Refetching all fields on proxy object of class " + entityClass);
+ LOGGER.trace("Refetching all fields on proxy object of class {}", entityClass);
PlcEntity plcEntity = entityClass.getAnnotation(PlcEntity.class);
if (plcEntity == null) {
throw new OPMException("Non PlcEntity supplied");
@@ -173,7 +174,7 @@ public class PlcEntityInterceptor {
PlcReadRequest request = requestBuilder.build();
- LOGGER.trace("Request for refetch of " + entityClass + " was build and is " + request.toString());
+ LOGGER.trace("Request for refetch of {} was build and is {}", entityClass, request);
PlcReadResponse response = getPlcReadResponse(request);
@@ -205,6 +206,7 @@ public class PlcEntityInterceptor {
* Checks if a field needs to be refetched, i.e., the cached values are too old.
*/
private static boolean needsToBeFetched(Map<String, Instant> lastFetched, Field field) {
+ Validate.notNull(field);
long cacheDurationMillis = field.getAnnotation(PlcField.class).cacheDurationMillis();
String fqn = getFqn(field);
if (lastFetched.containsKey(fqn)) {
diff --git a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/SimpleAliasRegistry.java b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/SimpleAliasRegistry.java
index 10ed5e9..494e55b 100644
--- a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/SimpleAliasRegistry.java
+++ b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/SimpleAliasRegistry.java
@@ -22,6 +22,7 @@ package org.apache.plc4x.java.opm;
import java.util.HashMap;
import java.util.Map;
import java.util.NoSuchElementException;
+import java.util.concurrent.ConcurrentHashMap;
/**
* Simple Map Based Implementation of {@link AliasRegistry}.
@@ -35,7 +36,7 @@ public class SimpleAliasRegistry implements AliasRegistry {
private final Map<String, String> aliasMap;
public SimpleAliasRegistry() {
- this(new HashMap<>());
+ this(new ConcurrentHashMap<>());
}
public SimpleAliasRegistry(Map<String, String> aliasMap) {
diff --git a/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityInterceptorTest.java b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityInterceptorTest.java
index d508690..0450cae 100644
--- a/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityInterceptorTest.java
+++ b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityInterceptorTest.java
@@ -27,6 +27,8 @@ import org.apache.plc4x.java.base.messages.DefaultPlcReadResponse;
import org.junit.Test;
import org.mockito.Mockito;
import org.mockito.stubbing.Answer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.util.Collections;
import java.util.concurrent.CompletableFuture;
@@ -42,6 +44,8 @@ import static org.mockito.Mockito.when;
public class PlcEntityInterceptorTest {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PlcEntityInterceptorTest.class);
+
private void runGetPlcResponseWIthException(Answer a) throws InterruptedException, ExecutionException, TimeoutException, OPMException {
PlcReadRequest request = Mockito.mock(PlcReadRequest.class);
CompletableFuture future = Mockito.mock(CompletableFuture.class);
@@ -63,7 +67,7 @@ public class PlcEntityInterceptorTest {
throw new InterruptedException();
});
} catch (InterruptedException | ExecutionException | TimeoutException e) {
- e.printStackTrace();
+ LOGGER.warn("Fetched exception", e);
} catch (OPMException e) {
exceptionWasThrown.set(true);
}
diff --git a/plc4j/utils/opm/src/test/resources/logback.xml b/plc4j/utils/opm/src/test/resources/logback.xml
index 74570cd..8b49981 100644
--- a/plc4j/utils/opm/src/test/resources/logback.xml
+++ b/plc4j/utils/opm/src/test/resources/logback.xml
@@ -29,7 +29,7 @@
</encoder>
</appender>
- <root level="trace">
+ <root level="debug">
<appender-ref ref="STDOUT"/>
</root>