From 134cbceb93f9308b389b08e1c80a57b134f2f4b8 Mon Sep 17 00:00:00 2001 From: Daniel Kamkha Date: Tue, 1 Jun 2021 16:02:27 +0200 Subject: [PATCH] Don't return null for non-nullable Blobs in Java (#935) Updated JNI conversion function for non-nullable Blobs to avoid returning `null` when the C++ function returns a null shared pointer. Having a null shared pointer coming from C++ when the type is specified as non-nullable in the IDL is a violation of contract. The correct fix for this issue would be to change `Blob` representation from `shared_ptr` to something that cannot be `null`. However, this would be a breaking change, so it has to be done later (#934). For now it's just the small JNI workaround. Resolves: #929 Signed-off-by: Daniel Kamkha --- CHANGELOG.md | 4 ++++ functional-tests/functional/CMakeLists.txt | 4 ++-- ...ArraysByteBufferTest.java => BlobsTest.java} | 17 ++++++++++++++++- .../lime/{ArraysByteBuffer.lime => Blobs.lime} | 5 +++++ .../src/cpp/{ArraysByteBuffer.cpp => Blobs.cpp} | 13 ++++++++++++- ...JniCppConversionUtilsImplementation.mustache | 2 +- 6 files changed, 40 insertions(+), 5 deletions(-) rename functional-tests/functional/android/src/test/java/com/here/android/test/{ArraysByteBufferTest.java => BlobsTest.java} (94%) rename functional-tests/functional/input/lime/{ArraysByteBuffer.lime => Blobs.lime} (96%) rename functional-tests/functional/input/src/cpp/{ArraysByteBuffer.cpp => Blobs.cpp} (92%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54efd4b557..6545e62312 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Gluecodium project Release Notes +## Unreleased +### Bug fixes: + * Fixed runtime issues in Java when a 'null' is returned from C++ for a non-nullable `Blob`. + ## 9.1.0 Release date: 2021-05-31 ### Features: diff --git a/functional-tests/functional/CMakeLists.txt b/functional-tests/functional/CMakeLists.txt index 2d629a9dce..46c5e49032 100644 --- a/functional-tests/functional/CMakeLists.txt +++ b/functional-tests/functional/CMakeLists.txt @@ -86,10 +86,10 @@ feature(MethodOverloading cpp android swift dart SOURCES ) feature(Blob cpp android swift dart SOURCES - input/src/cpp/ArraysByteBuffer.cpp + input/src/cpp/Blobs.cpp input/src/cpp/StaticByteArrayMethods.cpp - input/lime/ArraysByteBuffer.lime + input/lime/Blobs.lime input/lime/StaticByteArrayMethods.lime ) diff --git a/functional-tests/functional/android/src/test/java/com/here/android/test/ArraysByteBufferTest.java b/functional-tests/functional/android/src/test/java/com/here/android/test/BlobsTest.java similarity index 94% rename from functional-tests/functional/android/src/test/java/com/here/android/test/ArraysByteBufferTest.java rename to functional-tests/functional/android/src/test/java/com/here/android/test/BlobsTest.java index f74159c979..5c268e0413 100644 --- a/functional-tests/functional/android/src/test/java/com/here/android/test/ArraysByteBufferTest.java +++ b/functional-tests/functional/android/src/test/java/com/here/android/test/BlobsTest.java @@ -32,7 +32,7 @@ @RunWith(RobolectricTestRunner.class) @Config(sdk = Build.VERSION_CODES.M, application = RobolectricApplication.class) -public class ArraysByteBufferTest { +public class BlobsTest { @Test public void methodWithByteBuffer_emptyArray() { @@ -167,4 +167,19 @@ public void methodWithExplicitArrayInStruct_reversesArray() { assertNotNull(resultStruct); assertEquals(java.util.Arrays.asList((short) 3, (short) 2, (short) 1), resultStruct.image); } + + @Test + public void blobNullsBreakingNull() { + byte[] resultBuffer = BlobNulls.getBreakingNull(); + + assertNotNull(resultBuffer); + assertEquals(0, resultBuffer.length); + } + + @Test + public void blobNullsValidNull() { + byte[] resultBuffer = BlobNulls.getValidNull(); + + assertNull(resultBuffer); + } } diff --git a/functional-tests/functional/input/lime/ArraysByteBuffer.lime b/functional-tests/functional/input/lime/Blobs.lime similarity index 96% rename from functional-tests/functional/input/lime/ArraysByteBuffer.lime rename to functional-tests/functional/input/lime/Blobs.lime index a6648fa4a1..d29e54d6be 100644 --- a/functional-tests/functional/input/lime/ArraysByteBuffer.lime +++ b/functional-tests/functional/input/lime/Blobs.lime @@ -64,3 +64,8 @@ class ArraysByteBuffer { errorFlag: Boolean ): /* Output buffer */ Blob throws Explosive } + +class BlobNulls { + static fun getBreakingNull(): Blob + static fun getValidNull(): Blob? +} diff --git a/functional-tests/functional/input/src/cpp/ArraysByteBuffer.cpp b/functional-tests/functional/input/src/cpp/Blobs.cpp similarity index 92% rename from functional-tests/functional/input/src/cpp/ArraysByteBuffer.cpp rename to functional-tests/functional/input/src/cpp/Blobs.cpp index 4feaf83c07..1b68c38c3a 100644 --- a/functional-tests/functional/input/src/cpp/ArraysByteBuffer.cpp +++ b/functional-tests/functional/input/src/cpp/Blobs.cpp @@ -19,6 +19,7 @@ // ------------------------------------------------------------------------------------------------- #include "test/ArraysByteBuffer.h" +#include "test/BlobNulls.h" #include "another/TypeCollectionWithEnums.h" namespace test @@ -84,4 +85,14 @@ ArraysByteBuffer::method_that_explodes( const bool error_flag ) } } -} // namespace test +std::shared_ptr> +BlobNulls::get_breaking_null() { + return {}; +} + +lorem_ipsum::test::optional>> +BlobNulls::get_valid_null() { + return {}; +} + +} diff --git a/gluecodium/src/main/resources/templates/jni/utils/JniCppConversionUtilsImplementation.mustache b/gluecodium/src/main/resources/templates/jni/utils/JniCppConversionUtilsImplementation.mustache index a6fedb659a..25ee002c59 100644 --- a/gluecodium/src/main/resources/templates/jni/utils/JniCppConversionUtilsImplementation.mustache +++ b/gluecodium/src/main/resources/templates/jni/utils/JniCppConversionUtilsImplementation.mustache @@ -194,7 +194,7 @@ convert_to_jni( JNIEnv* env, const std::shared_ptr< std::vector< uint8_t > >& nv { if ( !nvalue ) { - return {}; + return make_local_ref(env, env->NewByteArray(0)); } jsize size = static_cast< jsize >( nvalue->size( ) );