rcarz/jira-client — pull request #43 by javier-molina (2014-08-01 06:35:53+00:00)
Title:  null validations for fields and more...

This pull request consist of three different areas.

1. Adding component.getDescription()
2. Adding number datatype to Field.toJson()
3. Include project key and issue type to error message in Field.getCreateMetadata()
4. Add null validations to FluentCreate.field and Field.toJson()

From the above, what I really need you to cast your eyes on is point 4 that I discuss below

Before the proposed changes, I found that inadvertently setting a field to null during FluentCreate.field() calls would later cause a NPE when calling FluentCreate.execute(). There was no way to troubleshoot what caused that NPE unless the developer using the jira-client debugged it.

I can see, other parts of Field.toJson() are using JSONNull.getInstance() instead of throwing an exception, however I fail to see why someone would want to set field to null in particular for the project field or in general for value field in any ValueTuple instance. FluentUpdate uses the REST remove operation that does not use Field.toJson() as far as I can tell. What am I missing here?

The discussion above turns around on whether to throw a more meaningfull exception when a null value is encountered or return the appropriate construct for a null value (such as JSONNull.getInstance()) instead.

Regardless of the approach, a check for null needs to be performed everywhere in Field.toJson() for every different type, otherwise those NPE will pop up and users of the library will have little clue as to the cause of it.