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.