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 537d756954 refactor(plc4j/spi): Updated the connection base to not
blidnly report supporting all 4 types of connection-operations.
537d756954 is described below
commit 537d756954df4f08306373eb8a5cf0981d402e5d
Author: Christofer Dutz <[email protected]>
AuthorDate: Sun Aug 16 15:58:07 2026 +0200
refactor(plc4j/spi): Updated the connection base to not blidnly report
supporting all 4 types of connection-operations.
---
.../plc4x/java/spi/drivers/ConnectionBase.java | 56 +++++-
.../spi/drivers/ConnectionBaseMetadataTest.java | 190 +++++++++++++++++++++
2 files changed, 242 insertions(+), 4 deletions(-)
diff --git
a/plc4j/spi/drivers/src/main/java/org/apache/plc4x/java/spi/drivers/ConnectionBase.java
b/plc4j/spi/drivers/src/main/java/org/apache/plc4x/java/spi/drivers/ConnectionBase.java
index 1d958f9e9a..5382941202 100644
---
a/plc4j/spi/drivers/src/main/java/org/apache/plc4x/java/spi/drivers/ConnectionBase.java
+++
b/plc4j/spi/drivers/src/main/java/org/apache/plc4x/java/spi/drivers/ConnectionBase.java
@@ -239,31 +239,79 @@ public abstract class ConnectionBase<C extends
Configuration> implements PlcConn
return authentication;
}
+ /**
+ * Derived from the operations the driver actually implements.
+ * <p>
+ * Every {@code on*} hook below has a default that fails the request with
"... not supported",
+ * so a driver only supports an operation if it overrides the
corresponding hook. Reporting
+ * a fixed {@code true} here - as this class used to - told every caller
that every driver
+ * supports everything, which is wrong for most of them: a driver that
connects but implements
+ * nothing claimed full support, and callers that check the metadata
before deciding what to do
+ * were sent into requests that could only fail.
+ * <p>
+ * The lookup walks the class hierarchy up to (but excluding) {@link
ConnectionBase}, so a hook
+ * inherited from an intermediate base counts as implemented - {@code
PollingSubscriptionConnectionBase}
+ * for instance provides {@code onSubscribe} for drivers whose protocol
has no native subscriptions.
+ * <p>
+ * What this can tell is what the <em>driver</em> implements, which is a
property of the class.
+ * Where the answer depends on the device at the other end - a controller
that does not offer an
+ * operation the driver otherwise speaks - only the driver can know, so it
should override this
+ * method and answer from what it learned during connect. This derivation
is the default for
+ * drivers that have no such distinction to make, and the result is
deliberately computed per
+ * call so an override is free to vary per connection.
+ */
@Override
public PlcConnectionMetadata getMetadata() {
+ return deriveMetadata(getClass());
+ }
+
+ private static PlcConnectionMetadata deriveMetadata(Class<?>
connectionClass) {
+ boolean read = overridesHook(connectionClass, "onRead",
PlcReadRequest.class);
+ boolean write = overridesHook(connectionClass, "onWrite",
PlcWriteRequest.class);
+ boolean subscribe = overridesHook(connectionClass, "onSubscribe",
PlcSubscriptionRequest.class);
+ boolean browse = overridesHook(connectionClass, "onBrowse",
PlcBrowseRequest.class)
+ || overridesHook(connectionClass, "onBrowseWithInterceptor",
PlcBrowseRequest.class, PlcBrowseRequestInterceptor.class);
+ LOGGER.debug("Derived metadata for {}: read={}, write={},
subscribe={}, browse={}",
+ connectionClass.getSimpleName(), read, write, subscribe, browse);
return new PlcConnectionMetadata() {
@Override
public boolean isReadSupported() {
- return true;
+ return read;
}
@Override
public boolean isWriteSupported() {
- return true;
+ return write;
}
@Override
public boolean isSubscribeSupported() {
- return true;
+ return subscribe;
}
@Override
public boolean isBrowseSupported() {
- return true;
+ return browse;
}
};
}
+ /**
+ * Whether {@code connectionClass} or any class between it and {@link
ConnectionBase} declares
+ * the given hook - i.e. whether the default "not supported"
implementation has been replaced.
+ */
+ private static boolean overridesHook(Class<?> connectionClass, String
name, Class<?>... parameterTypes) {
+ for (Class<?> current = connectionClass; (current != null) && (current
!= ConnectionBase.class); current = current.getSuperclass()) {
+ try {
+ current.getDeclaredMethod(name, parameterTypes);
+ return true;
+ } catch (NoSuchMethodException e) {
+ // Not declared here - keep walking up towards ConnectionBase.
+ }
+ }
+ return false;
+ }
+
@Override
public String getProtocolCode() {
return protocolCode;
diff --git
a/plc4j/spi/drivers/src/test/java/org/apache/plc4x/java/spi/drivers/ConnectionBaseMetadataTest.java
b/plc4j/spi/drivers/src/test/java/org/apache/plc4x/java/spi/drivers/ConnectionBaseMetadataTest.java
new file mode 100644
index 0000000000..5c58967fbb
--- /dev/null
+++
b/plc4j/spi/drivers/src/test/java/org/apache/plc4x/java/spi/drivers/ConnectionBaseMetadataTest.java
@@ -0,0 +1,190 @@
+/*
+ * 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.spi.drivers;
+
+import org.apache.plc4x.java.api.messages.PlcBrowseRequest;
+import org.apache.plc4x.java.api.messages.PlcBrowseResponse;
+import org.apache.plc4x.java.api.messages.PlcReadRequest;
+import org.apache.plc4x.java.api.messages.PlcReadResponse;
+import org.apache.plc4x.java.api.messages.PlcSubscriptionRequest;
+import org.apache.plc4x.java.api.messages.PlcSubscriptionResponse;
+import org.apache.plc4x.java.api.messages.PlcWriteRequest;
+import org.apache.plc4x.java.api.messages.PlcWriteResponse;
+import org.apache.plc4x.java.api.metadata.PlcConnectionMetadata;
+import org.apache.plc4x.java.spi.config.Configuration;
+import org.apache.plc4x.java.spi.drivers.tags.PlcTagHandler;
+import org.apache.plc4x.java.spi.values.PlcValueHandler;
+import org.junit.jupiter.api.Test;
+
+import java.util.concurrent.CompletableFuture;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * The connection metadata is derived from the operations a driver implements.
It used to be a fixed
+ * "everything is supported", which meant a driver that implements nothing
still claimed to support
+ * reading, writing, subscribing and browsing.
+ */
+class ConnectionBaseMetadataTest {
+
+ /**
+ * A driver that implements none of the operations - it can connect and
nothing else.
+ */
+ @Test
+ void reportsNothingForAConnectionThatImplementsNoOperation() {
+ PlcConnectionMetadata metadata = new EmptyConnection().getMetadata();
+
+ assertFalse(metadata.isReadSupported());
+ assertFalse(metadata.isWriteSupported());
+ assertFalse(metadata.isSubscribeSupported());
+ assertFalse(metadata.isBrowseSupported());
+ }
+
+ @Test
+ void reportsExactlyTheOperationsTheDriverImplements() {
+ PlcConnectionMetadata metadata = new
ReadWriteConnection().getMetadata();
+
+ assertTrue(metadata.isReadSupported());
+ assertTrue(metadata.isWriteSupported());
+ assertFalse(metadata.isSubscribeSupported(), "onSubscribe is not
overridden");
+ assertFalse(metadata.isBrowseSupported(), "onBrowse is not
overridden");
+ }
+
+ /**
+ * An operation provided by an intermediate base class counts as
implemented. This is how
+ * PollingSubscriptionConnectionBase gives protocols without native
subscriptions (Modbus and
+ * friends) a working subscribe.
+ */
+ @Test
+ void countsAnOperationInheritedFromAnIntermediateBase() {
+ PlcConnectionMetadata metadata = new
InheritsSubscribeConnection().getMetadata();
+
+ assertTrue(metadata.isSubscribeSupported(), "onSubscribe comes from
the intermediate base");
+ assertFalse(metadata.isReadSupported());
+ }
+
+ /**
+ * Browsing counts when either browse hook is implemented.
+ */
+ @Test
+ void countsBrowseFromTheInterceptorHookAsWell() {
+ assertTrue(new BrowseConnection().getMetadata().isBrowseSupported());
+ }
+
+ /**
+ * A driver whose capabilities depend on the device it connected to
overrides this and answers
+ * from what it learned during connect. Nothing in the base class may
cache across connections
+ * and defeat that.
+ */
+ @Test
+ void letsADriverAnswerPerConnection() {
+ DeviceDependentConnection limited = new
DeviceDependentConnection(false);
+ DeviceDependentConnection capable = new
DeviceDependentConnection(true);
+
+ assertFalse(limited.getMetadata().isWriteSupported());
+ assertTrue(capable.getMetadata().isWriteSupported(),
+ "two connections of the same class must be able to answer
differently");
+ }
+
+ private abstract static class TestConnection extends
ConnectionBase<Configuration> {
+ TestConnection() {
+ super(null, null, null);
+ }
+
+ @Override
+ public PlcTagHandler getTagHandler() {
+ return null;
+ }
+
+ @Override
+ public PlcValueHandler getValueHandler() {
+ return null;
+ }
+ }
+
+ private static class EmptyConnection extends TestConnection {
+ }
+
+ private static class ReadWriteConnection extends TestConnection {
+ @Override
+ protected CompletableFuture<PlcReadResponse> onRead(PlcReadRequest
readRequest) {
+ return CompletableFuture.completedFuture(null);
+ }
+
+ @Override
+ protected CompletableFuture<PlcWriteResponse> onWrite(PlcWriteRequest
writeRequest) {
+ return CompletableFuture.completedFuture(null);
+ }
+ }
+
+ private abstract static class SubscribingBase extends TestConnection {
+ @Override
+ protected CompletableFuture<PlcSubscriptionResponse>
onSubscribe(PlcSubscriptionRequest subscriptionRequest) {
+ return CompletableFuture.completedFuture(null);
+ }
+ }
+
+ private static class InheritsSubscribeConnection extends SubscribingBase {
+ }
+
+ private static class BrowseConnection extends TestConnection {
+ @Override
+ protected CompletableFuture<PlcBrowseResponse>
onBrowse(PlcBrowseRequest browseRequest) {
+ return CompletableFuture.completedFuture(null);
+ }
+ }
+
+ /**
+ * Speaks writing, but whether the device accepts it is only known once
connected.
+ */
+ private static class DeviceDependentConnection extends ReadWriteConnection
{
+ private final boolean deviceAllowsWriting;
+
+ DeviceDependentConnection(boolean deviceAllowsWriting) {
+ this.deviceAllowsWriting = deviceAllowsWriting;
+ }
+
+ @Override
+ public PlcConnectionMetadata getMetadata() {
+ PlcConnectionMetadata derived = super.getMetadata();
+ return new PlcConnectionMetadata() {
+ @Override
+ public boolean isReadSupported() {
+ return derived.isReadSupported();
+ }
+
+ @Override
+ public boolean isWriteSupported() {
+ return derived.isWriteSupported() && deviceAllowsWriting;
+ }
+
+ @Override
+ public boolean isSubscribeSupported() {
+ return derived.isSubscribeSupported();
+ }
+
+ @Override
+ public boolean isBrowseSupported() {
+ return derived.isBrowseSupported();
+ }
+ };
+ }
+ }
+}
