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

Update content validation to fail when the value of an ASCII_Integer is all spaces #9

Closed
jordanpadams opened this issue Jun 24, 2019 · 9 comments · Fixed by #88
Closed
Assignees
Labels
bug Something isn't working

Comments

@jordanpadams
Copy link
Member

When validating the attached, if you run as follows:

./validate -v 0 -f ../../whitespace_test/minimal_test_product_bad.xml

One of the info messages is:

INFO  [info.table.blank_field_value]   table 1, record 2, field 2: Field is blank.
Record 2 is an ASCII_Integer, but the value is a "space" character (ASCII 0x20) which is not a valid ASCII_Integer. This should be an ERROR not an INFO.
@jordanpadams
Copy link
Member Author

Test products attached. Note: aaaTest label should be used as an example to create a minimal_test_product_success.xml label where it will use special constants to allow for a blank field.

test_data.zip

@msbentley
Copy link

I've just noticed that this issue also affects the ASCII_Real data type - I get the same problem (validation passes, but when I switch to verbosity 1 I get a stream of "Field is blank" INFO level messages, but the product is essentially invalid).

Possibly it affects all numeric types?

@jordanpadams
Copy link
Member Author

De-scoped to work on AIP/SIP Generation software

@jordanpadams jordanpadams removed this from the PDS.25 (ends 2019-09-11) milestone Aug 23, 2019
@galenatjpl galenatjpl added this to the PDS.25 (ends 2019-09-11) milestone Aug 29, 2019
@hhlee445
Copy link
Contributor

hhlee445 commented Sep 5, 2019

Added github9 test case into regression test.

@jordanpadams jordanpadams changed the title ASCII_Integer should fail on space character Update content validation to fail when a space character is used when an ASCII_Integer type is specified Sep 11, 2019
jordanpadams added a commit that referenced this issue Oct 16, 2019
Updates made per #9 sufficed for character tables, but caused an issue for delimited tables.

This update allows for blank fields, but still catches issues when a space character is used instead.

According the PDS DSV Standard in the SR (SR 4C.1) "Leading and trailing spaces in a field are considered part of the field" so that is not an issue.

Resolves #137
@rchenatjpl
Copy link

I would change the title of this issue from
"Update content validation to fail when a space character is used when an ASCII_Integer type is specified" to
"Update content validation to fail when the value of an ASCII_Integer is all spaces"
because validate v1.17 allows leading and/or trailing spaces. Actually, I want to verify that that is the desired behavior.

@jordanpadams
Copy link
Member Author

@rchenatjpl that is correct. according to the SR and PDS DSV standard, leading and trailing whitespace are ok.

@jordanpadams jordanpadams changed the title Update content validation to fail when a space character is used when an ASCII_Integer type is specified Update content validation to fail when the value of an ASCII_Integer is all spaces Oct 23, 2019
jordanpadams added a commit that referenced this issue Jan 20, 2020
Previous solution to #9, was implemented with a bug that does not allow empty fields
in delimited tables. The updates committed here enable that, as well as two additional
tests to handle both use cases of an empty fields (valid) and a field that contains a
space (invalid).

Resolves #170
@rchenatjpl
Copy link

What's the final answer - should validate throw or not throw an error for an all-blank ASCII_Integer? My validate throws errors:
Begin Content Validation: file:/Users/rchen/Desktop/test/testPrep/val9.tab
ERROR [error.table.field_value_data_type_mismatch] table 1, record 1, field 2: Value does not match its data type 'ASCII_Integer': ' ' does not match the pattern '[+-]?\d+'
ERROR [error.table.field_value_data_type_mismatch] table 1, record 2, field 2: Value does not match its data type 'ASCII_Integer': ' ' does not match the pattern '[+-]?\d+'
End Content Validation: file:/Users/rchen/Desktop/test/testPrep/val9.tab
1 product validation(s) completed

@jordanpadams
Copy link
Member Author

@rchenatjpl invalid for a delimited table (PDS DSV), valid for a character table.

@rchenatjpl
Copy link

Thanks, Jordan. Validate v1.22.1 throws an error for both Table_Character and Table_Delimited. pds4_viewer chokes on Table_Character but displays Table_Delimited correctly.
Archive.zip

jordanpadams added a commit that referenced this issue Apr 11, 2020
Update FieldValueValidator to not only allow DelimitedTableRecord empty fields (value.isEmpty()), but also "empty" fields for FixedTableRecords, which are space-padded.

Also includes merging #206 tests with #9 tests.

Resolves #206
jordanpadams added a commit that referenced this issue Apr 11, 2020
…#207)

* issue_206: Validate incorrectly fails for ASCII_Integer value in Table_Character

* Update field value validation check for FixedTableRecord

Update FieldValueValidator to not only allow DelimitedTableRecord empty fields (value.isEmpty()), but also "empty" fields for FixedTableRecords, which are space-padded.

Also includes merging #206 tests with #9 tests.

Resolves #206

* Update integration tests per #206

Co-authored-by: Jordan Padams <jordan.h.padams@jpl.nasa.gov>
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.

6 participants