You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by sr...@apache.org on 2018/03/01 09:34:15 UTC

[incubator-plc4x] 02/02: further improve type safety by adding a couple more checks

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

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

commit f01125e3dcb6db298e1c83a8d0e492261667f411
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Thu Mar 1 10:34:06 2018 +0100

    further improve type safety by adding a couple more checks
---
 .../plc4x/java/api/connection/PlcReader.java       |  3 +-
 .../java/api/messages/items/ReadResponseItem.java  | 11 ++++++
 .../messages/specific/TypeSafePlcReadResponse.java | 43 ++++++++++++----------
 .../plc4x/java/api/connection/PlcReaderTest.java   |  4 +-
 .../plc4x/java/api/messages/APIMessageTests.java   |  9 ++---
 .../specific/TypeSafePlcReadResponseTest.java      | 26 ++++++++-----
 .../plc4x/java/s7/netty/Plc4XS7Protocol.java       |  5 +--
 7 files changed, 62 insertions(+), 39 deletions(-)

diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/connection/PlcReader.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/connection/PlcReader.java
index 7c76795..b8b1b8c 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/connection/PlcReader.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/connection/PlcReader.java
@@ -49,7 +49,8 @@ public interface PlcReader {
      */
     default <T> CompletableFuture<TypeSafePlcReadResponse<T>> read(TypeSafePlcReadRequest<T> readRequest) {
         Objects.requireNonNull(readRequest, "Read request must not be null");
-        return read((PlcReadRequest) readRequest).thenApply(TypeSafePlcReadResponse::of);
+        return read((PlcReadRequest) readRequest)
+            .thenApply(readResponse -> TypeSafePlcReadResponse.of(readResponse, readRequest.getDataType()));
     }
 
 }
diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadResponseItem.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadResponseItem.java
index 3fa9f0d..0116a1a 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadResponseItem.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/items/ReadResponseItem.java
@@ -20,6 +20,7 @@ package org.apache.plc4x.java.api.messages.items;
 
 import org.apache.plc4x.java.api.types.ResponseCode;
 
+import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
 
