Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reading of Neo4j literal byte[] arrays. #793

Merged
merged 5 commits into from
May 6, 2020
Merged

Conversation

michael-simons
Copy link
Collaborator

We optimized the previous convertToIterable (now iterableOf in CollectionUtils to actually return an iterable. In the case of byte arrays, the EntityAccessManager casts that Iterable into a collection to coerce it with existing values. That lead to the ClassCastException mentioned in #791.

The issue needs several fixes:

  • EntityAccessManager must only work on Iterables
  • The CollectionUtils must treat an array of primitive types separatly. It cannot be just casted into an Object[] array.

This change also introduces two NoOp-Converter (NoOpByteArrayConverter and NoOpWrappedByteArrayConverter) that allow users to circumenvent the conversion to Base64 arrays.

We can read Base64 encoded into byte[] arrays from HTTP, but not write byte[] arrays over HTTP as the HTTP endpoint doesn’t support byte[] as parameter.

This closes #791.

We optimized the previous `convertToIterable` (now `iterableOf` in `CollectionUtils` to actually return an iterable. In the case of byte arrays, the `EntityAccessManager` casts that `Iterable` into a collection to coerce it with existing values. That lead to the `ClassCastException` mentioned in #791.

The issue needs several fixes:
- `EntityAccessManager` must only work on `Iterables`
- The `CollectionUtils` must treat an array of primitive types separatly. It cannot be just casted into an `Object[]` array.

This change also introduces two NoOp-Converter (`NoOpByteArrayConverter` and `NoOpWrappedByteArrayConverter`) that allow users to circumenvent the conversion to Base64 arrays.

We can read Base64 encoded into byte[] arrays from HTTP, but not write byte[] arrays over HTTP as the HTTP endpoint doesn’t support byte[] as parameter.

This closes #791.
@codecov-io
Copy link

codecov-io commented May 6, 2020

Codecov Report

Merging #793 into master will increase coverage by 0.00%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #793   +/-   ##
=========================================
  Coverage     80.32%   80.33%           
- Complexity     3016     3031   +15     
=========================================
  Files           275      277    +2     
  Lines          9218     9248   +30     
  Branches       1377     1380    +3     
=========================================
+ Hits           7404     7429   +25     
  Misses         1310     1310           
- Partials        504      509    +5     
Impacted Files Coverage Δ Complexity Δ
...eo4j/ogm/typeconversion/MapCompositeConverter.java 91.66% <42.85%> (-4.59%) 34.00 <2.00> (+2.00) ⬇️
.../typeconversion/NoOpWrappedByteArrayConverter.java 69.23% <69.23%> (ø) 5.00 <5.00> (?)
...eo4j/ogm/metadata/reflect/EntityAccessManager.java 68.34% <85.71%> (-0.49%) 70.00 <4.00> (+2.00) ⬇️
...in/java/org/neo4j/ogm/support/CollectionUtils.java 100.00% <100.00%> (ø) 5.00 <1.00> (+1.00)
...o4j/ogm/typeconversion/NoOpByteArrayConverter.java 100.00% <100.00%> (ø) 3.00 <3.00> (?)
...ore/src/main/java/org/neo4j/ogm/session/Utils.java 46.15% <0.00%> (+3.29%) 32.00% <0.00%> (+1.00%)
...java/org/neo4j/ogm/result/adapter/BaseAdapter.java 100.00% <0.00%> (+22.22%) 4.00% <0.00%> (+1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f1bff7...1c08be4. Read the comment docs.

@michael-simons michael-simons merged commit 30605a4 into master May 6, 2020
@michael-simons michael-simons deleted the issue/791 branch May 6, 2020 10:14
michael-simons added a commit that referenced this pull request May 6, 2020
We optimized the previous `convertToIterable` (now `iterableOf` in `CollectionUtils` to actually return an iterable. In the case of byte arrays, the `EntityAccessManager` casts that `Iterable` into a collection to coerce it with existing values. That lead to the `ClassCastException` mentioned in #791.

The issue needs several fixes:
- `EntityAccessManager` must only work on `Iterables`
- The `CollectionUtils` must treat an array of primitive types separately. It cannot be just casted into an `Object[]` array.
- The `BaseAdapter` class is now clear about the fact that it turns arrays into collections and not only iterables. If it doesn't do this, several things breaks downstream.

This change also introduces two NoOp-Converter (`NoOpByteArrayConverter` and `NoOpWrappedByteArrayConverter`) that allow users to circumvent the conversion to Base64 arrays.

We can read Base64 encoded into byte[] arrays from HTTP, but not write byte[] arrays over HTTP as the HTTP endpoint doesn’t support byte[] as parameter.

This closes #791.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ClassCastException when reading entity with byte[] property from database
2 participants