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:30:03 UTC

[incubator-plc4x] branch develop updated (d9e475e -> 6e02169)

This is an automated email from the ASF dual-hosted git repository.

jfeinauer pushed a change to branch develop
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git.


    from d9e475e  - Fixed some issues reported during our 0.2.0 release
     new e0b067b  [OPM] Added AliasRegistry and SimpleAliasRegistry as default impl.
     new 95f2d3d  [OPM] Refactoring + Added timeout / cache to PlcField.java Annotation / added the functionality of the annotation.
     new 6e02169  [OPM] Refactoring, added comments from Sebastian.

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/plc4x/java/mock/PlcMockConnection.java  |  25 +++--
 .../org/apache/plc4x/java/mock/PlcMockDriver.java  |   2 +-
 .../org/apache/plc4x/java/opm/AliasRegistry.java}  |  34 ++++--
 .../java/org/apache/plc4x/java/opm/OpmUtils.java   |  49 ++++++++-
 .../plc4x/java/opm/PlcEntityInterceptor.java       |  95 +++++++++++++----
 .../apache/plc4x/java/opm/PlcEntityManager.java    |  36 +++++--
 .../java/org/apache/plc4x/java/opm/PlcField.java   |   2 -
 .../apache/plc4x/java/opm/SimpleAliasRegistry.java |  75 +++++++++++++
 .../apache/plc4x/java/opm/ConnectedEntityTest.java | 117 +++++++++++++++++++++
 .../org/apache/plc4x/java/opm/OpmUtilsTest.java    |  64 +++++++++++
 .../plc4x/java/opm/PlcEntityInterceptorTest.java   |  31 +++++-
 .../plc4x/java/opm/PlcEntityManagerTest.java       | 100 +++++++++++++++++-
 .../plc4x/java/opm/SimpleAliasRegistryTest.java    |  76 +++++++++++++
 13 files changed, 639 insertions(+), 67 deletions(-)
 copy plc4j/{protocols/test/src/main/java/org/apache/plc4x/java/mock/MockDevice.java => utils/opm/src/main/java/org/apache/plc4x/java/opm/AliasRegistry.java} (50%)
 create mode 100644 plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/SimpleAliasRegistry.java
 create mode 100644 plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/ConnectedEntityTest.java
 create mode 100644 plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/OpmUtilsTest.java
 create mode 100644 plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/SimpleAliasRegistryTest.java


[incubator-plc4x] 01/03: [OPM] Added AliasRegistry and SimpleAliasRegistry as default impl.

Posted by jf...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jfeinauer pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit e0b067bb2a2460fd8ffbbd8ee6971e9a89049a09
Author: Julian Feinauer <j....@pragmaticminds.de>
AuthorDate: Thu Nov 22 16:41:03 2018 +0100

    [OPM] Added AliasRegistry and SimpleAliasRegistry as default impl.
---
 .../apache/plc4x/java/mock/PlcMockConnection.java  |  19 ++--
 .../org/apache/plc4x/java/opm/AliasRegistry.java   |  54 +++++++++++
 .../java/org/apache/plc4x/java/opm/OpmUtils.java   |  46 ++++++++++
 .../plc4x/java/opm/PlcEntityInterceptor.java       |  38 ++++----
 .../apache/plc4x/java/opm/PlcEntityManager.java    |  27 ++++--
 .../apache/plc4x/java/opm/SimpleAliasRegistry.java |  74 +++++++++++++++
 .../org/apache/plc4x/java/opm/OpmUtilsTest.java    |  64 +++++++++++++
 .../plc4x/java/opm/PlcEntityInterceptorTest.java   |  27 ++++--
 .../plc4x/java/opm/PlcEntityManagerTest.java       | 100 ++++++++++++++++++++-
 .../plc4x/java/opm/SimpleAliasRegistryTest.java    |  76 ++++++++++++++++
 plc4j/utils/opm/src/test/resources/logback.xml     |   2 +-
 11 files changed, 487 insertions(+), 40 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 5bf5941..cdd3448 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
@@ -30,6 +30,8 @@ import org.apache.plc4x.java.base.messages.DefaultPlcReadRequest;
 import org.apache.plc4x.java.base.messages.DefaultPlcReadResponse;
 import org.apache.plc4x.java.base.messages.PlcReader;
 import org.apache.plc4x.java.base.messages.items.BaseDefaultFieldItem;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
