[rfc][icedtea-web][policyeditor] Parsing enhancements and unit tests

[Jiri Vanek]

The messages.properties changes are still not listed in changelog.


This is probably nit, but:
+        final Matcher targetMatcher = PolicyEditorPermissions.TARGET_PERMISSION.matcher(string);
+        final Matcher actionMatcher = PolicyEditorPermissions.ACTIONS_PERMISSION.matcher(string);
+        if (!targetMatcher.matches() && !actionMatcher.matches()) {
+            return null;
+        }
+
+        final String typeStr, targetStr, actionsStr;
+
+        if (actionMatcher.matches()) {
+            typeStr = actionMatcher.group(1);
+            targetStr = actionMatcher.group(2);
+            actionsStr = actionMatcher.group(3);
+        } else {
+        	typeStr = targetMatcher.group(1);
+            targetStr = targetMatcher.group(2);
+            actionsStr = "";
          }

the first matcher is needed only when second one fails. Maybe would be nice to remove the 
unnecessary matching.

similar for PolicyEditorPermissions which is still suspiciously similar:(


The chnage [\\w.] -> [\\w\\.]  looks enough :)



Thanx!
J.
On 03/06/2014 09:18 PM, Andrew Azores wrote:
> Thanks for review! Hopefully the new attached patch is nicer.
>
> Thanks,
>
> Andrew A
>
> ----- Original Message -----
> From: "Jiri Vanek" <jvanek at redhat.com>
> To: "Andrew Azores" <aazores at redhat.com>
> Cc: "IcedTea" <distro-pkg-dev at openjdk.java.net>
> Sent: Thursday, March 6, 2014 9:07:46 AM
> Subject: Re: [rfc][icedtea-web][policyeditor] Parsing enhancements and unit tests
>
> On 03/05/2014 06:01 PM, Andrew Azores wrote:
>> Hi,
>>
>> The previous thread was diverging into two quite different patches, so I'm splitting it.
>>
>> This patch improves parsing, especially handling of comments, and adds a lot of new unit testing for this. A few small bugs were found and fixed along the way.
>>
>
> The Messages.properties changes are missing in in-pathc chagelog
>
> public static CustomPermission fromString - more more and more tests. Try to hack yourself.  But generally ok.
> Well the comment is poem-writer's masterpiece :) But better then nothing :)
> The [\\w.] do not osund correct. The dot should be escaped to match dot, not any char. Or not? So this part will be:
> [[\\w\\.]+\\w+] (nottested - to match [java.][io.][permissions] and nto eg dsgfgogfdsgjsfdghjsfd[hj as now :)
> The content of quotes may be anything, ok? What about content to be qutes itself? (not jsut ${} eg?
> The first quotes are mandatory, and the scond not? As far as I read from regex.
> Why is compiled patter not global constant? It should be. It will not need recompile each launch time... Then there can be also some tests only to the regex itself.
>
>
> Why is the  CustomPermission.fromStirng copypasted to PolicyEditorPermissions ???
>
> Quick glance over tests is ook.
>
> Thanx!
>     J.
>