Skip to content

Commit

Permalink
Don't return null for non-nullable Blobs in Java (#935)
Browse files Browse the repository at this point in the history
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<vector>` 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 <[email protected]>
  • Loading branch information
DanielKamkha authored Jun 1, 2021
1 parent 6574ee5 commit 134cbce
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
4 changes: 2 additions & 2 deletions functional-tests/functional/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,8 @@ class ArraysByteBuffer {
errorFlag: Boolean
): /* Output buffer */ Blob throws Explosive
}

class BlobNulls {
static fun getBreakingNull(): Blob
static fun getValidNull(): Blob?
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
// -------------------------------------------------------------------------------------------------

#include "test/ArraysByteBuffer.h"
#include "test/BlobNulls.h"
#include "another/TypeCollectionWithEnums.h"

namespace test
Expand Down Expand Up @@ -84,4 +85,14 @@ ArraysByteBuffer::method_that_explodes( const bool error_flag )
}
}

} // namespace test
std::shared_ptr<std::vector<uint8_t>>
BlobNulls::get_breaking_null() {
return {};
}

lorem_ipsum::test::optional<std::shared_ptr<std::vector<uint8_t>>>
BlobNulls::get_valid_null() {
return {};
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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( ) );
Expand Down

0 comments on commit 134cbce

Please sign in to comment.