Skip to content
This repository was archived by the owner on Apr 20, 2026. It is now read-only.

Serialize Annotation object with all required fields even empty#110

Merged
sathish-kumar-subramani merged 2 commits into
masterfrom
annotation-serialization
Aug 8, 2022
Merged

Serialize Annotation object with all required fields even empty#110
sathish-kumar-subramani merged 2 commits into
masterfrom
annotation-serialization

Conversation

@sathish-kumar-subramani

@sathish-kumar-subramani sathish-kumar-subramani commented Aug 6, 2022

Copy link
Copy Markdown
Contributor

Having JsonInclude:NON_EMPTY removes empty fields (e.g., "")
When the required fields are removed, API calls fail due to missing
fields in the request.

Let's use less restrictive JsonInclude:NON_ABSENT annotation, which
removes only null & absent fields. Required fields are anyway non-nullable.

The failing test commit 5a1724f shows how NON_EMPTY rule works

Failed tests: 
  AnnotationTest.serializesWithEmptyFields:80 
Expected: is "{\"path\":\"\",\"annotation_level\":\"notice\",\"message\":\"\",\"title\":\"\",\"start_line\":1,\"end_line\":2}"
     but: was "{\"annotation_level\":\"notice\",\"title\":\"\",\"start_line\":1,\"end_line\":2}"

^ from failed build logs

Having JsonInclude:NON_EMPTY removes empty fields (e.g., "")
When the required fields are removed, API calls fail due to missing
 fields in the request.

Let's use less restrictive JsonInclude:NON_NULL annotation, which
removes only null fields. Required fields are anyway non-nullable.
@codecov

codecov Bot commented Aug 6, 2022

Copy link
Copy Markdown

Codecov Report

Merging #110 (4543f28) into master (d0c05a4) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #110   +/-   ##
=========================================
  Coverage     73.06%   73.06%           
  Complexity      245      245           
=========================================
  Files            39       39           
  Lines           880      880           
  Branches         39       39           
=========================================
  Hits            643      643           
  Misses          212      212           
  Partials         25       25           
Impacted Files Coverage Δ
.../java/com/spotify/github/v3/checks/Annotation.java 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

@sathish-kumar-subramani sathish-kumar-subramani merged commit 790f359 into master Aug 8, 2022
@sathish-kumar-subramani sathish-kumar-subramani deleted the annotation-serialization branch August 8, 2022 08:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants