improvement: allowing more granular control of reading behaviour for base64 #646
Conversation
|
Excellent! I'll try to have a look over the weekend (there's a healthy backlog of things but I want to make sure this still gets in 2.12). One other thing is the CLA; if I haven't asked for one, it's here: https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf and the easiest way usually is to print it, fill & sign, scan/photo, email to |
| /** | ||
| * Whether padding characters should be required or not while decoding | ||
| */ | ||
| private final transient PaddingReadBehaviour _paddingReadBehaviour; |
There was a problem hiding this comment.
Let's remove transient here -- not sure why preceding final field has it, possibly copy-paste issue originally (I will remove it from others too)
There was a problem hiding this comment.
Removed transient
|
Looks pretty good; one thing missing (aside from CLA) would be unit tests: just basic demonstration that handling works wrt new settings. |
|
I have made the requested changes (let me know if there are more scenarios/flows that need to be unit tested) |
|
@pavan-kalyan A few contributors have managed to do it all digitally -- not sure how unfortunately (to share details), but we are fine with digital print as signature, no need to produce physical copy. Just need an image (pdf, png, jpeg) with information and signature of some kind (like ones that are used on online documents, not necessarily based on your handwriting). |
|
I have sent the CLA to the email specified |
|
|
||
| } | ||
|
|
||
| private enum PaddingReadBehaviour { |
There was a problem hiding this comment.
Since exposed by public accessor, needs to be public type (I can change this post-merge).
There was a problem hiding this comment.
Changed and added comments and @since
|
Looks good. Could use some more javadocs in general, but one area where I think description is needed is for I hope to get this merged tomorrow, and did receive CLA (thank you!). |
|
I have added more comments, let me know if there should be more docs. |
|
@pavan-kalyan Thanks! I think that comment makes sense both there AND a brief note in each variant (in latter case can just say that "writes padding on output; does not accept padding when reading (may change latter with a call to [LINK to method to change])"? |
|
Changed to Edit: |
|
Merged to 2.12, up to master as well. Thank you very much for getting this through! |
|
Thanks for patiently guiding me through this! |
As Discussed in #500.
Allowing configuration of read behaviour via trio methods (withPadddingForbidden, withPaddingAllowed, withPaddingRequired)
existing usage of usesPadding now changes meaning to writePadding. (maybe this should be renamed?)
also there is a method included withWritePadding(boolean) to allow configuration of whether padding should be written derived from the Base64Variants.
Existing constructors set the following read behaviour. if usesPadding is enabled, then Padding on read is required. if usesPadding is disabled then Padding on Read is forbidden
Testing and validation pending.