This is an automated email from the ASF dual-hosted git repository.
chrisdutz pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git
The following commit(s) were added to refs/heads/develop by this push:
new f4c5ca4015 bug(plc4j/opm): OPM mishandles List and array fields
f4c5ca4015 is described below
commit f4c5ca401542e97d289318a732b72d846dc5af72
Author: Christofer Dutz <[email protected]>
AuthorDate: Fri Aug 14 16:16:41 2026 +0200
bug(plc4j/opm): OPM mishandles List and array fields
Fixes: #1947
---
.../plc4x/java/opm/PlcEntityInterceptor.java | 105 +++++++++++-
.../apache/plc4x/java/opm/CollectionFieldTest.java | 181 +++++++++++++++++++++
2 files changed, 283 insertions(+), 3 deletions(-)
diff --git
a/plc4j/tools/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityInterceptor.java
b/plc4j/tools/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityInterceptor.java
index 7b731be905..39a3358be8 100644
---
a/plc4j/tools/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityInterceptor.java
+++
b/plc4j/tools/opm/src/main/java/org/apache/plc4x/java/opm/PlcEntityInterceptor.java
@@ -31,8 +31,11 @@ import org.apache.plc4x.java.api.types.PlcResponseCode;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.lang.reflect.Array;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.time.Instant;
@@ -40,8 +43,11 @@ import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.Callable;
@@ -378,7 +384,7 @@ public class PlcEntityInterceptor {
// Fill into Cache
lastFetched.put(field.getName(), Instant.now());
- Object value = getTyped(m.getReturnType(), response, fqn);
+ Object value = getTyped(m.getReturnType(),
m.getGenericReturnType(), response, fqn);
setForField(field, proxy, value);
} catch (ClassCastException e) {
throw new OPMException("Unable to return response as suitable
type", e);
@@ -462,21 +468,114 @@ public class PlcEntityInterceptor {
Field field = clazz.getDeclaredField(targetTagName);
field.setAccessible(true);
try {
- field.set(o, getTyped(field.getType(), response, sourceTagName));
+ field.set(o, getTyped(field.getType(), field.getGenericType(),
response, sourceTagName));
} catch (ClassCastException e) {
throw new PlcRuntimeException(String.format("Unable to assign
return value %s to tag %s with type %s",
response.getObject(sourceTagName), targetTagName,
field.getType()), e);
}
}
+ /** The element type of a collection type, or Object when it isn't known.
*/
+ private static Class<?> elementType(Type genericType) {
+ if (genericType instanceof ParameterizedType parameterized) {
+ Type[] arguments = parameterized.getActualTypeArguments();
+ if (arguments.length == 1 && arguments[0] instanceof Class<?>
elementClass) {
+ return elementClass;
+ }
+ }
+ return Object.class;
+ }
+
+ /** Reads all values of a tag and returns them as an array of the given
component type. */
+ private static Object toArray(Class<?> componentType, PlcReadResponse
response, String sourceFieldName) {
+ List<?> values = new ArrayList<>(getAllTyped(componentType, response,
sourceFieldName));
+ Object array = Array.newInstance(componentType, values.size());
+ for (int i = 0; i < values.size(); i++) {
+ // Array.set unboxes for primitive component types.
+ Array.set(array, i, values.get(i));
+ }
+ return array;
+ }
+
+ /** Reads all values of a tag into a collection assignable to the declared
collection type. */
+ private static Object toCollection(Class<?> clazz, Class<?> elementType,
PlcReadResponse response,
+ String sourceFieldName) {
+ Collection<?> values = getAllTyped(elementType, response,
sourceFieldName);
+ if (clazz.isAssignableFrom(ArrayList.class)) {
+ return new ArrayList<>(values);
+ }
+ if (clazz.isAssignableFrom(LinkedHashSet.class)) {
+ return new LinkedHashSet<>(values);
+ }
+ throw new ClassCastException("Unable to return the values of tag " +
sourceFieldName
+ + " as an instance of " + clazz + ". Use List, Collection, Set or
an array.");
+ }
+
+ /**
+ * The typed accessor matching the element type, mirroring the scalar
conversions below.
+ * Unknown types fall back to the plain objects of the response.
+ */
+ private static Collection<?> getAllTyped(Class<?> elementType,
PlcReadResponse response, String sourceFieldName) {
+ if (elementType == boolean.class || elementType == Boolean.class) {
+ return response.getAllBooleans(sourceFieldName);
+ } else if (elementType == byte.class || elementType == Byte.class) {
+ return response.getAllBytes(sourceFieldName);
+ } else if (elementType == short.class || elementType == Short.class) {
+ return response.getAllShorts(sourceFieldName);
+ } else if (elementType == int.class || elementType == Integer.class) {
+ return response.getAllIntegers(sourceFieldName);
+ } else if (elementType == long.class || elementType == Long.class) {
+ return response.getAllLongs(sourceFieldName);
+ } else if (elementType == float.class || elementType == Float.class) {
+ return response.getAllFloats(sourceFieldName);
+ } else if (elementType == double.class || elementType == Double.class)
{
+ return response.getAllDoubles(sourceFieldName);
+ } else if (elementType == BigInteger.class) {
+ return response.getAllBigIntegers(sourceFieldName);
+ } else if (elementType == BigDecimal.class) {
+ return response.getAllBigDecimals(sourceFieldName);
+ } else if (elementType == String.class) {
+ return response.getAllStrings(sourceFieldName);
+ } else if (elementType == LocalTime.class) {
+ return response.getAllTimes(sourceFieldName);
+ } else if (elementType == LocalDate.class) {
+ return response.getAllDates(sourceFieldName);
+ } else if (elementType == LocalDateTime.class) {
+ return response.getAllDateTimes(sourceFieldName);
+ }
+ return response.getAllObjects(sourceFieldName);
+ }
+
+ static Object getTyped(Class<?> clazz, PlcReadResponse response, String
sourceFieldName) {
+ return getTyped(clazz, null, response, sourceFieldName);
+ }
+
+ /**
+ * Converts the value of one tag of a response into the type of the
annotated field.
+ *
+ * @param clazz the declared type of the field or the return type of
the getter
+ * @param genericType the generic type of the same, when known. It carries
the element type of a
+ * collection field ({@code List<Integer>}), which the
raw class does not -
+ * see GH-1947. May be null, in which case collection
elements come back as
+ * plain objects.
+ */
@SuppressWarnings({"squid:S3776", "squid:MethodCyclomaticComplexity"})
// Cognitive Complexity not too high, as highly structured
- static Object getTyped(Class<?> clazz, PlcReadResponse response, String
sourceFieldName) {
+ static Object getTyped(Class<?> clazz, Type genericType, PlcReadResponse
response, String sourceFieldName) {
LOGGER.debug("getTyped clazz: {}, response: {}, tagName: {}", clazz,
response, sourceFieldName);
if (response.getResponseCode(sourceFieldName) != PlcResponseCode.OK) {
throw new PlcRuntimeException(String.format("Unable to read
specified tag '%s', response code was '%s'",
sourceFieldName, response.getResponseCode(sourceFieldName)));
}
+ // A tag reading several values (e.g. "input-register:1:UINT[2]") has
to be assigned to an
+ // array or collection field, with its elements converted to the
field's element type -
+ // handing out the raw PlcValues makes every element access throw a
ClassCastException.
+ if (clazz.isArray()) {
+ return toArray(clazz.getComponentType(), response,
sourceFieldName);
+ }
+ if (Collection.class.isAssignableFrom(clazz)) {
+ return toCollection(clazz, elementType(genericType), response,
sourceFieldName);
+ }
if (clazz.isPrimitive()) {
if (clazz == boolean.class) {
return response.getBoolean(sourceFieldName);
diff --git
a/plc4j/tools/opm/src/test/java/org/apache/plc4x/java/opm/CollectionFieldTest.java
b/plc4j/tools/opm/src/test/java/org/apache/plc4x/java/opm/CollectionFieldTest.java
new file mode 100644
index 0000000000..de872d90ba
--- /dev/null
+++
b/plc4j/tools/opm/src/test/java/org/apache/plc4x/java/opm/CollectionFieldTest.java
@@ -0,0 +1,181 @@
+/*
+ * 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
+ *
+ * https://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.plc4x.java.api.types.PlcResponseCode;
+import org.apache.plc4x.java.spi.drivers.messages.DefaultPlcReadResponse;
+import org.apache.plc4x.java.spi.drivers.messages.items.DefaultPlcResponseItem;
+import org.apache.plc4x.java.spi.drivers.messages.items.PlcResponseItem;
+import org.apache.plc4x.java.api.value.PlcValue;
+import org.apache.plc4x.java.spi.values.PlcList;
+import org.apache.plc4x.java.spi.values.PlcUINT;
+import org.apache.plc4x.java.DefaultPlcDriverManager;
+import org.apache.plc4x.java.mock.connection.MockConnection;
+import org.apache.plc4x.java.mock.connection.MockDevice;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import java.lang.reflect.Field;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * A tag reading several values has to be converted to the element type of the
annotated field -
+ * see GH-1947. The values used to be handed out as the raw PlcValues, so
+ * {@code List<Integer> l; l.get(0)} threw a ClassCastException, and an array
field could not be
+ * assigned at all.
+ */
+class CollectionFieldTest {
+
+ /** The reporter's field: {@code @PlcTag("input-register:1:UINT[2]")
List<Integer> registers;} */
+ private List<Integer> registers;
+ private Collection<Integer> asCollection;
+ private Set<Integer> asSet;
+ private int[] asPrimitiveArray;
+ private Integer[] asBoxedArray;
+ private List<String> asStrings;
+ private List<?> withoutElementType;
+ private java.util.Deque<Integer> unsupportedCollection;
+
+ @Test
+ void listOfIntegerYieldsIntegers() throws Exception {
+ Object value = convert("registers");
+
+ assertInstanceOf(List.class, value);
+ List<?> list = (List<?>) value;
+ assertEquals(2, list.size());
+ // The point of the issue: the elements are Integers, not PlcUINTs.
+ assertEquals(Integer.valueOf(1), list.get(0));
+ assertEquals(Integer.valueOf(2), list.get(1));
+ for (Object element : list) {
+ assertInstanceOf(Integer.class, element);
+ }
+ }
+
+ @Test
+ void collectionAndSetAreAlsoSupported() throws Exception {
+ assertInstanceOf(Collection.class, convert("asCollection"));
+ assertInstanceOf(Set.class, convert("asSet"));
+ assertEquals(2, ((Collection<?>) convert("asCollection")).size());
+ }
+
+ /**
+ * The reporter's second finding: an array field could not be assigned at
all.
+ */
+ @Test
+ void primitiveArrayIsAssignable() throws Exception {
+ Object value = convert("asPrimitiveArray");
+
+ assertInstanceOf(int[].class, value);
+ assertArrayEquals(new int[]{1, 2}, (int[]) value);
+ }
+
+ @Test
+ void boxedArrayIsAssignable() throws Exception {
+ Object value = convert("asBoxedArray");
+
+ assertInstanceOf(Integer[].class, value);
+ assertArrayEquals(new Integer[]{1, 2}, (Integer[]) value);
+ }
+
+ @Test
+ void elementTypeDrivesTheConversion() throws Exception {
+ Object value = convert("asStrings");
+
+ List<?> list = (List<?>) value;
+ assertInstanceOf(String.class, list.get(0));
+ }
+
+ /**
+ * Without a resolvable element type the values come back as plain objects
rather than failing.
+ */
+ @Test
+ void unknownElementTypeFallsBackToObjects() throws Exception {
+ Object value = convert("withoutElementType");
+
+ assertEquals(2, ((List<?>) value).size());
+ }
+
+ @Test
+ void unsupportedCollectionTypeIsReported() {
+ assertThrows(ClassCastException.class, () ->
convert("unsupportedCollection"));
+ }
+
+ /**
+ * The whole way through OPM, as the reporter used it: a @PlcTag on a List
field, read through
+ * the entity manager.
+ */
+ @Test
+ void entityWithAListFieldIsPopulated() throws Exception {
+ DefaultPlcDriverManager driverManager = new DefaultPlcDriverManager();
+ MockConnection connection = (MockConnection)
driverManager.getConnection("mock:collections");
+ MockDevice mockDevice = Mockito.mock(MockDevice.class);
+ Mockito.when(mockDevice.read(Mockito.any())).thenReturn(
+ new DefaultPlcResponseItem<>(PlcResponseCode.OK,
+ new PlcList(List.of(new PlcUINT(1), new PlcUINT(2)))));
+ connection.setDevice(mockDevice);
+
+ RegisterEntity entity = new PlcEntityManager(driverManager)
+ .read(RegisterEntity.class, "mock:collections");
+
+ assertEquals(List.of(1, 2), entity.getRegisters());
+ assertArrayEquals(new int[]{1, 2}, entity.getRaw());
+ }
+
+ @PlcEntity
+ public static class RegisterEntity {
+
+ @PlcTag("input-register:1:UINT[2]")
+ private List<Integer> registers;
+
+ @PlcTag("input-register:1:UINT[2]")
+ private int[] raw;
+
+ public RegisterEntity() {
+ // For OPM
+ }
+
+ public List<Integer> getRegisters() {
+ return registers;
+ }
+
+ public int[] getRaw() {
+ return raw;
+ }
+ }
+
+ /** Converts a two-element UINT response into the declared type of the
named field. */
+ private Object convert(String fieldName) throws NoSuchFieldException {
+ Field field = CollectionFieldTest.class.getDeclaredField(fieldName);
+ return PlcEntityInterceptor.getTyped(field.getType(),
field.getGenericType(), response(), "tag");
+ }
+
+ private DefaultPlcReadResponse response() {
+ PlcValue value = new PlcList(List.of(new PlcUINT(1), new PlcUINT(2)));
+ PlcResponseItem<PlcValue> item = new
DefaultPlcResponseItem<>(PlcResponseCode.OK, value);
+ return new DefaultPlcReadResponse(null,
Collections.singletonMap("tag", item));
+ }
+}
