From 8eea8bc7817af7f0b0b211ae8ed9d38d45de229e Mon Sep 17 00:00:00 2001 From: zart colwing Date: Sun, 20 Mar 2022 19:18:16 +0100 Subject: [PATCH 1/4] add a unit-test to make sure an hypothesis is true - add protobuf-java-util dependency to the pom.xml, required by the new unit-test add a unit-test to make sure an hypothesis is true --- support-lite/pom.xml | 24 ++++++++++++------- .../ProtobufStandardMappingsTest.java | 19 ++++++++++++--- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/support-lite/pom.xml b/support-lite/pom.xml index 029eef48..409d739a 100755 --- a/support-lite/pom.xml +++ b/support-lite/pom.xml @@ -1,13 +1,13 @@ - 4.0.0 @@ -33,6 +33,12 @@ 2.7.4 provided + + com.google.protobuf + protobuf-java-util + ${protobuf.java.version} + test + org.junit.jupiter junit-jupiter-api diff --git a/support-lite/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java b/support-lite/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java index 99f6e16a..9372e261 100644 --- a/support-lite/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java +++ b/support-lite/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java @@ -9,12 +9,12 @@ * Licensed under the EUPL, Version 1.1 or – as soon they will be * approved by the European Commission - subsequent versions of the * EUPL (the "Licence"); - * + * * You may not use this work except in compliance with the Licence. * You may obtain a copy of the Licence at: - * + * * http://ec.europa.eu/idabc/eupl5 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the Licence is distributed on an "AS IS" basis, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -36,6 +36,7 @@ import org.junit.jupiter.api.Test; import com.google.protobuf.Timestamp; +import com.google.protobuf.util.Timestamps; public class ProtobufStandardMappingsTest { @@ -112,4 +113,16 @@ public void mapNegativeDuration_fromProto() { assertEquals(pbDuration, MAPPER.mapDuration(duration)); } + @Test + void timestamp_toEpochMilli_equals_Timestamps_toMillis() { + // GIVEN + Timestamp nowTimestamp = MAPPER.mapToTimestamp(Instant.now()); + + // WHEN + long millis1 = MAPPER.toEpochMilliseconds(nowTimestamp); + long millis2 = Timestamps.toMillis(nowTimestamp); + + // THEN + assertEquals(millis1, millis2); + } } From 6faeaeb35aa92dcc99874242b26e24172ef4e189 Mon Sep 17 00:00:00 2001 From: zart colwing Date: Sun, 20 Mar 2022 19:25:38 +0100 Subject: [PATCH 2/4] update the implementation to ban null return value The mapper are not allowed to return null anymore. It is the responsibility of the application to perform domain value check and defaultToNull post processing when appropriate. --- .../common/ProtobufStandardMappings.java | 42 ++++++++-------- .../mapstruct/ProtobufStandardMappings.java | 39 +++++++-------- .../mapstruct/ProtobufStandardMappings.java | 49 +++++++------------ 3 files changed, 59 insertions(+), 71 deletions(-) diff --git a/support-core/src/main/java/no/entur/abt/mapstruct/common/ProtobufStandardMappings.java b/support-core/src/main/java/no/entur/abt/mapstruct/common/ProtobufStandardMappings.java index bc5bdb96..0f17f48c 100644 --- a/support-core/src/main/java/no/entur/abt/mapstruct/common/ProtobufStandardMappings.java +++ b/support-core/src/main/java/no/entur/abt/mapstruct/common/ProtobufStandardMappings.java @@ -9,12 +9,12 @@ * Licensed under the EUPL, Version 1.1 or – as soon they will be * approved by the European Commission - subsequent versions of the * EUPL (the "Licence"); - * + * * You may not use this work except in compliance with the Licence. * You may obtain a copy of the Licence at: - * + * * http://ec.europa.eu/idabc/eupl5 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the Licence is distributed on an "AS IS" basis, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -50,44 +50,44 @@ default ByteString mapByteString(byte[] array) { } default byte[] mapByteString(ByteString in) { - if (in != null && !in.isEmpty()) { - return in.toByteArray(); - } - - return null; + return in != null ? in.toByteArray() : new byte[0]; } default ByteString mapByteStringToString(String string) { - return ByteString.copyFromUtf8(string); + return ByteString.copyFromUtf8(string != null ? string : ""); } default String mapStringToByteString(ByteString in) { - if (in != null && !in.isEmpty()) { - return in.toStringUtf8(); - } - - return null; + return in != null ? in.toStringUtf8() : ""; } default com.google.type.Date mapLocalDate(LocalDate t) { - return com.google.type.Date.newBuilder().setYear(t.getYear()).setMonth(t.getMonthValue()).setDay(t.getDayOfMonth()).build(); + return t != null + ? com.google.type.Date.newBuilder().setYear(t.getYear()).setMonth(t.getMonthValue()).setDay(t.getDayOfMonth()).build() + : com.google.type.Date.getDefaultInstance(); } default LocalDate mapDate(com.google.type.Date t) { - return LocalDate.of(t.getYear(), t.getMonth(), t.getDay()); + return t != null + ? LocalDate.of(t.getYear(), t.getMonth(), t.getDay()) + : LocalDate.of(1970, 1, 1); } default com.google.type.TimeOfDay mapLocalTime(LocalTime t) { - return com.google.type.TimeOfDay.newBuilder().setHours(t.getHour()).setMinutes(t.getMinute()).setSeconds(t.getSecond()).setNanos(t.getNano()).build(); + return t != null + ? com.google.type.TimeOfDay.newBuilder().setHours(t.getHour()).setMinutes(t.getMinute()).setSeconds(t.getSecond()).setNanos(t.getNano()).build() + : com.google.type.TimeOfDay.getDefaultInstance(); } default LocalTime mapTimeOfDay(com.google.type.TimeOfDay t) { - return LocalTime.of(t.getHours(), t.getMinutes(), t.getSeconds(), t.getNanos()); + return t != null + ? LocalTime.of(t.getHours(), t.getMinutes(), t.getSeconds(), t.getNanos()) + : LocalTime.MIDNIGHT; } default Timestamp map(LocalDateTime i) { if (i == null) { - return null; + return Timestamp.getDefaultInstance(); } TimeZone systemDefault = TimeZone.getDefault(); @@ -99,7 +99,9 @@ default Timestamp map(LocalDateTime i) { } default Timestamp map(OffsetDateTime in) { - return Timestamp.newBuilder().setSeconds(in.toEpochSecond()).setNanos(0).build(); + return in != null + ? Timestamp.newBuilder().setSeconds(in.toEpochSecond()).setNanos(0).build() + : Timestamp.getDefaultInstance(); } default float map(FloatValue f) { diff --git a/support-lite/src/main/java/no/entur/abt/mapstruct/ProtobufStandardMappings.java b/support-lite/src/main/java/no/entur/abt/mapstruct/ProtobufStandardMappings.java index 81a94a8d..958e1361 100644 --- a/support-lite/src/main/java/no/entur/abt/mapstruct/ProtobufStandardMappings.java +++ b/support-lite/src/main/java/no/entur/abt/mapstruct/ProtobufStandardMappings.java @@ -9,12 +9,12 @@ * Licensed under the EUPL, Version 1.1 or – as soon they will be * approved by the European Commission - subsequent versions of the * EUPL (the "Licence"); - * + * * You may not use this work except in compliance with the Licence. * You may obtain a copy of the Licence at: - * + * * http://ec.europa.eu/idabc/eupl5 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the Licence is distributed on an "AS IS" basis, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -44,37 +44,36 @@ public interface ProtobufStandardMappings extends no.entur.abt.mapstruct.common. ProtobufStandardMappings INSTANCE = Mappers.getMapper(ProtobufStandardMappings.class); default Instant mapToInstant(Timestamp t) { - if (t == null || (t.getSeconds() == 0 && t.getNanos() == 0)) { - return null; - } - return Instant.ofEpochSecond(t.getSeconds(), t.getNanos()); + return t != null + ? Instant.ofEpochSecond(t.getSeconds(), t.getNanos()) + : Instant.EPOCH; } default Long toEpochMilliseconds(Timestamp instance) { - Instant instant = mapToInstant(instance); - return instant == null ? null : instant.toEpochMilli(); + return mapToInstant(instance).toEpochMilli(); } default Timestamp mapToTimestamp(Instant i) { - if (i == null || i.getEpochSecond() == 0) { - return null; - } - return Timestamp.newBuilder().setSeconds(i.getEpochSecond()).setNanos(i.getNano()).build(); + return i != null + ? Timestamp.newBuilder().setSeconds(i.getEpochSecond()).setNanos(i.getNano()).build() + : Timestamp.getDefaultInstance(); } - default Timestamp fromEpochMilliseconds(Long instance) { - if (instance == null) { - return null; - } - Instant instant = Instant.ofEpochMilli(instance); - return mapToTimestamp(instant); + default Timestamp fromEpochMilliseconds(Long millis) { + return mapToTimestamp(Instant.ofEpochMilli(millis != null ? millis : 0L)); } default Duration mapDuration(com.google.protobuf.Duration t) { - return Duration.ofSeconds(t.getSeconds(), t.getNanos()); + return t != null + ? Duration.ofSeconds(t.getSeconds(), t.getNanos()) + : Duration.ZERO; } default com.google.protobuf.Duration mapDuration(Duration t) { + if (t == null) { + return com.google.protobuf.Duration.getDefaultInstance(); + } + long seconds = t.getSeconds(); int nanos = t.getNano(); diff --git a/support-standard/src/main/java/no/entur/abt/mapstruct/ProtobufStandardMappings.java b/support-standard/src/main/java/no/entur/abt/mapstruct/ProtobufStandardMappings.java index 3dc6d6a9..ea83daa3 100644 --- a/support-standard/src/main/java/no/entur/abt/mapstruct/ProtobufStandardMappings.java +++ b/support-standard/src/main/java/no/entur/abt/mapstruct/ProtobufStandardMappings.java @@ -9,12 +9,12 @@ * Licensed under the EUPL, Version 1.1 or – as soon they will be * approved by the European Commission - subsequent versions of the * EUPL (the "Licence"); - * + * * You may not use this work except in compliance with the Licence. * You may obtain a copy of the Licence at: - * + * * http://ec.europa.eu/idabc/eupl5 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the Licence is distributed on an "AS IS" basis, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -31,7 +31,6 @@ import com.google.protobuf.Timestamp; import com.google.protobuf.util.Durations; -import com.google.protobuf.util.Timestamps; /*** * @@ -45,47 +44,35 @@ public interface ProtobufStandardMappings extends no.entur.abt.mapstruct.common. ProtobufStandardMappings INSTANCE = Mappers.getMapper(ProtobufStandardMappings.class); default Instant mapToInstant(Timestamp t) { - if (Timestamps.isValid(t) && (t.getSeconds() > 0 || t.getNanos() > 0)) { - return Instant.ofEpochSecond(t.getSeconds(), t.getNanos()); - } else { - return null; - } + return t != null + ? Instant.ofEpochSecond(t.getSeconds(), t.getNanos()) + : Instant.EPOCH; } default Long toEpochMilliseconds(Timestamp instance) { - if (instance != null) { - return Timestamps.toMillis(instance); - } else { - return null; - } + return mapToInstant(instance).toEpochMilli(); } default Timestamp mapToTimestamp(Instant i) { - if (i == null || i.getEpochSecond() == 0) { - return null; - } else { - return Timestamp.newBuilder().setSeconds(i.getEpochSecond()).setNanos(i.getNano()).build(); - } + return i != null + ? Timestamp.newBuilder().setSeconds(i.getEpochSecond()).setNanos(i.getNano()).build() + : Timestamp.getDefaultInstance(); } - default Timestamp fromEpochMilliseconds(long millis) { - return Timestamps.fromMillis(millis); + default Timestamp fromEpochMilliseconds(Long millis) { + return mapToTimestamp(Instant.ofEpochMilli(millis != null ? millis : 0L)); } default Duration mapDuration(com.google.protobuf.Duration t) { - if (t != null) { - return Duration.ofSeconds(t.getSeconds(), t.getNanos()); - } else { - return null; - } + return t != null + ? Duration.ofSeconds(t.getSeconds(), t.getNanos()) + : Duration.ZERO; } default com.google.protobuf.Duration mapDuration(Duration t) { - if (t != null) { - return Durations.fromNanos(t.toNanos()); - } else { - return null; - } + return t != null + ? Durations.fromNanos(t.toNanos()) + : com.google.protobuf.Duration.getDefaultInstance(); } } From 753bf6e187bdd5cdb6543a14e3cefeebbb2c5297 Mon Sep 17 00:00:00 2001 From: zart colwing Date: Sun, 20 Mar 2022 19:27:14 +0100 Subject: [PATCH 3/4] replace one unit-test to conform with the new implementation that ban null replace one unit-test to conform with the new implementation that ban null --- .../abt/mapstruct/ProtobufStandardMappingsTest.java | 7 +++++++ .../abt/mapstruct/ProtobufStandardMappingsTest.java | 13 ++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/support-lite/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java b/support-lite/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java index 9372e261..d1f74b42 100644 --- a/support-lite/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java +++ b/support-lite/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java @@ -66,11 +66,18 @@ public void testMapLocalDateToTimestampWintertime() { assertEquals(l, back); } + // No longer true with the new implementation that ban null. @Test + @org.junit.jupiter.api.Disabled public void mapToInstant_whenSecondsAndNanosIs0_thenMapToNull() { assertNull(MAPPER.mapToInstant(Timestamp.newBuilder().build())); } + @Test + public void mapToInstant_whenSecondsAndNanosIs0_thenMapToEPOCH() { + assertEquals(MAPPER.mapToInstant(Timestamp.newBuilder().build()), Instant.EPOCH); + } + @Test public void mapToInstant_whenNanosIsSet_thenMapToInstant() { assertEquals(3000, MAPPER.mapToInstant(Timestamp.newBuilder().setNanos(3000).build()).getNano()); diff --git a/support-standard/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java b/support-standard/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java index 81846341..f513d21a 100644 --- a/support-standard/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java +++ b/support-standard/src/test/java/no/entur/abt/mapstruct/ProtobufStandardMappingsTest.java @@ -9,12 +9,12 @@ * Licensed under the EUPL, Version 1.1 or – as soon they will be * approved by the European Commission - subsequent versions of the * EUPL (the "Licence"); - * + * * You may not use this work except in compliance with the Licence. * You may obtain a copy of the Licence at: - * + * * http://ec.europa.eu/idabc/eupl5 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the Licence is distributed on an "AS IS" basis, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -66,11 +66,18 @@ public void testMapLocalDateToTimestampWintertime() { assertEquals(l, back); } + // No longer true with the new implementation that ban null. @Test + @org.junit.jupiter.api.Disabled public void mapToInstant_whenSecondsAndNanosIs0_thenMapToNull() { assertNull(MAPPER.mapToInstant(Timestamp.newBuilder().build())); } + @Test + public void mapToInstant_whenSecondsAndNanosIs0_thenMapToEPOCH() { + assertEquals(MAPPER.mapToInstant(Timestamp.newBuilder().build()), Instant.EPOCH); + } + @Test public void mapToInstant_whenNanosIsSet_thenMapToInstant() { assertEquals(3000, MAPPER.mapToInstant(Timestamp.newBuilder().setNanos(3000).build()).getNano()); From 41c1abb8a4bfaef01a97ae1e3573a39cb658786d Mon Sep 17 00:00:00 2001 From: zart colwing Date: Sun, 20 Mar 2022 19:27:56 +0100 Subject: [PATCH 4/4] add "target/" to the .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index b4f5e923..240abc93 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,4 @@ hs_err_pid* .README.md.html +target/