GeoTools

PropertyDataStore doesn't support empty trailing spaces

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 8.0-M0
  • Component/s: property plugin
  • Labels:
    None

Description

Reading from the following .properties file fails:

_=description:String,name:String
GenericEntity.f004=description-f004|name-f004
GenericEntity.f007=description-f007|

The second feature has an empty name, but the parser fails. Solution is to replace

text = line.substring(split + 1).split("\\|");

by 

text = line.substring(split + 1).split("\\|", -1);

at PropertyAttributeReader.

Issue Links

relates to

Sub-task - The sub-task of the issue GEOT-3546 PropertyAttributeReader fails to translate empty value to null

  • Blocker - Blocks development and/or testing work, production could not run
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Gabriel Roldan added a comment -
Jody, I've committed a patch and updated test case at r37006 as I need the geoserver build going. Please verify its correctness before closing the issue?

Cheers,
Gabriel
Show
Gabriel Roldan added a comment - Jody, I've committed a patch and updated test case at r37006 as I need the geoserver build going. Please verify its correctness before closing the issue? Cheers, Gabriel
Gabriel Roldan made changes -
Field Original Value New Value
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Hide
Permalink
Jody Garnett added a comment -
I am a bit confused; my copy of Java does not have javadocs for split( string, int ) so I cannot quite tell what you are doing.
There are a couple of places in the multiline patch where training spaces will not work; namely:

http://download.oracle.com/javase/6/docs/api/java/util/Properties.html#load(java.io.Reader)

Reading that more carefully I see only leading whitespace is ignored; so I will no longer call String.trim(). this does mean if you require leading whitespace we will need to "escape" it with backslashes.

I think we better escape the | character as well.
Show
Jody Garnett added a comment - I am a bit confused; my copy of Java does not have javadocs for split( string, int ) so I cannot quite tell what you are doing. There are a couple of places in the multiline patch where training spaces will not work; namely: http://download.oracle.com/javase/6/docs/api/java/util/Properties.html#load(java.io.Reader) Reading that more carefully I see only leading whitespace is ignored; so I will no longer call String.trim(). this does mean if you require leading whitespace we will need to "escape" it with backslashes. I think we better escape the | character as well.
Hide
Permalink
Jody Garnett added a comment -
I cannot tell what you are expecting from an empty attribute: null or "" ? Your patch does not test the result.
We explicitly code up "<null>" to represent null; but reviewing the code returns "" as null if the attributeType isNillable and not empty string.

Your patch adds a new entry "fid5" which conflicts a bit with the use of fid5 as explored when making concurrent modifications.
Show
Jody Garnett added a comment - I cannot tell what you are expecting from an empty attribute: null or "" ? Your patch does not test the result. We explicitly code up "<null>" to represent null; but reviewing the code returns "" as null if the attributeType isNillable and not empty string. Your patch adds a new entry "fid5" which conflicts a bit with the use of fid5 as explored when making concurrent modifications.
Hide
Permalink
Jody Garnett added a comment -
Okay commit made to address leading whitespace only; explicit test cases for "table". Will leave you fid5 changes in as I am worried to fix them.

Please review commit when ready gabriel and close when happy.
Show
Jody Garnett added a comment - Okay commit made to address leading whitespace only; explicit test cases for "table". Will leave you fid5 changes in as I am worried to fix them. Please review commit when ready gabriel and close when happy.
Ben Caradoc-Davies made changes -
Link This issue relates to GEOT-3546 [ GEOT-3546 ]
Hide
Permalink
Andrea Aime added a comment -
Mass closing all issues that have been in "resolved" state for more than one month without further comments
Show
Andrea Aime added a comment - Mass closing all issues that have been in "resolved" state for more than one month without further comments
Andrea Aime made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

  • Assignee:
    Jody Garnett
    Reporter:
    Gabriel Roldan
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: