Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDDTool: erroneously replaces TBD values with null in JSON output #227

Closed
ztaylor54 opened this issue Sep 8, 2020 · 3 comments · Fixed by #232
Closed

LDDTool: erroneously replaces TBD values with null in JSON output #227

ztaylor54 opened this issue Sep 8, 2020 · 3 comments · Fixed by #232
Assignees
Labels
bug Something isn't working

Comments

@ztaylor54
Copy link

Describe the bug
When running LDDTool to produce JSON output, <definition> values in <DD_Attribute> that start with TBD are replaced with null in the output.

e.g. the below <DD_Attribute>

<!-- TODO definition & value domain for image_acquire_mode -->
  <DD_Attribute>
    <name>image_acquire_mode</name>
    <version_id>1.0</version_id>
    <local_identifier>image_acquire_mode</local_identifier>
    <nillable_flag>false</nillable_flag>
    <submitter_name>Cristina De Cesare</submitter_name>
    <definition>TBD</definition>
    <DD_Value_Domain>
      <enumeration_flag>false</enumeration_flag>
      <value_data_type>ASCII_Short_String_Collapsed</value_data_type>
      <unit_of_measure_type>Units_of_None</unit_of_measure_type>
    </DD_Value_Domain>
  </DD_Attribute>

became

{
    "attribute": {
      "identifier": "0001_NASA_PDS_1.img_surface.Instrument_Information.img_surface.image_acquire_mode" ,
      "title": "image_acquire_mode" ,
      "registrationAuthorityId": "0001_NASA_PDS_1" ,
      "nameSpaceId": "img_surface" ,
      "steward": "img" ,
      "versionId": "1.14" ,
      "description": "null" ,
      "isNillable": "false" ,
      "isEnumerated": "false" ,
      "isDeprecated": "false" ,
      "dataType": "ASCII_Short_String_Collapsed" ,
      "dataTypeId": "0001_NASA_PDS_1.pds.ASCII_Short_String_Collapsed" ,
      "minimumCharacters": "1" ,
      "maximumCharacters": "255" ,
      "minimumValue": "Unbounded" ,
      "maximumValue": "Unbounded" ,
      "pattern": "null" ,
      "unitOfMeasure": "null" ,
      "unitOfMeasureId": "null" ,
      "unitId": "null" ,
      "defaultUnitId": "null"
    }
  }

Note that in the JSON, description is null.

To Reproduce
Steps to reproduce the behavior:

  1. Install LDDTool version 11.3.2
  2. Download any PDS local data dictionary (I used IMG_SURFACE)
  3. Change the <definition> attribute on any of the <DD_Attribute> blocks to start with TBD
  4. Run LDDTool with -lpJ options to produce JSON output
  5. Inspect attribute block in JSON output for xpath of attribute you changed to TBD
  6. See null in the description

Expected behavior
I expected the text in the <definition> attribute to pass through to the description in the JSON output.

Version of Software Used

% /usr/local/lddtool-11.3.2/bin/lddtool -v
 
LDDTool Version: 11.3.2
Built with IM Version: 1.14.0.0
Build Date: 2020-08-17 14:53:24

Test Data / Additional context
I suspect the offending code block is in pds4-information-model/model-dmdocument/src/main/java/gov/nasa/pds/model/plugin/WriteLODSKOSFileDOM.java:

// Format the String for JSON
public String formValue(String lString) {
	String rString = lString;
	if (rString == null) rString = "null";
	if (rString.indexOf("TBD") == 0) rString = "null";
	rString = DOMInfoModel.escapeJSONChar(rString);
	rString = "\"" + rString + "\"";
	return rString;
}

The .indexOf("TBD") == 0 is functionally equivalent to a .startsWith("TBD"), so I tested this by adding some text before the TBD value in the dictionary, then re-running LDDTool:

% diff json/PDS4_IMG_SURFACE_1E00_1200.JSON json/PDS4_IMG_SURFACE_1E00_1200_BAD.JSON                          
6c6
<       "Date": "Tue Sep 01 08:36:15 PDT 2020" ,
---
>       "Date": "Tue Sep 01 08:20:06 PDT 2020" ,
1526c1526
<             "description": "This is TBD" ,
---
>             "description": "null" ,

When adding text before the TBD, I get the expected behavior.

Additional Thoughts
When investigating this bug I was curious if this was an isolated issue, so I did some digging:

% grep TBD pds4-information-model/model-dmdocument/src/main/java/gov/nasa/pds/model/plugin/WriteLODSKOSFileDOM.java | wc -l
      13

So there are a couple occurrences in the offending file, but when I checked the whole repository:

% grep -r TBD pds4-information-model | wc -l                                                                               
   47808

Is TBD an official placeholder value that is documented somewhere? Surely it is a reasonable value to have in a dictionary attribute when a value is going to be filled in later, and null in the JSON loses that information, and just makes it seem like the value is missing in error.

@ztaylor54 ztaylor54 added bug Something isn't working triage-needed labels Sep 8, 2020
@jshughes
Copy link
Collaborator

The IMTool/LDDTool software, by default, initializes all attribute values to "TBD_attribute_name". The string "TBD*" is assumed to be a variation of the JAVA value "null" and is ultimately written as "null" if no value is supplied. The proposed fix is to recognize the string "TBD" as a user-provided value and then replace it with something like "_TBD" to eliminate the ambiguity. This is a kludge however it is assumed that all user-provided "TBDs" will ultimately be removed. Comments welcome.

It is interesting that this bug was only just found.

@jordanpadams jordanpadams changed the title LDDTool replaces TBD values with null in JSON output LDDTool: erroneously replaces TBD values with null in JSON output Sep 16, 2020
@jordanpadams
Copy link
Member

@ztaylor54 ⬆️ let us know if this will not work for you

@ztaylor54
Copy link
Author

This should work. @jshughes is right - all user-provided "TBD*" will ultimately be removed, but while LDDs are in development it adds some confusion.

Thanks!

jshughes added a commit that referenced this issue Sep 21, 2020
LDDTool: erroneously replaces TBD values with null in JSON output. When running LDDTool to produce JSON output, <definition> values in <DD_Attribute> that start with TBD are replaced with null in the output.

Since LDDTool uses "TBD" as an equivalent for "null" to indicate that a user value was not provided, the solution is to prepend a user supplied "TBD" with "_" resulting in "_TBD".

Resolves #227
jshughes added a commit that referenced this issue Sep 21, 2020
LDDTool: erroneously replaces TBD values with null in JSON output. When running LDDTool to produce JSON output, <definition> values in <DD_Attribute> that start with TBD are replaced with null in the output.

Since LDDTool uses "TBD" as an equivalent for "null" to indicate that a user value was not provided, the solution is to prepend a user supplied "TBD" with "_" resulting in "_TBD".

Resolves #227
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants