jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kamil Nezval" <kamil.nez...@xitee.com>
Subject Property constraints modification
Date Thu, 13 Oct 2011 10:38:14 GMT
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).
 
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.

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


Mime
View raw message