hi kamil, On Thu, Oct 13, 2011 at 12:38 PM, Kamil Nezval <[email protected]> wrote: > Hi, > > I've been playing around with a possibility to modify already registered > node types and found 2 things (bugs?) regarding to changing property > constraints: > > 1) according to the java doc > (http://jackrabbit.apache.org/api/2.2/org/apache/jackrabbit/spi/commons/node > type/NodeTypeDefDiff.html) it should be possible to remove all constraints > from a property, but it is not (marked as a MAJOR change).
correct, that's a bug in the current implementation. > > 2) it's allowed (TRIVIAL) to set a constraint to a property that had no > constraint at all before, which is wrong, because it could affect the > consistency of existing repository content. correct again, good catch! could you please file a jira issue [0]? thanks! stefan [0] http://jackrabbit.apache.org/issue-tracker.html > > In NodeTypeDefDiff.PropDefDiff.init() there is this test that covers both > situations: > > if (!set1.equals(set2)) { > // valueConstraints have been modified > if (set2.containsAll(set1)) { > // new set is a superset of old set > // => constraints have been removed > // (TRIVIAL change, since constraints are OR'ed) > type = TRIVIAL; > } else { > // constraint have been removed/modified (MAJOR change); > // since we're unable to semantically compare > // value constraints (e.g. regular expressions), all > // modifications are considered a MAJOR change. > type = MAJOR; > } > } > > where set1 is a list of original constraints and set2 contains new > constraints. > > I think the correct version should be something like this (not saying > exactly): > > if (!set1.equals(set2)) { > // valueConstraints have been modified > if (set2.isEmpty()) { > // removing all constraints is a TRIVIAL change > type = TRIVIAL; > } else if (set1.isEmpty()){ > // setting constraints is a MAJOR change > type = MAJOR; > } else if (set2.containsAll(set1))) { > // new set is a superset of old set > // => constraints have been added > // (TRIVIAL change, since constraints are OR'ed) > type = TRIVIAL; > } else { > .... > type = MAJOR; > } > > Could anyone confirm my thoughts? Or am I missing anything? > > Thanks. > > Kamil > >
