Registering node type with property definitions without validation?

[bjornbal]

When I try to add a new nodetype with for example the type as LONG, I try to add another property definition with constraints. But now I add this constraint as a non-long type (f.e. a simple string like 'hello'). I expected to receive an exception.

I do, when I call session.save() after registerNodeType in the nodeTypeManager, still I am unsure why I need to explicitly call the session.save() to receive this exception since I see a save call in the registerNodeType method. In examples on the net this session.save() is not even called.

But now here comes the issue, when I run the unit test including session.save() where I get the expected exception thrown. But now I add this, this fails:

// First you have 53 node types
assertEquals("The size of the total node types is not correct", 53, nodeTypeManager.getAllNodeTypes().getSize());
nodeTypeManager.registerNodeType(nodeTypeTemplate, false);

// Save should throw an exception
try {
session.save();
fail("Should have thrown a RepositoryException!");
} catch (RepositoryException ex) {
assertEquals("The thrown cause class should be the same", NumberFormatException.class, ex.getCause().getClass());
}

// Then it should stay at 53!
assertEquals("The size of the total node types is not correct", 53, nodeTypeManager.getAllNodeTypes().getSize());

It all goes fine, until the last line. This assertion fails. There are 54 node types instead of 53! I believe this behavoir is not intended.

When I try to call to fetch data, I get the following exception and this is the point where you see the data turned inconsistent.

This log appears:

java.lang.NumberFormatException: For input string: "fqdsf"
  at java.lang.NumberFormatException.forInputString(NumberFormatException.java:48)
  at java.lang.Long.parseLong(Long.java:410)
  at java.lang.Long.parseLong(Long.java:468)
  at org.modeshape.jcr.JcrPropertyDefinition$LongConstraintChecker.parseValue(JcrPropertyDefinition.java:846)
  at org.modeshape.jcr.JcrPropertyDefinition$LongConstraintChecker.parseValue(JcrPropertyDefinition.java:826)
  at org.modeshape.jcr.JcrPropertyDefinition$RangeConstraintChecker.parseValueConstraint(JcrPropertyDefinition.java:692)
  at org.modeshape.jcr.JcrPropertyDefinition$RangeConstraintChecker.<init>(JcrPropertyDefinition.java:620)
  at org.modeshape.jcr.JcrPropertyDefinition$LongConstraintChecker.<init>(JcrPropertyDefinition.java:831)
  at org.modeshape.jcr.JcrPropertyDefinition.createChecker(JcrPropertyDefinition.java:470)
  at org.modeshape.jcr.JcrPropertyDefinition.getMinimumValue(JcrPropertyDefinition.java:332)
  at org.modeshape.jcr.NodeTypeSchemata.addAllNodesTable(NodeTypeSchemata.java:230)
  at org.modeshape.jcr.NodeTypeSchemata.<init>(NodeTypeSchemata.java:138)
  at org.modeshape.jcr.RepositoryNodeTypeManager.getRepositorySchemata(RepositoryNodeTypeManager.java:293)
  at org.modeshape.jcr.JcrRepository$RepositoryMonitorFactory.indexingMonitor(JcrRepository.java:1917)
  at org.modeshape.jcr.JcrRepository$RepositoryMonitorFactory.createMonitor(JcrRepository.java:1912)
  at org.modeshape.jcr.txn.Transactions.newMonitor(Transactions.java:243)
  at org.modeshape.jcr.txn.SynchronizedTransactions$SynchronizedTransaction.<init>(SynchronizedTransactions.java:165)
  at org.modeshape.jcr.txn.SynchronizedTransactions.begin(SynchronizedTransactions.java:77)
  at org.modeshape.jcr.cache.RepositoryCache.runInTransaction(RepositoryCache.java:468)
  at org.modeshape.jcr.cache.RepositoryCache.workspace(RepositoryCache.java:836)
  at org.modeshape.jcr.cache.RepositoryCache.getWorkspaceCache(RepositoryCache.java:1047)
  at org.modeshape.jcr.JcrSession.<init>(JcrSession.java:180)
  at org.modeshape.jcr.JcrXaSession.<init>(JcrXaSession.java:49)
  at org.modeshape.jcr.JcrRepository.login(JcrRepository.java:693)
  at org.modeshape.jcr.JcrRepository.login(JcrRepository.java:155)
  at be.admb.jcr.modeshape.manager.jcr.impl.JcrSessionFactory.getSession(JcrSessionFactory.java:77)
  at be.admb.jcr.modeshape.manager.jcr.util.SessionFactoryUtils.doGetSession(SessionFactoryUtils.java:77)
  at be.admb.jcr.modeshape.manager.jcr.util.SessionFactoryUtils.getSession(SessionFactoryUtils.java:109)
  at be.admb.jcr.modeshape.manager.jcr.impl.JcrTemplate.execute(JcrTemplate.java:77)
  at be.admb.jcr.modeshape.manager.dao.impl.JcrDaoImpl.getCompleteNode(JcrDaoImpl.java:77)
  at be.admb.jcr.modeshape.manager.service.impl.JcrServiceImpl.getNode(JcrServiceImpl.java:67)