@@ -30,9 +31,19 @@ public class ReadResponseItem<T> extends ResponseItem<ReadRequestItem<T>> {
     public ReadResponseItem(ReadRequestItem<T> requestItem, ResponseCode responseCode, List<T> values) {
         super(requestItem, responseCode);
         Objects.requireNonNull(values, "Values must not be null");
+        for (T value : values) {
+            if (!requestItem.getDatatype().isAssignableFrom(value.getClass())) {
+                throw new IllegalArgumentException("Datatype of " + value + " doesn't macht required datatype of " + requestItem.getDatatype());
+            }
+        }
         this.values = values;
     }
 
+    @SafeVarargs
+    public ReadResponseItem(ReadRequestItem<T> requestItem, ResponseCode responseCode, T... values) {
+        this(requestItem, responseCode, Arrays.asList(values));
+    }
+
     public List<T> getValues() {
         return values;
     }
diff --git a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadResponse.java b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadResponse.java
index f32e527..d6ed804 100644
--- a/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadResponse.java
+++ b/plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadResponse.java
@@ -59,32 +59,35 @@ public class TypeSafePlcReadResponse<T> extends PlcReadResponse {
         return (Optional<ReadResponseItem<T>>) super.getResponseItem();
     }
 
-    @SuppressWarnings("unchecked")
-    public static <T> TypeSafePlcReadResponse<T> of(PlcReadResponse plcReadResponse) {
+    public static <T> TypeSafePlcReadResponse<T> of(PlcReadResponse plcReadResponse, Class<T> clazz) {
+        Objects.requireNonNull(plcReadResponse, "PlcReadResponse must not be null");
+        Objects.requireNonNull(clazz, "Class must not be null");
         if (plcReadResponse instanceof TypeSafePlcReadResponse) {
-            return (TypeSafePlcReadResponse) plcReadResponse;
-        }
-        if (plcReadResponse.getRequest() instanceof TypeSafePlcReadRequest) {
-            return new TypeSafePlcReadResponse((TypeSafePlcReadRequest) plcReadResponse.getRequest(), plcReadResponse.getResponseItems());
-        }
-        List<? extends ReadResponseItem<?>> responseItems = plcReadResponse.getResponseItems();
-        Objects.requireNonNull(responseItems, "Response items on " + plcReadResponse + " must not be null");
-        Class type = null;
-        for (ReadResponseItem<?> responseItem : responseItems) {
-            if (!responseItem.getValues().isEmpty()) {
-                type = responseItem.getValues().get(0).getClass();
-                break;
+            @SuppressWarnings("unchecked")
+            TypeSafePlcReadResponse<T> typeSafePlcReadResponse = (TypeSafePlcReadResponse<T>) plcReadResponse;
+            Class type = typeSafePlcReadResponse.getRequest().getDataType();
+            if (type != clazz) {
+                throw new IllegalArgumentException("Expected type " + clazz + " doesn't match found type " + type);
             }
+            return typeSafePlcReadResponse;
         }
-        if (type != null) {
-            for (ReadResponseItem<?> responseItem : responseItems) {
-                checkList(responseItem.getValues(), type);
+        @SuppressWarnings("unchecked")
+        List<ReadResponseItem<T>> responseItems = (List<ReadResponseItem<T>>) plcReadResponse.getResponseItems();
+        Objects.requireNonNull(responseItems, "Response items on " + plcReadResponse + " must not be null");
+        if (plcReadResponse.getRequest() instanceof TypeSafePlcReadRequest) {
+            @SuppressWarnings("unchecked")
+            TypeSafePlcReadRequest<T> typeSafePlcReadRequest = (TypeSafePlcReadRequest<T>) plcReadResponse.getRequest();
+            Class type = typeSafePlcReadRequest.getDataType();
+            if (type != clazz) {
+                throw new IllegalArgumentException("Expected type " + clazz + " doesn't match found type " + type);
             }
+            return new TypeSafePlcReadResponse<>(typeSafePlcReadRequest, responseItems);
         }
-        if (type == null) {
-            type = Object.class;
+        for (ReadResponseItem<?> responseItem : responseItems) {
+            checkList(responseItem.getValues(), clazz);
         }
-        return new TypeSafePlcReadResponse(new TypeSafePlcReadRequest(type, plcReadResponse.getRequest()), responseItems);
+        TypeSafePlcReadRequest<T> request = new TypeSafePlcReadRequest<>(clazz, plcReadResponse.getRequest());
+        return new TypeSafePlcReadResponse<>(request, responseItems);
     }
 
     private static void checkList(List<?> list, Class<?> type) {
diff --git a/plc4j/api/src/test/java/org/apache/plc4x/java/api/connection/PlcReaderTest.java b/plc4j/api/src/test/java/org/apache/plc4x/java/api/connection/PlcReaderTest.java
index 12abf77..e97c288 100644
--- a/plc4j/api/src/test/java/org/apache/plc4x/java/api/connection/PlcReaderTest.java
+++ b/plc4j/api/src/test/java/org/apache/plc4x/java/api/connection/PlcReaderTest.java
@@ -47,9 +47,11 @@ public class PlcReaderTest {
     @Test
     public void readWrongType() throws Exception {
         try {
-            ((PlcReader) readRequest -> CompletableFuture.completedFuture(new PlcReadResponse(readRequest, (List) Collections.singletonList(new ReadResponseItem(readRequest.getRequestItem().get(), ResponseCode.OK, Collections.singletonList(1))))))
+            ((PlcReader) readRequest -> CompletableFuture.completedFuture(new PlcReadResponse(readRequest, (List) Collections.singletonList(new ReadResponseItem(readRequest.getRequestItem().get(), ResponseCode.OK, 1)))))
                 .read(new TypeSafePlcReadRequest<>(String.class, mock(Address.class))).get();
             fail("Should throw an exception");
+        } catch (IllegalArgumentException e) {
+            assertThat(e.getMessage(), equalTo("Datatype of 1 doesn't macht required datatype of class java.lang.String"));
         } catch (ExecutionException e) {
             assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
             assertThat(e.getCause().getMessage(), equalTo("Unexpected data type class java.lang.Integer on readRequestItem. Expected class java.lang.String"));
diff --git a/plc4j/api/src/test/java/org/apache/plc4x/java/api/messages/APIMessageTests.java b/plc4j/api/src/test/java/org/apache/plc4x/java/api/messages/APIMessageTests.java
index 2517409..6c44f65 100644
--- a/plc4j/api/src/test/java/org/apache/plc4x/java/api/messages/APIMessageTests.java
+++ b/plc4j/api/src/test/java/org/apache/plc4x/java/api/messages/APIMessageTests.java
@@ -32,7 +32,6 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
@@ -70,7 +69,7 @@ public class APIMessageTests {
     public void readResponseItem() {
         MockAddress address = new MockAddress("mock:/DATA");
         ReadRequestItem<Byte> readRequestItem = new ReadRequestItem<>(Byte.class, address, 1);
-        ReadResponseItem<Byte> readResponseItem = new ReadResponseItem<>(readRequestItem, ResponseCode.OK, Collections.emptyList());
+        ReadResponseItem<Byte> readResponseItem = new ReadResponseItem<>(readRequestItem, ResponseCode.OK);
         assertThat("Unexpected response code", readResponseItem.getResponseCode(), equalTo(ResponseCode.OK));
         assertThat(readResponseItem.getValues(), empty());
         assertThat("Unexpected read request item", readResponseItem.getRequestItem(), equalTo(readRequestItem));
@@ -155,7 +154,7 @@ public class APIMessageTests {
         List<ReadResponseItem<?>> responseItems = new ArrayList<>();
         MockAddress address = new MockAddress("mock:/DATA");
         ReadRequestItem<Byte> readRequestItem = new ReadRequestItem<>(Byte.class, address, 1);
-        ReadResponseItem<Byte> readResponseItem = new ReadResponseItem<>(readRequestItem, ResponseCode.OK, Collections.emptyList());
+        ReadResponseItem<Byte> readResponseItem = new ReadResponseItem<>(readRequestItem, ResponseCode.OK);
         responseItems.add(readResponseItem);
         PlcReadResponse plcReadResponse = new PlcReadResponse(plcReadRequest, responseItems);
         assertThat("Unexpected number of response items", plcReadResponse.getRequest().getNumberOfItems(), equalTo(0));
@@ -251,8 +250,8 @@ public class APIMessageTests {
         MockAddress address = new MockAddress("mock:/DATA");
         ReadRequestItem<Byte> readRequestItem1 = new ReadRequestItem<>(Byte.class, address, 1);
         ReadRequestItem<Byte> readRequestItem2 = new ReadRequestItem<>(Byte.class, address, 1);
-        ReadResponseItem<Byte> readResponseItem1 = new ReadResponseItem<>(readRequestItem1, ResponseCode.OK, Collections.emptyList());
-        ReadResponseItem<Byte> readResponseItem2 = new ReadResponseItem<>(readRequestItem2, ResponseCode.OK, Collections.emptyList());
+        ReadResponseItem<Byte> readResponseItem1 = new ReadResponseItem<>(readRequestItem1, ResponseCode.OK);
+        ReadResponseItem<Byte> readResponseItem2 = new ReadResponseItem<>(readRequestItem2, ResponseCode.OK);
         responseItems.add(readResponseItem1);
         responseItems.add(readResponseItem2);
         PlcReadResponse plcReadResponse = new PlcReadResponse(plcReadRequest, responseItems);
diff --git a/plc4j/api/src/test/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadResponseTest.java b/plc4j/api/src/test/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadResponseTest.java
index ce43b9b..e437307 100644
--- a/plc4j/api/src/test/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadResponseTest.java
+++ b/plc4j/api/src/test/java/org/apache/plc4x/java/api/messages/specific/TypeSafePlcReadResponseTest.java
@@ -25,7 +25,6 @@ import org.apache.plc4x.java.api.types.ResponseCode;
 import org.junit.Before;
 import org.junit.Test;
 
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -37,35 +36,44 @@ public class TypeSafePlcReadResponseTest {
 
     @Before
     public void setUp() {
-        readResponseItemString = new ReadResponseItem<>(mock(ReadRequestItem.class), ResponseCode.OK, Arrays.asList("", ""));
+        ReadRequestItem<String> mock = mock(ReadRequestItem.class);
+        when(mock.getDatatype()).thenReturn(String.class);
+        readResponseItemString = new ReadResponseItem<>(mock, ResponseCode.OK, "", "");
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void constuctor() {
-        TypeSafePlcReadRequest mock = mock(TypeSafePlcReadRequest.class);
+        TypeSafePlcReadRequest<String> mock = mock(TypeSafePlcReadRequest.class);
         when(mock.getDataType()).thenReturn(String.class);
         new TypeSafePlcReadResponse<>(mock, readResponseItemString);
         new TypeSafePlcReadResponse<>(mock, Collections.singletonList(readResponseItemString));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void constuctorWrong() {
+        TypeSafePlcReadRequest<Byte> mock = mock(TypeSafePlcReadRequest.class);
         when(mock.getDataType()).thenReturn(Byte.class);
 
         // expects an exception
-        new TypeSafePlcReadResponse<>(mock, readResponseItemString);
+        new TypeSafePlcReadResponse(mock, readResponseItemString);
     }
 
     @Test
     public void of() {
         {
-            TypeSafePlcReadResponse.of(mock(PlcReadResponse.class, RETURNS_DEEP_STUBS));
+            TypeSafePlcReadResponse.of(mock(PlcReadResponse.class, RETURNS_DEEP_STUBS), String.class);
         }
         {
             PlcReadResponse response = mock(PlcReadResponse.class, RETURNS_DEEP_STUBS);
-            when(response.getRequest()).thenReturn(mock(TypeSafePlcReadRequest.class, RETURNS_DEEP_STUBS));
-            TypeSafePlcReadResponse.of(response);
+            TypeSafePlcReadRequest typeSafePlcReadRequest = mock(TypeSafePlcReadRequest.class, RETURNS_DEEP_STUBS);
+            when(typeSafePlcReadRequest.getDataType()).thenReturn(String.class);
+            when(response.getRequest()).thenReturn(typeSafePlcReadRequest);
+            TypeSafePlcReadResponse.of(response, String.class);
         }
         {
             PlcReadResponse response = mock(PlcReadResponse.class, RETURNS_DEEP_STUBS);
             when(response.getResponseItems()).thenReturn((List) Collections.singletonList(readResponseItemString));
-            TypeSafePlcReadResponse.of(response);
+            TypeSafePlcReadResponse.of(response, String.class);
         }
     }
 
diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java
index 2acc90a..9fcec13 100644
--- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java
+++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/Plc4XS7Protocol.java
@@ -161,8 +161,7 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
             // Handle the response to a read request.
             if (request instanceof PlcReadRequest) {
                 response = decodeReadRequest(responseMessage, requestContainer);
-            }
-            else if (request instanceof PlcWriteRequest) {
+            } else if (request instanceof PlcWriteRequest) {
                 response = decodeWriteRequest(responseMessage, requestContainer);
             }
 
@@ -238,7 +237,7 @@ public class Plc4XS7Protocol extends MessageToMessageCodec<S7Message, PlcRequest
             ReadResponseItem responseItem;
             // Something went wrong.
             if (responseCode != ResponseCode.OK) {
-                responseItem = new ReadResponseItem<>(requestItem, responseCode, null);
+                responseItem = new ReadResponseItem<>(requestItem, responseCode);
             }
             // All Ok.
             else {

-- 
To stop receiving notification emails like this one, please contact
sruehl@apache.org.