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 <sru...@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
sru...@apache.org.

Reply via email to