I suppose this is a bug. If wished I can report this to JIRA including an unit test.

Note: The server does start up, but I need to wipe out the MySQL data since it is completely unusable now. I cannot even unregister the nodetype, it throws the same exception.

[bjornbal]

Another thing where I think it seems incorrect:

- Add a node type with a property definition as BOOLEAN, which is allowed according to Defining custom node types - ModeShape 3 - Project Documentation Editor.

- Add a constraint f.e. 'true'.

- Make the property definition mandatory (I think this part is not neccesary, you can add it afterwards aswell and still get the same error in theory)

- Now add a node with this node type and fill in the property (f.e. true boolean)

- Exception occurs, see log:

java.lang.IllegalStateException: Invalid property type: 6
  at org.modeshape.jcr.JcrPropertyDefinition.createChecker(JcrPropertyDefinition.java:485)
  at org.modeshape.jcr.JcrPropertyDefinition.satisfiesConstraints(JcrPropertyDefinition.java:266)
  at org.modeshape.jcr.RepositoryNodeTypeManager$NodeTypes.findPropertyDefinition(RepositoryNodeTypeManager.java:1453)
  at org.modeshape.jcr.AbstractJcrNode.setProperty(AbstractJcrNode.java:1773)
  at org.modeshape.jcr.AbstractJcrNode.setProperty(AbstractJcrNode.java:1460)
  at org.modeshape.jcr.AbstractJcrNode.setProperty(AbstractJcrNode.java:108)

There is no BOOLEAN checker available I see. But it is allowed in the definition?

[rhauch]

Bjorn Bal wrote:

When I try to add a new nodetype with for example the type as LONG, I try to add another property definition with constraints. But now I add this constraint as a non-long type (f.e. a simple string like 'hello'). I expected to receive an exception.

I do, when I call session.save() after registerNodeType in the nodeTypeManager, still I am unsure why I need to explicitly call the session.save() to receive this exception since I see a save call in the registerNodeType method.

Calling 'save()' has nothing to do with registering node types. It sounds like ModeShape needs to do a bit more validation of the constraints at the time of registration, so please log an issue and add a test case that clearly demonstrates the problem.

BTW, you seem to be struggling with dynamically creating node types, so I just wanted to make sure that you know about CND files and the ability to define them in a convenient file form. I think that most people define their custom node types using CNDs.

[rhauch]

Yes, BOOLEAN is a valid property type. No checker is required/allowed because the values can only be 'true' or 'false' boolean values, and a constraint checker would add no value. However, clearly ModeShape should handle this better, either by allowing the constraint check to be defined (and possibly ignoring it silently) or by throwing a proper exception that such a constraint check is unnecessary.

Note that even a property defined by a property definition of type BOOLEAN can be set with a String or Long value, as long as that value is convertible to booleans per the JSR-333 spec. For example, the string "true" will convert to a boolean 'true', but a string "1" will not. And these conversions are done at the 'setProperty' methods, and ModeShape will actually store the boolean form.

[bjornbal]

Thanks for your time Randall. save() was indeed not neccesary. I've modified the unit test and just unregistering node type caused this exception aswell.

I have created two issues according those two problems.

Boolean problem ('true' constraint and 'true' property field even causes this):

[MODE-2150] Create node type with boolean type and constraints causes checker exception - JBoss Issue Tracker