@@ -39,6 +41,8 @@ import java.util.stream.Collectors;
 
 public class PlcMockConnection implements PlcConnection, PlcReader {
 
+    private static final Logger logger = LoggerFactory.getLogger(PlcMockConnection.class);
+
     private final String name;
     private final PlcAuthentication authentication;
 
@@ -55,6 +59,7 @@ public class PlcMockConnection implements PlcConnection, PlcReader {
     }
 
     public void setDevice(MockDevice device) {
+        logger.info("Set Mock Devie on Mock Connection " + this + " with device " + device);
         this.device = device;
     }
 
@@ -65,14 +70,12 @@ public class PlcMockConnection implements PlcConnection, PlcReader {
 
     @Override
     public boolean isConnected() {
-        // is connected if a device is set
-        return device != null;
+        return true;
     }
 
     @Override
     public void close() {
-        // unset device
-        this.device = null;
+        logger.info("Closing MockConnection with device " + device);
     }
 
     @Override
@@ -103,10 +106,16 @@ public class PlcMockConnection implements PlcConnection, PlcReader {
     @Override
     public CompletableFuture<PlcReadResponse> read(PlcReadRequest readRequest) {
         return CompletableFuture.supplyAsync(new Supplier<PlcReadResponse>() {
+
             @Override
             public PlcReadResponse get() {
+                logger.debug("Sending read request to MockDevice");
                 Map<String, Pair<PlcResponseCode, BaseDefaultFieldItem>> response = readRequest.getFieldNames().stream()
-                    .collect(Collectors.toMap(Function.identity(), name -> device.read(((MockField) readRequest.getField(name)).getFieldQuery())));
+                    .collect(Collectors.toMap(
+                        Function.identity(),
+                        name -> device.read(((MockField) readRequest.getField(name)).getFieldQuery())
+                        )
+                    );
                 return new DefaultPlcReadResponse((DefaultPlcReadRequest)readRequest, response);
             }
         });
diff --git a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/AliasRegistry.java b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/AliasRegistry.java
new file mode 100644
index 0000000..943a98f
--- /dev/null
+++ b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/AliasRegistry.java
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.plc4x.java.opm;
+
+/**
+ * This interface can be used to give "aliases" for field names in {@link PlcEntity}s {@link PlcField} strings.
+ * These are then resolved.
+ */
+public interface AliasRegistry {
+
+    /**
+     * Checks if this registry can resolve this alias
+     */
+    boolean canResolve(String alias);
+
+    /**
+     * Checks if this registry can resolve this alias for the given connection.
+     */
+    boolean canResolve(String connection, String alias);
+
+    /**
+     * Resolves an alias to a valid PLC Field Adress
+     * @param alias
+     * @return
+     */
+    String resolve(String alias);
+
+    /**
+     * Resolves an alias to a valid PLC Field based on the connection.
+     * This means that the same alias could be resolved to different Addresses for different connections.
+     * @param connection
+     * @param alias
+     * @return
+     */
+    String resolve(String connection, String alias);
+
+}
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 29ede9c..0791a93 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,11 +19,17 @@
 
 package org.apache.plc4x.java.opm;
 
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 /**
  * Utility methods for usage with OPM.
  */
 public final class OpmUtils {
 
+    public static final String ADDRESS = "address";
+    static final Pattern pattern = Pattern.compile("^\\$\\{(?<" + ADDRESS + ">.*)\\}$");
+
     private OpmUtils() {
         // Util class
     }
@@ -42,4 +48,44 @@ public final class OpmUtils {
         return annotation;
     }
 
+    static String getOrResolveAddress(AliasRegistry registry, String addressString) {
+        if (!OpmUtils.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 {
+            return addressString;
+        }
+    }
+
+    /**
+     * Checks whether a given String is a valid OPM Expression, this means
+     * either an Address or an alias ${xxx}.
+     *
+     * @param s
+     * @return
+     */
+    static boolean isValidExpression(String s) {
+        return (s.startsWith("$") && pattern.matcher(s).matches()) || s.startsWith("$") == false;
+    }
+
+    static boolean isAlias(String s) {
+        return s.startsWith("$") && pattern.matcher(s).matches();
+    }
+
+    static String getAlias(String s) {
+        Matcher matcher = pattern.matcher(s);
+        if (matcher.matches() == false) {
+            throw new IllegalArgumentException("Invalid Syntax, no Alias found in String '" + s + "'. Synatx is ${xxx}");
+        }
+        return matcher.group(ADDRESS);
+    }
+
 }
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 00c0fe0..2e41155 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
@@ -49,7 +49,7 @@ import java.util.concurrent.TimeoutException;
 
 /**
  * Interceptor for dynamic functionality of @{@link PlcEntity}.
- * Basically, its {@link #intercept(Object, Method, Callable, String, PlcDriverManager)} method is called for each
+ * Basically, its {@link #intercept(Object, Method, Callable, String, PlcDriverManager, AliasRegistry)} method is called for each
  * invocation of a method on a connected @{@link PlcEntity} and does then the dynamic part.
  *
  * For those not too familiar with the JVM's dispatch on can roughly imagine the intercept method being a "regular"
@@ -71,9 +71,9 @@ public class PlcEntityInterceptor {
     /**
      * Basic Intersector for all methods on the proxy object.
      * It checks if the invoked method is a getter and if so, only retrieves the requested field, forwarding to
-     * the {@link #fetchValueForGetter(Method, PlcDriverManager,String)} method.
+     * the {@link #fetchValueForGetter(Method, PlcDriverManager, String, AliasRegistry)} method.
      * <p>
-     * If the field is no getter, then all fields are refreshed by calling {@link #refetchAllFields(Object, PlcDriverManager, String)}
+     * If the field is no getter, then all fields are refreshed by calling {@link #refetchAllFields(Object, PlcDriverManager, String, AliasRegistry)}
      * and then, the method is invoked.
      *
      * @param proxy    Object to intercept
@@ -87,8 +87,9 @@ public class PlcEntityInterceptor {
     @SuppressWarnings("unused")
     @RuntimeType
     public static Object intercept(@This Object proxy, @Origin Method method, @SuperCall Callable<?> callable,
-           @FieldValue(PlcEntityManager.PLC_ADDRESS_FIELD_NAME) String address,
-           @FieldValue(PlcEntityManager.DRIVER_MANAGER_FIELD_NAME) PlcDriverManager driverManager) throws OPMException {
+                                   @FieldValue(PlcEntityManager.PLC_ADDRESS_FIELD_NAME) String address,
+                                   @FieldValue(PlcEntityManager.DRIVER_MANAGER_FIELD_NAME) PlcDriverManager driverManager,
+                                   @FieldValue(PlcEntityManager.ALIAS_REGISTRY) AliasRegistry registry) throws OPMException {
         LOGGER.trace("Invoked method {} on connected PlcEntity {}", method.getName(), method.getDeclaringClass().getName());
 
         // If "detached" (i.e. _driverManager is null) simply forward the call
@@ -108,7 +109,7 @@ public class PlcEntityInterceptor {
             // Fetch single value
             LOGGER.trace("Invoked method {} is getter, trying to find annotated field and return requested value",
                 method.getName());
-            return fetchValueForGetter(method, driverManager, address);
+            return fetchValueForGetter(method, driverManager, address, registry);
         }
 
         if (method.getName().startsWith("is") && (method.getReturnType() == boolean.class || method.getReturnType() == Boolean.class)) {
@@ -118,13 +119,13 @@ public class PlcEntityInterceptor {
             // Fetch single value
             LOGGER.trace("Invoked method {} is boolean flag method, trying to find annotated field and return requested value",
                 method.getName());
-            return fetchValueForIsGetter(method, driverManager, address);
+            return fetchValueForIsGetter(method, driverManager, address, registry);
         }
 
         // Fetch all values, than invoke method
         try {
             LOGGER.trace("Invoked method is no getter, refetch all fields and invoke method {} then", method.getName());
-            refetchAllFields(proxy, driverManager, address);
+            refetchAllFields(proxy, driverManager, address, registry);
             return callable.call();
         } catch (Exception e) {
             throw new OPMException("Unable to forward invocation " + method.getName() + " on connected PlcEntity", e);
@@ -136,12 +137,14 @@ public class PlcEntityInterceptor {
      *
      * @param proxy Object to refresh the fields on.
      * @param driverManager
+     * @param registry
      * @throws OPMException on various errors.
      */
     @SuppressWarnings("squid:S1141") // Nested try blocks readability is okay, move to other method makes it imho worse
-    static void refetchAllFields(Object proxy, PlcDriverManager driverManager, String address) throws OPMException {
+    static void refetchAllFields(Object proxy, PlcDriverManager driverManager, String address, AliasRegistry registry) 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);
         PlcEntity plcEntity = entityClass.getAnnotation(PlcEntity.class);
         if (plcEntity == null) {
             throw new OPMException("Non PlcEntity supplied");
@@ -157,12 +160,14 @@ public class PlcEntityInterceptor {
                 .forEach(field ->
                     requestBuilder.addItem(
                         field.getDeclaringClass().getName() + "." + field.getName(),
-                        field.getAnnotation(PlcField.class).value()
+                        OpmUtils.getOrResolveAddress(registry, field.getAnnotation(PlcField.class).value())
                     )
                 );
 
             PlcReadRequest request = requestBuilder.build();
 
+            LOGGER.trace("Request for refetch of " + entityClass + " was build and is " + request.toString());
+
             PlcReadResponse response = getPlcReadResponse(request);
 
             // Fill all requested fields
@@ -182,15 +187,16 @@ public class PlcEntityInterceptor {
         }
     }
 
-    private static Object fetchValueForIsGetter(Method m, PlcDriverManager driverManager, String address) throws OPMException {
-        return fetchValueForGetter(m, 2, driverManager, address);
+    private static Object fetchValueForIsGetter(Method m, PlcDriverManager driverManager, String address, AliasRegistry registry) throws OPMException {
+        return fetchValueForGetter(m, 2, driverManager, address, registry);
     }
 
-    private static Object fetchValueForGetter(Method m, PlcDriverManager driverManager, String address) throws OPMException {
-        return fetchValueForGetter(m, 3, driverManager, address);
+    private static Object fetchValueForGetter(Method m, PlcDriverManager driverManager, String address, AliasRegistry registry) throws OPMException {
+        return fetchValueForGetter(m, 3, driverManager, address, registry);
     }
 
-    private static Object fetchValueForGetter(Method m, int prefixLength, PlcDriverManager driverManager, String address) throws OPMException {
+    private static Object fetchValueForGetter(Method m, int prefixLength, PlcDriverManager driverManager,
+                                              String address, AliasRegistry registry) throws OPMException {
         String s = m.getName().substring(prefixLength);
         // First char to lower
         String variable = s.substring(0, 1).toLowerCase().concat(s.substring(1));
@@ -210,7 +216,7 @@ public class PlcEntityInterceptor {
             String fqn = field.getDeclaringClass().getName() + "." + field.getName();
 
             PlcReadRequest request = connection.readRequestBuilder()
-                .addItem(fqn, annotation.value())
+                .addItem(fqn, OpmUtils.getOrResolveAddress(registry, annotation.value()))
                 .build();
 
             PlcReadResponse response = getPlcReadResponse(request);
diff --git a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java
index 0c0b4b2..a27fed7 100644
--- a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java
+++ b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java
@@ -22,8 +22,6 @@ package org.apache.plc4x.java.opm;
 import net.bytebuddy.ByteBuddy;
 import net.bytebuddy.description.modifier.Visibility;
 import net.bytebuddy.implementation.MethodDelegation;
-import org.apache.commons.configuration2.Configuration;
-import org.apache.commons.configuration2.SystemConfiguration;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.reflect.FieldUtils;
 import org.apache.plc4x.java.PlcDriverManager;
@@ -41,6 +39,8 @@ import java.util.Arrays;
 import java.util.concurrent.Callable;
 
 import static net.bytebuddy.matcher.ElementMatchers.any;
+import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
+import static net.bytebuddy.matcher.ElementMatchers.not;
 
 /**
  * Plc4x equivalent of Jpas EntityManager for implementing Object-Plc-Mapping.
@@ -74,7 +74,7 @@ import static net.bytebuddy.matcher.ElementMatchers.any;
  * A connected @{@link PlcEntity} can be disconnected calling {@link #disconnect(Object)}, then it behaves like the
  * regular Pojo it was before.
  * <p>
- * All invocations on the getters are forwarded to the {@link PlcEntityInterceptor#intercept(Object, Method, Callable, String, PlcDriverManager)}
+ * All invocations on the getters are forwarded to the {@link PlcEntityInterceptor#intercept(Object, Method, Callable, String, PlcDriverManager, AliasRegistry)}
  * method.
  */
 public class PlcEntityManager {
@@ -83,15 +83,22 @@ public class PlcEntityManager {
 
     public static final String PLC_ADDRESS_FIELD_NAME = "_plcAddress";
     static final String DRIVER_MANAGER_FIELD_NAME = "_driverManager";
+    static final String ALIAS_REGISTRY = "_aliasRegistry";
 
     private final PlcDriverManager driverManager;
+    private final SimpleAliasRegistry registry;
 
     public PlcEntityManager() {
-        this.driverManager = new PlcDriverManager();
+        this(new PlcDriverManager());
     }
 
     public PlcEntityManager(PlcDriverManager driverManager) {
+        this(driverManager, new SimpleAliasRegistry());
+    }
+
+    public PlcEntityManager(PlcDriverManager driverManager, SimpleAliasRegistry registry) {
         this.driverManager = driverManager;
+        this.registry = registry;
     }
 
     public <T> T read(Class<T> clazz, String address) throws OPMException {
@@ -109,7 +116,7 @@ public class PlcEntityManager {
                 .forEach(field ->
                     requestBuilder.addItem(
                         field.getDeclaringClass().getName() + "." + field.getName(),
-                        field.getAnnotation(PlcField.class).value()
+                        OpmUtils.getOrResolveAddress(registry, field.getAnnotation(PlcField.class).value())
                     )
                 );
 
@@ -135,7 +142,7 @@ public class PlcEntityManager {
         } catch (InstantiationException | InvocationTargetException | NoSuchMethodException | NoSuchFieldException | IllegalAccessException e) {
             throw new OPMException("Unable to fetch PlcEntity " + clazz.getName(), e);
         } catch (Exception e) {
-            throw new OPMException("Unknown Error", e);
+            throw new OPMException("Unexpected Exception: " + e.getMessage(), e);
         }
     }
 
@@ -156,7 +163,8 @@ public class PlcEntityManager {
                 .subclass(clazz)
                 .defineField(PLC_ADDRESS_FIELD_NAME, String.class, Visibility.PRIVATE)
                 .defineField(DRIVER_MANAGER_FIELD_NAME, PlcDriverManager.class, Visibility.PRIVATE)
-                .method(any()).intercept(MethodDelegation.to(PlcEntityInterceptor.class))
+                .defineField(ALIAS_REGISTRY, AliasRegistry.class, Visibility.PRIVATE)
+                .method(not(isDeclaredBy(Object.class))).intercept(MethodDelegation.to(PlcEntityInterceptor.class))
                 .make()
                 .load(Thread.currentThread().getContextClassLoader())
                 .getLoaded()
@@ -165,9 +173,10 @@ public class PlcEntityManager {
             // Set connection value into the private field
             FieldUtils.writeDeclaredField(instance, PLC_ADDRESS_FIELD_NAME, address, true);
             FieldUtils.writeDeclaredField(instance, DRIVER_MANAGER_FIELD_NAME, driverManager, true);
+            FieldUtils.writeDeclaredField(instance, ALIAS_REGISTRY, registry, true);
 
             // Initially fetch all values
-            PlcEntityInterceptor.refetchAllFields(instance, driverManager, address);
+            PlcEntityInterceptor.refetchAllFields(instance, driverManager, address, registry);
 
             return instance;
         } catch (NoSuchMethodException | InvocationTargetException | InstantiationException | IllegalAccessException | IllegalAccessError e) {
@@ -193,7 +202,7 @@ public class PlcEntityManager {
             }
             FieldUtils.writeDeclaredField(entity, DRIVER_MANAGER_FIELD_NAME, null, true);
         } catch (IllegalAccessException e) {
-            throw new OPMException("Unbale to fetch driverManager instance on entity instance", e);
+            throw new OPMException("Unable to fetch driverManager instance on entity instance", e);
         }
     }
 
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
new file mode 100644
index 0000000..10ed5e9
--- /dev/null
+++ b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/SimpleAliasRegistry.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.plc4x.java.opm;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.NoSuchElementException;
+
+/**
+ * Simple Map Based Implementation of {@link AliasRegistry}.
+ * It is not connection specific and forwards connection aware methods to the "simple" methods.
+ */
+public class SimpleAliasRegistry implements AliasRegistry {
+
+    /**
+     * Map from alias -> plc field address
+     */
+    private final Map<String, String> aliasMap;
+
+    public SimpleAliasRegistry() {
+        this(new HashMap<>());
+    }
+
+    public SimpleAliasRegistry(Map<String, String> aliasMap) {
+        this.aliasMap = aliasMap;
+    }
+
+    /**
+     * Register an Alias in the Registry.
+     */
+    public void register(String alias, String address) {
+        this.aliasMap.put(alias, address);
+    }
+
+    @Override
+    public boolean canResolve(String connection, String alias) {
+        return canResolve(alias);
+    }
+
+    @Override
+    public String resolve(String connection, String alias) {
+        return resolve(alias);
+    }
+
+    @Override
+    public boolean canResolve(String alias) {
+        return aliasMap.containsKey(alias);
+    }
+
+    @Override
+    public String resolve(String alias) {
+        if (!canResolve(alias)) {
+            throw new NoSuchElementException("Unable to resolve '" + alias + "'");
+        }
+        return aliasMap.get(alias);
+    }
+}
diff --git a/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/OpmUtilsTest.java b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/OpmUtilsTest.java
new file mode 100644
index 0000000..27694d4
--- /dev/null
+++ b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/OpmUtilsTest.java
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.plc4x.java.opm;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class OpmUtilsTest {
+
+    @Test
+    public void expression_matches() {
+        assertTrue(OpmUtils.pattern.matcher("${Hallo}").matches());
+        assertTrue(OpmUtils.pattern.matcher("${Hallo:Hallo}").matches());
+        // ...
+        assertTrue(OpmUtils.pattern.matcher("${Ha{}llo}").matches());
+    }
+
+    @Test
+    public void getAlias_matches() {
+        String alias = OpmUtils.getAlias("${hallo}");
+
+        assertEquals("hallo", alias);
+    }
+
+    @Test
+    public void isAlias_bothCases() {
+        // True
+        assertTrue(OpmUtils.isAlias("${hallo}"));
+        assertTrue(OpmUtils.isAlias("${hal{}lo}"));
+        assertTrue(OpmUtils.isAlias("${hallo:hallo}"));
+        // False
+        assertFalse(OpmUtils.isAlias("hallo"));
+        assertFalse(OpmUtils.isAlias("${hallo"));
+        assertFalse(OpmUtils.isAlias("${ha}llo"));
+    }
+
+    @Test
+    public void isValidExpression_startingDollar_false() {
+        assertFalse(OpmUtils.isValidExpression("${hallo"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void getAlias_illegalString_throws() {
+        OpmUtils.getAlias("hallo");
+    }
+}
\ No newline at end of file
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 7108474..d508690 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
@@ -32,8 +32,10 @@ import java.util.Collections;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.when;
@@ -49,12 +51,27 @@ public class PlcEntityInterceptorTest {
         PlcEntityInterceptor.getPlcReadResponse(request);
     }
 
-    @Test(expected = OPMException.class)
-    public void getPlcReadResponse_catchesInterruptedException_rethrows() throws OPMException, InterruptedException, ExecutionException, TimeoutException {
-        runGetPlcResponseWIthException(invocation -> {
-            throw new InterruptedException();
+    @Test
+    public void getPlcReadResponse_catchesInterruptedException_rethrows() throws InterruptedException {
+        AtomicBoolean exceptionWasThrown = new AtomicBoolean(false);
+        // Run in different Thread
+        Thread thread = new Thread(new Runnable() {
+            @Override
+            public void run() {
+                try {
+                    runGetPlcResponseWIthException(invocation -> {
+                        throw new InterruptedException();
+                    });
+                } catch (InterruptedException | ExecutionException | TimeoutException e) {
+                    e.printStackTrace();
+                } catch (OPMException e) {
+                    exceptionWasThrown.set(true);
+                }
+            }
         });
-        return;
+        thread.start();
+        thread.join();
+        assertTrue(exceptionWasThrown.get());
     }
 
     @Test(expected = OPMException.class)
diff --git a/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityManagerTest.java b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityManagerTest.java
index 429b100..e97bf6e 100644
--- a/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityManagerTest.java
+++ b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/PlcEntityManagerTest.java
@@ -35,13 +35,15 @@ import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
-import java.util.concurrent.CompletableFuture;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 public class PlcEntityManagerTest {
 
@@ -133,6 +135,66 @@ public class PlcEntityManagerTest {
         UninstantiableEntity entity = entityManager.connect(UninstantiableEntity.class, "mock:test");
     }
 
+    @Test
+    public void read_resolveAlias_works() throws OPMException, PlcConnectionException {
+        SimpleAliasRegistry registry = new SimpleAliasRegistry();
+        registry.register("alias", "real_field");
+
+        // Mock
+        PlcDriverManager driverManager = new PlcDriverManager();
+        PlcMockConnection connection = (PlcMockConnection) driverManager.getConnection("mock:test");
+        MockDevice mockDevice = Mockito.mock(MockDevice.class);
+        when(mockDevice.read(any())).thenReturn(Pair.of(PlcResponseCode.OK, new DefaultStringFieldItem("value")));
+        connection.setDevice(mockDevice);
+
+        PlcEntityManager entityManager = new PlcEntityManager(driverManager, registry);
+        entityManager.read(AliasEntity.class, "mock:test");
+
+        // Assert that "field" was queried
+        verify(mockDevice).read(eq("real_field"));
+    }
+
+    @Test
+    public void connect_resolveAlias_works() throws PlcConnectionException, OPMException {
+        SimpleAliasRegistry registry = new SimpleAliasRegistry();
+        registry.register("alias", "real_field");
+
+        // Mock
+        PlcDriverManager driverManager = new PlcDriverManager();
+        PlcMockConnection connection = (PlcMockConnection) driverManager.getConnection("mock:test");
+        MockDevice mockDevice = Mockito.mock(MockDevice.class);
+        when(mockDevice.read(any())).thenReturn(Pair.of(PlcResponseCode.OK, new DefaultStringFieldItem("value")));
+        connection.setDevice(mockDevice);
+
+        PlcEntityManager entityManager = new PlcEntityManager(driverManager, registry);
+        entityManager.connect(AliasEntity.class, "mock:test");
+
+        // Assert that "field" was queried
+        verify(mockDevice, times(1)).read(eq("real_field"));
+    }
+
+    @Test(expected = OPMException.class)
+    public void read_unknownAlias_throws() throws OPMException {
+        PlcEntityManager entityManager = new PlcEntityManager();
+
+        entityManager.read(AliasEntity.class, "mock:test");
+    }
+
+    @Test
+    public void read_badAlias_throws() {
+        PlcEntityManager entityManager = new PlcEntityManager();
+
+        String message = null;
+        try {
+            entityManager.read(BadAliasEntity.class, "mock:test");
+        } catch (OPMException e) {
+            message = e.getMessage();
+        }
+
+        assertNotNull(message);
+        assertTrue(message.contains("Invalid Syntax, either use field address (no starting $) or an alias with Syntax ${xxx}. But given was"));
+    }
+
     @PlcEntity
     private static class UninstantiableEntity {
 
@@ -157,4 +219,34 @@ public class PlcEntityManagerTest {
         }
     }
 
+    @PlcEntity
+    public static class AliasEntity {
+
+        @PlcField("${alias}")
+        private String aliasedField;
+
+        public AliasEntity() {
+            // for OPM
+        }
+
+        public String getAliasedField() {
+            return aliasedField;
+        }
+    }
+
+    @PlcEntity
+    public static class BadAliasEntity {
+
+        @PlcField("${alias")
+        private String aliasedField;
+
+        public BadAliasEntity() {
+            // for OPM
+        }
+
+        public String getAliasedField() {
+            return aliasedField;
+        }
+    }
+
 }
\ No newline at end of file
diff --git a/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/SimpleAliasRegistryTest.java b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/SimpleAliasRegistryTest.java
new file mode 100644
index 0000000..bc6d8ad
--- /dev/null
+++ b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/SimpleAliasRegistryTest.java
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.plc4x.java.opm;
+
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.NoSuchElementException;
+
+import static org.junit.Assert.*;
+
+public class SimpleAliasRegistryTest {
+
+    public static final String ADDRESS = "DB2:1234";
+    public static final String ALIAS = "some_field";
+
+    @Test
+    public void register_containsValue() {
+        SimpleAliasRegistry registry = new SimpleAliasRegistry();
+        registry.register(ALIAS, ADDRESS);
+
+        // Perform checks
+        checkMethods(registry);
+    }
+
+    @Test
+    public void defaultMap_containsValue() {
+        HashMap<String, String> map = new HashMap<>();
+        map.put(ALIAS, ADDRESS);
+        SimpleAliasRegistry registry = new SimpleAliasRegistry(map);
+
+        // Perform checks
+        checkMethods(registry);
+    }
+
+    @Test
+    public void canResolve_unknownAlias_returnFalse() {
+        SimpleAliasRegistry registry = new SimpleAliasRegistry();
+
+        assertFalse(registry.canResolve(ALIAS));
+    }
+
+    @Test(expected = NoSuchElementException.class)
+    public void resolve_unknownAlias_throws() {
+        SimpleAliasRegistry registry = new SimpleAliasRegistry();
+
+        registry.resolve(ALIAS);
+    }
+
+    private void checkMethods(SimpleAliasRegistry registry) {
+        // Can Resolve
+        assertTrue(registry.canResolve(ALIAS));
+        assertTrue(registry.canResolve("unknown_connection", ALIAS));
+
+        // Resolve
+        assertEquals(ADDRESS, registry.resolve(ALIAS));
+        assertEquals(ADDRESS, registry.resolve("unknown_connection", ALIAS));
+    }
+}
\ No newline at end of file
diff --git a/plc4j/utils/opm/src/test/resources/logback.xml b/plc4j/utils/opm/src/test/resources/logback.xml
index 8b49981..74570cd 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="debug">
+  <root level="trace">
     <appender-ref ref="STDOUT"/>
   </root>
 


[incubator-plc4x] 02/03: [OPM] Refactoring + Added timeout / cache to PlcField.java Annotation / added the functionality of the annotation.

Posted by jf...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jfeinauer pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit 95f2d3daa4d1ae311d6ecad11664f371218fd8c3
Author: Julian Feinauer <j....@pragmaticminds.de>
AuthorDate: Fri Nov 23 00:55:23 2018 +0100

    [OPM] Refactoring + Added timeout / cache to PlcField.java Annotation / added the functionality of the annotation.
---
 .../org/apache/plc4x/java/opm/AliasRegistry.java   |   7 +-
 .../java/org/apache/plc4x/java/opm/OpmUtils.java   |  15 ++-
 .../plc4x/java/opm/PlcEntityInterceptor.java       |  85 +++++++++++----
 .../apache/plc4x/java/opm/PlcEntityManager.java    |  13 ++-
 .../java/org/apache/plc4x/java/opm/PlcField.java   |   2 -
 .../apache/plc4x/java/opm/ConnectedEntityTest.java | 117 +++++++++++++++++++++
 6 files changed, 197 insertions(+), 42 deletions(-)

diff --git a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/AliasRegistry.java b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/AliasRegistry.java
index 943a98f..b980fb1 100644
--- a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/AliasRegistry.java
+++ b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/AliasRegistry.java
@@ -36,18 +36,13 @@ public interface AliasRegistry {
     boolean canResolve(String connection, String alias);
 
     /**
-     * Resolves an alias to a valid PLC Field Adress
-     * @param alias
-     * @return
+     * Resolves an alias to a valid PLC Field Address
      */
     String resolve(String alias);
 
     /**
      * Resolves an alias to a valid PLC Field based on the connection.
      * This means that the same alias could be resolved to different Addresses for different connections.
-     * @param connection
-     * @param alias
-     * @return
      */
     String resolve(String connection, String alias);
 
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 0791a93..61dfc7c 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
@@ -25,10 +25,10 @@ import java.util.regex.Pattern;
 /**
  * Utility methods for usage with OPM.
  */
-public final class OpmUtils {
+final class OpmUtils {
 
-    public static final String ADDRESS = "address";
-    static final Pattern pattern = Pattern.compile("^\\$\\{(?<" + ADDRESS + ">.*)\\}$");
+    private static final String ADDRESS = "address";
+    static final Pattern pattern = Pattern.compile("^\\$\\{(?<" + ADDRESS + ">.*)}$");
 
     private OpmUtils() {
         // Util class
@@ -68,12 +68,9 @@ public final class OpmUtils {
     /**
      * Checks whether a given String is a valid OPM Expression, this means
      * either an Address or an alias ${xxx}.
-     *
-     * @param s
-     * @return
      */
     static boolean isValidExpression(String s) {
-        return (s.startsWith("$") && pattern.matcher(s).matches()) || s.startsWith("$") == false;
+        return !s.startsWith("$") || pattern.matcher(s).matches();
     }
 
     static boolean isAlias(String s) {
@@ -82,8 +79,8 @@ public final class OpmUtils {
 
     static String getAlias(String s) {
         Matcher matcher = pattern.matcher(s);
-        if (matcher.matches() == false) {
-            throw new IllegalArgumentException("Invalid Syntax, no Alias found in String '" + s + "'. Synatx is ${xxx}");
+        if (!matcher.matches()) {
+            throw new IllegalArgumentException("Invalid Syntax, no Alias found in String '" + s + "'. Syntax is ${xxx}");
         }
         return matcher.group(ADDRESS);
     }
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 2e41155..9f99378 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
@@ -38,10 +38,13 @@ import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.math.BigDecimal;
 import java.math.BigInteger;
+import java.time.Instant;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.LocalTime;
+import java.time.temporal.ChronoUnit;
 import java.util.Arrays;
+import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
@@ -49,7 +52,7 @@ import java.util.concurrent.TimeoutException;
 
 /**
  * Interceptor for dynamic functionality of @{@link PlcEntity}.
- * Basically, its {@link #intercept(Object, Method, Callable, String, PlcDriverManager, AliasRegistry)} method is called for each
+ * Basically, its {@link #intercept(Object, Method, Callable, String, PlcDriverManager, AliasRegistry, Map)} method is called for each
  * invocation of a method on a connected @{@link PlcEntity} and does then the dynamic part.
  *
  * For those not too familiar with the JVM's dispatch on can roughly imagine the intercept method being a "regular"
@@ -71,9 +74,9 @@ public class PlcEntityInterceptor {
     /**
      * Basic Intersector for all methods on the proxy object.
      * It checks if the invoked method is a getter and if so, only retrieves the requested field, forwarding to
-     * the {@link #fetchValueForGetter(Method, PlcDriverManager, String, AliasRegistry)} method.
+     * the {@link #fetchValueForGetter(Object, Method, PlcDriverManager, String, AliasRegistry, Map)} method.
      * <p>
-     * If the field is no getter, then all fields are refreshed by calling {@link #refetchAllFields(Object, PlcDriverManager, String, AliasRegistry)}
+     * If the field is no getter, then all fields are refreshed by calling {@link #refetchAllFields(Object, PlcDriverManager, String, AliasRegistry, Map)}
      * and then, the method is invoked.
      *
      * @param proxy    Object to intercept
@@ -89,7 +92,8 @@ public class PlcEntityInterceptor {
     public static Object intercept(@This Object proxy, @Origin Method method, @SuperCall Callable<?> callable,
                                    @FieldValue(PlcEntityManager.PLC_ADDRESS_FIELD_NAME) String address,
                                    @FieldValue(PlcEntityManager.DRIVER_MANAGER_FIELD_NAME) PlcDriverManager driverManager,
-                                   @FieldValue(PlcEntityManager.ALIAS_REGISTRY) AliasRegistry registry) throws OPMException {
+                                   @FieldValue(PlcEntityManager.ALIAS_REGISTRY) AliasRegistry registry,
+                                   @FieldValue(PlcEntityManager.LAST_FETCHED) Map<String, Instant> lastFetched) throws OPMException {
         LOGGER.trace("Invoked method {} on connected PlcEntity {}", method.getName(), method.getDeclaringClass().getName());
 
         // If "detached" (i.e. _driverManager is null) simply forward the call
@@ -109,7 +113,8 @@ public class PlcEntityInterceptor {
             // Fetch single value
             LOGGER.trace("Invoked method {} is getter, trying to find annotated field and return requested value",
                 method.getName());
-            return fetchValueForGetter(method, driverManager, address, registry);
+
+            return fetchValueForGetter(proxy, method, driverManager, address, registry, lastFetched);
         }
 
         if (method.getName().startsWith("is") && (method.getReturnType() == boolean.class || method.getReturnType() == Boolean.class)) {
@@ -119,13 +124,13 @@ public class PlcEntityInterceptor {
             // Fetch single value
             LOGGER.trace("Invoked method {} is boolean flag method, trying to find annotated field and return requested value",
                 method.getName());
-            return fetchValueForIsGetter(method, driverManager, address, registry);
+            return fetchValueForIsGetter(proxy, method, driverManager, address, registry, lastFetched);
         }
 
         // Fetch all values, than invoke method
         try {
             LOGGER.trace("Invoked method is no getter, refetch all fields and invoke method {} then", method.getName());
-            refetchAllFields(proxy, driverManager, address, registry);
+            refetchAllFields(proxy, driverManager, address, registry, lastFetched);
             return callable.call();
         } catch (Exception e) {
             throw new OPMException("Unable to forward invocation " + method.getName() + " on connected PlcEntity", e);
@@ -136,12 +141,13 @@ public class PlcEntityInterceptor {
      * Renews all values of all Fields that are annotated with {@link PlcEntity}.
      *
      * @param proxy Object to refresh the fields on.
-     * @param driverManager
-     * @param registry
+     * @param driverManager Driver Manager to use
+     * @param registry AliasRegistry to use
+     * @param lastFetched
      * @throws OPMException on various errors.
      */
     @SuppressWarnings("squid:S1141") // Nested try blocks readability is okay, move to other method makes it imho worse
-    static void refetchAllFields(Object proxy, PlcDriverManager driverManager, String address, AliasRegistry registry) throws OPMException {
+    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);
@@ -157,9 +163,10 @@ public class PlcEntityInterceptor {
 
             Arrays.stream(entityClass.getDeclaredFields())
                 .filter(field -> field.isAnnotationPresent(PlcField.class))
+                .filter(field -> needsToBeFetched(lastFetched, field))
                 .forEach(field ->
                     requestBuilder.addItem(
-                        field.getDeclaringClass().getName() + "." + field.getName(),
+                        getFqn(field),
                         OpmUtils.getOrResolveAddress(registry, field.getAnnotation(PlcField.class).value())
                     )
                 );
@@ -172,6 +179,9 @@ public class PlcEntityInterceptor {
 
             // Fill all requested fields
             for (String fieldName : response.getFieldNames()) {
+                // Fill into Cache
+                lastFetched.put(fieldName, Instant.now());
+
                 LOGGER.trace("Value for field {}  is {}", fieldName, response.getObject(fieldName));
                 String clazzFieldName = StringUtils.substringAfterLast(fieldName, ".");
                 try {
@@ -187,16 +197,33 @@ public class PlcEntityInterceptor {
         }
     }
 
-    private static Object fetchValueForIsGetter(Method m, PlcDriverManager driverManager, String address, AliasRegistry registry) throws OPMException {
-        return fetchValueForGetter(m, 2, driverManager, address, registry);
+    private static String getFqn(Field field) {
+        return field.getDeclaringClass().getName() + "." + field.getName();
+    }
+
+    /**
+     * 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) {
+        long cacheDurationMillis = field.getAnnotation(PlcField.class).cacheDurationMillis();
+        String fqn = getFqn(field);
+        if (lastFetched.containsKey(fqn)) {
+            Instant last = lastFetched.get(fqn);
+            return Instant.now().minus(cacheDurationMillis, ChronoUnit.MILLIS).isAfter(last);
+        }
+        return true;
+    }
+
+    private static Object fetchValueForIsGetter(Object proxy, Method m, PlcDriverManager driverManager, String address, AliasRegistry registry, Map<String, Instant> lastFetched) throws OPMException {
+        return fetchValueForGetter(proxy, m, 2, driverManager, address, registry, lastFetched);
     }
 
-    private static Object fetchValueForGetter(Method m, PlcDriverManager driverManager, String address, AliasRegistry registry) throws OPMException {
-        return fetchValueForGetter(m, 3, driverManager, address, registry);
+    private static Object fetchValueForGetter(Object proxy, Method m, PlcDriverManager driverManager, String address, AliasRegistry registry, Map<String, Instant> lastFetched) throws OPMException {
+        return fetchValueForGetter(proxy, m, 3, driverManager, address, registry, lastFetched);
     }
 
-    private static Object fetchValueForGetter(Method m, int prefixLength, PlcDriverManager driverManager,
-                                              String address, AliasRegistry registry) throws OPMException {
+    private static Object fetchValueForGetter(Object proxy, Method m, int prefixLength, PlcDriverManager driverManager,
+                                              String address, AliasRegistry registry, Map<String, Instant> lastFetched) throws OPMException {
         String s = m.getName().substring(prefixLength);
         // First char to lower
         String variable = s.substring(0, 1).toLowerCase().concat(s.substring(1));
@@ -209,18 +236,32 @@ public class PlcEntityInterceptor {
         } catch (NoSuchFieldException e) {
             throw new OPMException("Unable to identify field with name '" + variable + "' for call to '" + m.getName() + "'", e);
         }
+
+        // Use Fully qualified Name as field index
+        String fqn = getFqn(field);
+
+        // Check if cache is still active
+        if (!needsToBeFetched(lastFetched, field)) {
+            // Return the current value
+            try {
+                field.setAccessible(true);
+                return field.get(proxy);
+            } catch (IllegalAccessException e) {
+                throw new OPMException("Unable to restore cached (previous) value for field '" + field.getName() + "'", e);
+            }
+        }
         try (PlcConnection connection = driverManager.getConnection(address)) {
             // Catch the exception, if no reader present (see below)
 
-            // Use Fully qualified Name as field index
-            String fqn = field.getDeclaringClass().getName() + "." + field.getName();
-
             PlcReadRequest request = connection.readRequestBuilder()
                 .addItem(fqn, OpmUtils.getOrResolveAddress(registry, annotation.value()))
                 .build();
 
             PlcReadResponse response = getPlcReadResponse(request);
 
+            // Fill into Cache
+            lastFetched.put(field.getName(), Instant.now());
+
             return getTyped(m.getReturnType(), response, fqn);
         } catch (ClassCastException e) {
             throw new OPMException("Unable to return response as suitable type", e);
@@ -238,8 +279,8 @@ public class PlcEntityInterceptor {
      * @param response        Response to fetch the response from
      * @param targetFieldName Name of the field in the object
      * @param sourceFieldName Name of the field in the response
-     * @throws NoSuchFieldException
-     * @throws IllegalAccessException
+     * @throws NoSuchFieldException If a field is not present in entity
+     * @throws IllegalAccessException If a field in the entity cannot be accessed
      */
     static void setField(Class<?> clazz, Object o, PlcReadResponse response, String targetFieldName, String sourceFieldName) throws NoSuchFieldException, IllegalAccessException {
         LOGGER.debug("setField on clazz: {}, Object: {}, response: {}, targetFieldName: {}, sourceFieldName:{} ", clazz, o, response, targetFieldName, sourceFieldName);
diff --git a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java
index a27fed7..81ebbea 100644
--- a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java
+++ b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityManager.java
@@ -35,10 +35,12 @@ import org.slf4j.LoggerFactory;
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.time.Instant;
 import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.Callable;
 
-import static net.bytebuddy.matcher.ElementMatchers.any;
 import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
 import static net.bytebuddy.matcher.ElementMatchers.not;
 
@@ -74,7 +76,8 @@ import static net.bytebuddy.matcher.ElementMatchers.not;
  * A connected @{@link PlcEntity} can be disconnected calling {@link #disconnect(Object)}, then it behaves like the
  * regular Pojo it was before.
  * <p>
- * All invocations on the getters are forwarded to the {@link PlcEntityInterceptor#intercept(Object, Method, Callable, String, PlcDriverManager, AliasRegistry)}
+ * All invocations on the getters are forwarded to the
+ * {@link PlcEntityInterceptor#intercept(Object, Method, Callable, String, PlcDriverManager, AliasRegistry, Map)}
  * method.
  */
 public class PlcEntityManager {
@@ -84,6 +87,7 @@ public class PlcEntityManager {
     public static final String PLC_ADDRESS_FIELD_NAME = "_plcAddress";
     static final String DRIVER_MANAGER_FIELD_NAME = "_driverManager";
     static final String ALIAS_REGISTRY = "_aliasRegistry";
+    public static final String LAST_FETCHED = "_lastFetched";
 
     private final PlcDriverManager driverManager;
     private final SimpleAliasRegistry registry;
@@ -164,6 +168,7 @@ public class PlcEntityManager {
                 .defineField(PLC_ADDRESS_FIELD_NAME, String.class, Visibility.PRIVATE)
                 .defineField(DRIVER_MANAGER_FIELD_NAME, PlcDriverManager.class, Visibility.PRIVATE)
                 .defineField(ALIAS_REGISTRY, AliasRegistry.class, Visibility.PRIVATE)
+                .defineField(LAST_FETCHED, Map.class, Visibility.PRIVATE)
                 .method(not(isDeclaredBy(Object.class))).intercept(MethodDelegation.to(PlcEntityInterceptor.class))
                 .make()
                 .load(Thread.currentThread().getContextClassLoader())
@@ -174,9 +179,11 @@ public class PlcEntityManager {
             FieldUtils.writeDeclaredField(instance, PLC_ADDRESS_FIELD_NAME, address, true);
             FieldUtils.writeDeclaredField(instance, DRIVER_MANAGER_FIELD_NAME, driverManager, true);
             FieldUtils.writeDeclaredField(instance, ALIAS_REGISTRY, registry, true);
+            Map<String, Instant> lastFetched = new HashMap<>();
+            FieldUtils.writeDeclaredField(instance, LAST_FETCHED, lastFetched, true);
 
             // Initially fetch all values
-            PlcEntityInterceptor.refetchAllFields(instance, driverManager, address, registry);
+            PlcEntityInterceptor.refetchAllFields(instance, driverManager, address, registry, lastFetched);
 
             return instance;
         } catch (NoSuchMethodException | InvocationTargetException | InstantiationException | IllegalAccessException | IllegalAccessError e) {
diff --git a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcField.java b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcField.java
index cd0474e..f6b499d 100644
--- a/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcField.java
+++ b/plc4j/utils/opm/src/main/java/org/apache/plc4x/java/opm/PlcField.java
@@ -31,7 +31,5 @@ import java.lang.annotation.Target;
 @Target({ElementType.METHOD, ElementType.FIELD})
 public @interface PlcField {
     String value();
-    // TODO enable both annotation values in the Interceptor / Entitymanager
     long cacheDurationMillis() default 1000;
-    boolean throwOnUnavailable() default true;
 }
diff --git a/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/ConnectedEntityTest.java b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/ConnectedEntityTest.java
new file mode 100644
index 0000000..0be09df
--- /dev/null
+++ b/plc4j/utils/opm/src/test/java/org/apache/plc4x/java/opm/ConnectedEntityTest.java
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.plc4x.java.opm;
+
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.plc4x.java.PlcDriverManager;
+import org.apache.plc4x.java.api.exceptions.PlcConnectionException;
+import org.apache.plc4x.java.api.types.PlcResponseCode;
+import org.apache.plc4x.java.base.messages.items.DefaultStringFieldItem;
+import org.apache.plc4x.java.mock.MockDevice;
+import org.apache.plc4x.java.mock.PlcMockConnection;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.util.stream.IntStream;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.*;
+
+/**
+ * Tests for Connected Entities.
+ */
+public class ConnectedEntityTest {
+
+    @Test
+    public void useCache() throws PlcConnectionException, OPMException {
+        // Mock
+        PlcDriverManager driverManager = new PlcDriverManager();
+        PlcMockConnection connection = (PlcMockConnection) driverManager.getConnection("mock:cached");
+        MockDevice mock = Mockito.mock(MockDevice.class);
+        when(mock.read(any())).thenReturn(Pair.of(PlcResponseCode.OK, new DefaultStringFieldItem("hallo")));
+        connection.setDevice(mock);
+        PlcEntityManager entityManager = new PlcEntityManager(driverManager);
+
+        // Trigger a fetch
+        CachingEntity entity = entityManager.connect(CachingEntity.class, "mock:cached");
+        // Trigger second fetch
+        assertEquals("hallo", entity.getField());
+
+        verify(mock, timeout(1_000).times(1)).read(any());
+    }
+
+    @Test
+    public void useCache_timeout_refetches() throws PlcConnectionException, OPMException, InterruptedException {
+        // Mock
+        PlcDriverManager driverManager = new PlcDriverManager();
+        PlcMockConnection connection = (PlcMockConnection) driverManager.getConnection("mock:cached");
+        MockDevice mock = Mockito.mock(MockDevice.class);
+        when(mock.read(any())).thenReturn(Pair.of(PlcResponseCode.OK, new DefaultStringFieldItem("hallo")));
+        connection.setDevice(mock);
+        PlcEntityManager entityManager = new PlcEntityManager(driverManager);
+
+        // Trigger a fetch
+        CachingEntity entity = entityManager.connect(CachingEntity.class, "mock:cached");
+        Thread.sleep(500);
+        // Trigger second fetch
+        assertEquals("hallo", entity.getField());
+
+        verify(mock, timeout(1_000).times(2)).read(any());
+    }
+
+    @Test
+    public void cache_manyRequests_onlyOneToPlc() throws PlcConnectionException, OPMException {
+        // Mock
+        PlcDriverManager driverManager = new PlcDriverManager();
+        PlcMockConnection connection = (PlcMockConnection) driverManager.getConnection("mock:cached");
+        MockDevice mock = Mockito.mock(MockDevice.class);
+        when(mock.read(any())).thenReturn(Pair.of(PlcResponseCode.OK, new DefaultStringFieldItem("hallo")));
+        connection.setDevice(mock);
+        PlcEntityManager entityManager = new PlcEntityManager(driverManager);
+
+        // Trigger a fetch
+        CachingEntity entity = entityManager.connect(CachingEntity.class, "mock:cached");
+        // Trigger Many Fetches via getter
+        IntStream.range(1,100).forEach(i -> entity.getField());
+        IntStream.range(1,100).forEach(i -> entity.dummyMethod());
+
+        verify(mock, timeout(1_000).times(1)).read(any());
+    }
+
+    @PlcEntity
+    public static class CachingEntity {
+
+        @PlcField(value = "address", cacheDurationMillis = 100)
+        private String field;
+
+        public CachingEntity() {
+            // For OPM
+        }
+
+        public String getField() {
+            return field;
+        }
+
+        public void dummyMethod() {
+            // do nothing
+        }
+    }
+}
\ No newline at end of file


[incubator-plc4x] 03/03: [OPM] Refactoring, added comments from Sebastian.

Posted by jf...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jfeinauer pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git

commit 6e0216972630c9b1d6ef5bb33f3f3b46e568a07e
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>