[ https://jira.jboss.org/jira/browse/DNA-546?page=com.atlassian.jira.plugin... ] Brian Carothers commented on DNA-546: ------------------------------------- The root cause of this behavior is the Basic model's composite primary key for node entities (workspace ID, node UUID). This approach is very efficient in general, but causes problems with the portions of the TCK that require replacement of nodes with the same UUIDs (e.g., document imports that contain nodes with a UUID that is already in use with the workspace). In this case, the default implementation in the JCR layer tries to delete the existing node with a given UUID and then add a new node (usually in a different location) with the same UUID. This would require the JPA session to contain two versions of the same entity (one being deleted and another being added). This just doesn't work under the JPA spec. So then the question becomes "how does this get fixed?" One approach would be to modify the JCR import (and workspace copy and clone) code to implement an update of this type instead of a delete/insert strategy. This approach would require a two-pass approach. First, the UUIDs of all of the incoming nodes would have to be collected, then the potential nodes that might be deleted would have to be computed. This set of added nodes would be a proper subset of the deleted nodes, since deleting a node implies deleting all of its ancestors as well. After that, the nodes that would have been deleted and then inserted would have to be moved to their new location and their children removed, but this would have to be ordered carefully since the removed children may themselves be moved as part of the operation. That's not everything that would need to happen to work around this, but it's already a lot. And only one current (or planned) connector has this problem, so it's pretty debatable whether it should have been solved in the JCR layer anyway. Alternatively, we could just add a new model for the JPA connector which was built to support this in the first place. I'm going to try that first. My proposed model is based on the current model (the "Basic" model) and in fact would reuse a fair amount of the common code. The new model would have three main changes from the Basic model: 1. It would have an artificial key to avoid the problem described in this defect. 2. It would store the node and its properties in a single table (collapsing the ChildEntity and PropertiesEntity from the Basic model into a single NodeEntity) 3. It would be based on the MapRepository so that it can reuse most of the working, debugged MapRequestProcessor implementation. I expect to retain the mechanism for managing large properties and tracking references from the BasicModel. I also am reusing the property serialization code since it's already been tested and shaken down. Any feedback on this approach is, as always, welcome. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-546?page=com.atlassian.jira.plugin... ] Brian Carothers updated DNA-546: -------------------------------- Attachment: (was: DNA-546_map_node_change.patch) -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-546?page=com.atlassian.jira.plugin... ] Brian Carothers updated DNA-546: -------------------------------- Attachment: DNA-546_jpa_connector_refactor.patch Attached patch (DNA-546_jpa_connector_refactor.patch) that refactors the existing JPA connector framework to more readily support the new model that I'm working on. The patch removes the relationship between the model and the request processor and adds a relationship between the model and its connection. To work within the MapRepository framework, I had to be able to pass a reference to the MapRepository into the MapRequestProcessor. This ruled out using the existing JpaConnection. The cleanest way to isolate the reusable pieces of the code seemed to be to rename JpaConnection to BasicJpaConnection, move it into model.basic, and then replace Model.createRequestProcessor(...) with Model.createConnection(JpaSource). This required a few changes to class visibility (but not constructor visibility) and the addition of some accessors to JpaSource. Clearly, there are alternate ways to accomplish this same goal. I am going to wait for some feedback before I commit this patch. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-546?page=com.atlassian.jira.plugin... ] Brian Carothers updated DNA-546: -------------------------------- Attachment: DNA-546_simple_model_WIP.patch Attaching a snapshot of the work in progress in case anyone's curious. This version mostly works, although it doesn't respect References in copy or clone operations and won't enforce referential integrity. It also has some wonky behavior around committing changes that I can't seem to figure. This causes race conditions that lead to failures when running the L2 TCK tests. Having given that disclaimer, the structure of the code is just about where I think it should be. It heavily leverages the MapRepository framework and the existing Basic model. It does differ from the other MapRepository implementations in that it creates a new MapRepository instance with every JpaSource.getConnection call - the other implementations share the same MapRepository instance across multiple connections. Feedback is welcome as always. My intent is to take yet another look at the race condition on the commits and then work on integrating references. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-546?page=com.atlassian.jira.plugin... ] Brian Carothers updated DNA-546: -------------------------------- Attachment: DNA-546_tck_test.patch Attached patch that adds TCK integration tests for new JPA connector model. I am not committing this patch at this time because these tests pass consistently and in their entirety if and only if JcrRepository.WORKSPACES_SHARE_SYSTEM_BRANCH is set to false. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-546?page=com.atlassian.jira.plugin... ] Brian Carothers updated DNA-546: -------------------------------- Attachment: DNA-546_final.patch Attached patch that cleans up the simple JPA model by fixing the last broken test, removing semi-implemented referential integrity checks, adding copyright headers to a number of files, and adding a full TCK integration test. At this point, the simple model passes all of the TCK tests (including the workspace and session import tests) and the recommended fix/workaround for users who need to import things into a JPA-backed repository is to use the simple model instead of the basic model. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira