Added replacer to avoid 10e in node-names #91
Conversation
wachterjohannes
force-pushed
the
hotfix/avoid-10e-node-names
branch
from
cb6e3b3
to
37da348
Compare
| @@ -196,6 +196,9 @@ private function getName(AutoNameBehavior $document, NodeInterface $parentNode, | |||
|
|
|||
| $name = $this->slugifier->slugify($title); | |||
|
|
|||
| // jackrabbit can not handle node-names which contains a number followed by "e" e.g. 10e | |||
| $name = preg_replace('((\d+)e)', '$1-e', $name); | |||
There was a problem hiding this comment.
i think in my analysis at jackalope/jackalope#313 i found that only .-% before a number trigger the problem. and that uppercase E is also a problem. maybe '([\.-%]\d+)([eE])', '$1-$2 ?
There was a problem hiding this comment.
thats right (: i will change it to this regex. thanks for your feedback
|
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 |
|
@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. |
wachterjohannes
force-pushed
the
hotfix/avoid-10e-node-names
branch
from
37da348
to
b537289
Compare
|
The JIRA issue is https://issues.apache.org/jira/browse/JCR-3985, just putting it here for reference. |
|
This error also happens for |
|
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.
|
|
@danrot most of the cases should be covered because |
|
The problem is that the |
wachterjohannes
force-pushed
the
hotfix/avoid-10e-node-names
branch
3 times, most recently
from
f490475
to
3fb04b2
Compare
|
@danrot fixed |
| ======= | ||
| * dev-master | ||
| * HOTFIX #91 Added replacer to avoid 10e in node-names | ||
| >>>>>>> added replacer to avoid 10e in node-names |
There was a problem hiding this comment.
Please solve that merge conflict 😃
There was a problem hiding this comment.
haha (: seems that i need glasses
wachterjohannes
force-pushed
the
hotfix/avoid-10e-node-names
branch
from
3fb04b2
to
82e9ede
Compare
|
@danrot now everything should be fine |
| */ | ||
|
|
||
|
|
There was a problem hiding this comment.
These two additions were not here before, were they? Both of them should be removed, same for the other file.
|
And can you please rebase the branch and remove the merge commit? 😃 |
wachterjohannes
force-pushed
the
hotfix/avoid-10e-node-names
branch
from
82e9ede
to
62b83a2
Compare
|
ping @wachterjohannes |
wachterjohannes
force-pushed
the
hotfix/avoid-10e-node-names
branch
2 times, most recently
from
fc15a9f
to
86bcb12
Compare
|
@danrot rebased |
| @@ -199,6 +200,9 @@ private function getName( | |||
|
|
|||
| $name = $this->slugifier->slugify($title); | |||
|
|
|||
| // jackrabbit can not handle node-names which contains a number followed by "e" e.g. 10e | |||
| $name = preg_replace('(([\.%-]?\d+)([eE]))', '$1-$2', $name); | |||
There was a problem hiding this comment.
The first part of the regex [\.%-]? could be removed now, since it matches everytime. The ? modifier says that it can occur, but doesn't have to. If we really want to make this happen only when there is a ., %, - or nothing in front of it the regex gets more complicated, and I don't know if it is worth it 😕
wachterjohannes
force-pushed
the
hotfix/avoid-10e-node-names
branch
from
86bcb12
to
026fb2f
Compare
This PR avoids
10ein node names by replacing it. This replaces sulu/sulu#2387 .