Namere validity in the subscription validator

Compile the namere value to detect early the possible exceptions. 

Also check the nameprefix, must follow the same rules as a name.

[Thomas.J.Waldmann, 2013-07-20 12:34:30]

https://codereview.appspot.com/11403044/diff/1/MoinMoin/storage/middleware/va...
File MoinMoin/storage/middleware/validation.py (right):

https://codereview.appspot.com/11403044/diff/1/MoinMoin/storage/middleware/va...
MoinMoin/storage/middleware/validation.py:349: "'itemid', 'name', 'tags',
'namere' or 'nameprefix'.")
this should use same constants as the code uses elsewhere.

https://codereview.appspot.com/11403044/diff/1/MoinMoin/storage/middleware/va...
MoinMoin/storage/middleware/validation.py:359: re.compile(r'{0}'.format(value))
what's the purpose of r'{0}'.format(value) ?

https://codereview.appspot.com/11403044/diff/1/MoinMoin/storage/middleware/va...
MoinMoin/storage/middleware/validation.py:364: element.add_error("The value of
subscription is not valid.")
"Subscription has invalid value."

[ana.balica, 2013-07-20 13:22:39]

https://codereview.appspot.com/11403044/diff/1/MoinMoin/storage/middleware/va...
File MoinMoin/storage/middleware/validation.py (right):

https://codereview.appspot.com/11403044/diff/1/MoinMoin/storage/middleware/va...
MoinMoin/storage/middleware/validation.py:359: re.compile(r'{0}'.format(value))
On 2013/07/20 12:34:31, Thomas.J.Waldmann wrote:
> what's the purpose of r'{0}'.format(value) ?

To compile a raw string pattern. 
But actually, the pattern is a user input and will be stored as unicode. Hence
"namere::\d" will be stored as "namere::\\d" with the escaped characters.

Maybe we need to first replace '\\' with '\' and then use them as raw strings?

[waldi, 2013-07-20 13:45:23]

On 2013/07/20 13:22:39, ana.balica wrote:
>
https://codereview.appspot.com/11403044/diff/1/MoinMoin/storage/middleware/va...
> File MoinMoin/storage/middleware/validation.py (right):
> 
>
https://codereview.appspot.com/11403044/diff/1/MoinMoin/storage/middleware/va...
> MoinMoin/storage/middleware/validation.py:359:
re.compile(r'{0}'.format(value))
> On 2013/07/20 12:34:31, Thomas.J.Waldmann wrote:
> > what's the purpose of r'{0}'.format(value) ?
> To compile a raw string pattern. 

This makes no sense, to be exact this should not do anything useful. It is not
your responsibility to escape or un-escape stuff.

> But actually, the pattern is a user input and will be stored as unicode. Hence
> "namere::\d" will be stored as "namere::\\d" with the escaped characters.

What do you mean with "stored"? How it looks on disk is not your concern but the
responsibility of the storage.

> Maybe we need to first replace '\\' with '\' and then use them as raw strings?

No! Raw literals are something to be used in source files, you never handle data
from source files.

[Thomas.J.Waldmann, 2013-07-20 13:49:04]

guess there is a misunderstanding here:

if you have some python code file and it uses string literals, like

normal = "foo\n"
raw = r"foo\n"

the normal string value will then have a linefeed character at the end, because
PYTHON interprets the STRING LITERAL and replaces the \n by the linefeed when
creating the corresponding string object.

for the raw string, it won't interpret it and just take it "as is", so \n will
be just a backslash and a n.

if you already have a string object (like stuff you read from metadata), it is
not a string literal and python will never interpret it like that.
so if you type some \n into metadata, it will always be backslash n and never
will get transformed to a linefeed automatically.

thus, what you are doing above is basically a NOP and you can just use
re.compile(value).

[Thomas.J.Waldmann, 2013-07-20 13:51:37]

one more thing: look at the flags re.compile is using, you'll usually want
re.UNICODE when dealing with unicode.

[waldi, 2013-07-20 13:56:32]

https://codereview.appspot.com/11403044/diff/6001/MoinMoin/storage/middleware...
File MoinMoin/storage/middleware/validation.py (right):

https://codereview.appspot.com/11403044/diff/6001/MoinMoin/storage/middleware...
MoinMoin/storage/middleware/validation.py:349: "ITEMID, NAME, TAGS, NAMERE or
NAMEPREFIX.")
This is a unrelated change.
Please stick to the things you've written in the description.

https://codereview.appspot.com/11403044/diff/6001/MoinMoin/storage/middleware...
MoinMoin/storage/middleware/validation.py:355: elif keyword in (keys.NAME,
keys.NAMEPREFIX, ):
Unrelated change. It changes the behavior of "nameprefix", "namere".

https://codereview.appspot.com/11403044/diff/6001/MoinMoin/storage/middleware...
MoinMoin/storage/middleware/validation.py:364: element.add_error("Subscription
has invalid value.")
This also does not belong in this changeset.

[waldi, 2013-07-20 13:56:58]

On 2013/07/20 13:56:32, waldi wrote:
>
https://codereview.appspot.com/11403044/diff/6001/MoinMoin/storage/middleware...
> File MoinMoin/storage/middleware/validation.py (right):
> 
>
https://codereview.appspot.com/11403044/diff/6001/MoinMoin/storage/middleware...
> MoinMoin/storage/middleware/validation.py:355: elif keyword in (keys.NAME,
> keys.NAMEPREFIX, ):
Unrelated change. It changes the behavior of "nameprefix", not "namere".