Skip to content

Conversation

@zartc
Copy link

@zartc zartc commented Mar 20, 2022

Here's the code that implements the ban of null return values from mappers.

  • null return values from domain->proto mapper will end up with a NPE thrown by Protobuff.
  • null return values from proto->domain is less dangerous but it is best to let the application decide what to do in a post-mapper step.

The changes consist of systematically return a "default' value: empty string or arrays, EPOCH, zero, or protobuff's getDefaultInstance.

Close #176

zartc added 4 commits March 20, 2022 19:58
- 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
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.
… null

replace one unit-test to conform with the new implementation that ban null
@seime
Copy link
Contributor

seime commented Sep 8, 2022

@zartc Sorry for the late response,

I agree with your reasoning. On the proto->domain side I would like to see 2 different implementations, yours (map to empty domain) as well as the existing one (map to null).

If we split the existing mapper class into 3 variants

  • DomainToProto
  • ProtoToEmptyDomain
  • ProtoToNullDomain

the developer can select the appropriate implementation based on his/her needs. It also avoid breaking (runtime) any existing use of this library (mapper uses attribute must be updated, but this is detected compile time)

@wirekang
Copy link

Any news?

@erlendnils1
Copy link
Contributor

This has unfortunately gone unnoticed. The handling for mapping TO protobuf has been included in #498

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.

Mappers in protobuf-support-(core | standard | lite) should **never return null**.

4 participants