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 09:27:15 UTC
[incubator-plc4x] 02/05: [OPM] Refactoring + Added timeout / cache
to PlcField.java Annotation / added the functionality of the annotation.
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
commit c5fbff8582e9147dc5cbc2d61c2dac6a99d05f0b
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