sulu / sulu-document-manager

Sulu document manager
sulu.io
6 stars 7 forks source link

Added replacer to avoid 10e in node-names #91

Closed wachterjohannes closed 8 years ago

wachterjohannes commented 8 years ago

This PR avoids 10e in node names by replacing it. This replaces https://github.com/sulu/sulu/pull/2387 .

dbu commented 8 years ago

can the user ever manually define node names? or does that also go through this auto-namer? you might want to un-escape when loading, unless the names are never publicly visible (not even in api calls). if you do, you should replace (\d)+(-+)([eE]) by $1$2-$3 and always remove one - so that names that would need no escaping are not mangled.

wachterjohannes commented 8 years ago

@dbu all nodes which we use for querying is generated by this auto-name behavior. i don't think we want to undo this for the user because if he want to use this path it should be valid.

danrot commented 8 years ago

The JIRA issue is https://issues.apache.org/jira/browse/JCR-3985, just putting it here for reference.

danrot commented 8 years ago

This error also happens for .10e and only 10e, but the regex doesn't cover these. I guess when it comes to the regex they are both the same, because the . is probably removed before.

dbu commented 8 years ago

the jira ticket says that its about big decimal parsing. which makes no sense probably, but maybe reading how java parses big decimals can help with the escaping to cover all cases.

wachterjohannes commented 8 years ago

@danrot most of the cases should be covered because . or , will be replaced by a - so they wont ma a problem there. i have tested now multiple formats and i have not found any which causes this problem ...

danrot commented 8 years ago

The problem is that the . doesn't come to the regex, at least that is my guess. And just calling a site 10e also didn't work, which makes sense, since the regex says that there has to be something before 10e.

wachterjohannes commented 8 years ago

@danrot fixed

wachterjohannes commented 8 years ago

@danrot now everything should be fine

danrot commented 8 years ago

And can you please rebase the branch and remove the merge commit? 😃

chirimoya commented 8 years ago

ping @wachterjohannes

wachterjohannes commented 8 years ago

@danrot rebased