Request for annotation for null values on pojo fields of type String.

[BrentonPoke]

A @Default annotation was requested to protect pojos coming back with null strings in #661. I interpreted it to mean something along the lines of what's in the unit test. Would appreciate feedback.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.31%. Comparing base (125a9ca) to head (f54d76a).
⚠️ Report is 220 commits behind head on master.

Files with missing lines	Patch %	Lines
src/main/java/org/influxdb/dto/Point.java	94.44%	0 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #693      +/-   ##
============================================
+ Coverage     88.26%   88.31%   +0.04%     
  Complexity      730      730              
============================================
  Files            69       69              
  Lines          2540     2558      +18     
  Branches        268      277       +9     
============================================
+ Hits           2242     2259      +17     
  Misses          210      210              
- Partials         88       89       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[BrentonPoke]

Anyone with the ability to re-run jobs mind running the one that failed? All the others passed.

[sranka]

@BrentonPoke I restarted the tests, they now pass. I am not an expert in this library, I took a quick look at the new code while restarting the tests. It seems to me that

• the Default value always overrides any value of the field
• the new test is not testing the Default annotation, it checks that the set values are transferred to point
• the new Default annotation always sets a String value even for other supported field types (Double, Integer, Float, BigDecimal, Instant, Boolean), which would certainly be unexpected
• the relationship to null/empty values mapped to 0 for a double ? #661 is not clear to me, it seems to me that it is already fixed by 6afc6ad#diff-bd011207d3113c41cd8be4968d2321cb

[BrentonPoke]

@BrentonPoke I restarted the tests, they now pass. I am not an expert in this library, I took a quick look at the new code while restarting the tests. It seems to me that

• the Default value always overrides any value of the field
• the new test is not testing the Default annotation, it checks that the set values are transferred to point
• the new Default annotation always sets a String value even for other supported field types (Double, Integer, Float, BigDecimal, Instant, Boolean), which would certainly be unexpected
• the relationship to null/empty values mapped to 0 for a double ? #661 is not clear to me, it seems to me that it is already fixed by 6afc6ad#diff-bd011207d3113c41cd8be4968d2321cb

Thanks for responding, I'm taking another look at this since I originally did it early in the morning, but the comment here is what was agreed upon as the desired change. I went off the end of this discussion that resolved the need-more-info label. For the test, I did struggle a bit in terms of trying to develop a test to check that the value is set to the default. I'm trying a Black Box testing approach, but this is my first time processing an annotation, so I'm doing this PR to learn how. I'm changing the processing of the annotation to only trigger on a String variable, but the reason I didn't include numerical values is that it wasn't part of the request agreed upon at the end of #661.

[BrentonPoke]

Ok, I think it's ready for another look, if anyone wants.