Constraint checker problem:

[MODE-2149] Adding wrong constraint type causes data inconsistency - JBoss Issue Tracker

Oh, I completely agree to make more use of the CND import which seems much more flexible. But the problem is, there should be the possibly to just use a form to add this node type and thus we need to register those node types programatically aswell. However, we do offer both solutions.

[rhauch]

Thanks for logging those issues.

ModeShape should allow you to define and re-define node types programmatically. But if you do, make sure that you will not corrupt the data. JCR does not mandate and ModeShape does not validate any changed node type definitions against the actual content before registering the updated definition. This is extremely complicated to do automatically and requires more knowledge of the content than ModeShape can efficiently and quickly obtain. And many times, the content needs to be modified, which is something ModeShape cannot and will not do. So while this sounds like a major hindrance, in fact it actually gives you a lot more flexibility and control than you would if ModeShape simply prevented anything bad from happening. Some kinds of changes simply require you to work in multiple steps.

Adding property definitions and/or child node definitions is very straightforward, as long as they are not mandatory. If they are mandatory and you know you have existing content that uses the affected node type, you probably need to

1. initially make the new property and/or child node definitions not mandatory,
2. manually alter your content to add properties and/or children on all nodes where they will be required by the new definitions,
3. update the node type to make the property/child definitions mandatory.

Removing property definitions and/or child node definitions requires you validating that no content actually makes use of these definitions. Often queries will be sufficient, but sometimes you might need to use a combination of queries (to find the nodes of that node type) and code that does more complex checking. Obviously such content needs to be fixed before you can remove the definitions, so you might first need to:

1. update the node type to make the property/child definitions not-mandatory,
2. clean up any content that uses those definitions,
3. update the node type to remove the property/child definitions

Changing the type of a property definition requires a little more work. If the property values all happen to be convertible (using the standard JCR property type conversion rules) to the new property type, then you should be able to simply change the property definition and any existing property values will be auto-converted. For example, this might work if you have a LONG property that you want to change to a DOUBLE, or have a BOOLEAN property that you want to change to a STRING. However, most of the time you will have to manually adjust the content with code. If your node type does not use residual property definitions, then you can:

1. update the node type to add a temporary residual property definition (with acceptable constraints and/or types) and change the type of the existing non-residual property definition; any existing properties that used to match the now-changed property definition will no longer do so, and will instead match the residual property definition, and the nodes will remain valid
2. go through the content and programmatically convert the property values to the desired type; at this point, the property will match the new property definition
3. remove the temporary residual property definition

There are other scenarios, too, though many are simple variations/alterations of the above processes. Basically, you need to always ensure that your existing content always remain valid, and sometimes that requires adding temporary property and/or child node definitions that will keep the node valid while you modify the content.

[bjornbal]

Thanks for your explanation, Randall. I am very aware about the fact that (currently?) any modification of the node type is the responsibility of the user to keep the content valid. This is exact the reason why we decided the user won't have the possibility to edit an existing node type... Not yet... We are not sure yet if this (by us or Modeshape) will be possible in the future, exact because of those reasons/possible problems you've just described.

Currently in our browser (JCR manager) you only have the possibility to remove and add a new node type. You cannot remove a node type while it is in use by a node. This is currently quite a safe solution for us to ensure the data will remain valid.

However those issues only occured while or after defining them, without any modification.

I've got one additional question. If the data got corrupted, currently there is no other way to wipe out the tables or restore the backup? Are there other solutions, f.e. fixing data by editing rows? We tried that latter, but it had no effect.

[rhauch]

I've got one additional question. If the data got corrupted, currently there is no other way to wipe out the tables or restore the backup? Are there other solutions, f.e. fixing data by editing rows? We tried that latter, but it had no effect.

Actually, you can use ModeShape's backup facility to recover completely. However, ModeShape probably could add a special kind of "recovery" user or session that does not perform the validation upon load; that might allow an application to fix the data. The problem with this is that it would be very dangerous to use except in this special situation. So we'd have to be comfortable that any such solution would be relatively difficult to use and impossible to "accidentally" use. Feel free to log a feature request in our JIRA about it (be sure to thoroughly describe the feature and why it's needed), but I can't promise when we might incorporate such a feature.