You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2021/08/23 15:37:17 UTC
[GitHub] [jackrabbit-filevault] kwin opened a new pull request #159: JCRVLT-551 try to set UUID from package
kwin opened a new pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159
add additional import options to tweak UUID collision behavior
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] kwin commented on a change in pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#discussion_r694566705
##########
File path: vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
##########
@@ -809,89 +813,47 @@ private void handleAuthorizable(Node node, DocViewNode ni) throws RepositoryExce
private StackElement addNode(DocViewNode ni) throws RepositoryException, IOException {
final Node currentNode = stack.getNode();
- // find old node
- Node oldNode = null;
- Node node = null;
+ Node existingNode = null;
if ("".equals(ni.label)) {
// special case for root node update
- node = currentNode;
- } else if (ni.uuid == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
+ existingNode = currentNode;
} else {
- try {
- node = session.getNodeByUUID(ni.uuid);
- if (!node.getParent().isSame(currentNode)) {
- log.warn("Packaged node at {} is referenceable and collides with existing node at {}. Will create new UUID.",
- currentNode.getPath() + "/" + ni.label,
- node.getPath());
- ni.uuid = null;
- ni.props.remove(JcrConstants.JCR_UUID);
- ni.props.remove(JcrConstants.JCR_BASEVERSION);
- ni.props.remove(JcrConstants.JCR_PREDECESSORS);
- ni.props.remove(JcrConstants.JCR_SUCCESSORS);
- ni.props.remove(JcrConstants.JCR_VERSIONHISTORY);
- node = null;
- }
- } catch (ItemNotFoundException e) {
- // ignore
- }
- if (node == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
- } else {
- if (!node.getName().equals(ni.name)) {
- // if names mismatches => replace
- oldNode = node;
- node = null;
- }
-
+ if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
+ existingNode = currentNode.getNode(ni.label);
}
- }
- // if old node is not included in the package, ignore rewrite
- if (oldNode != null && !isIncluded(oldNode, oldNode.getDepth() - rootDepth)) {
- node = oldNode;
- oldNode = null;
- }
-
- // TODO: under which condition do we end up here?
- if (oldNode != null) {
- // check versionable
- new VersioningState(stack, oldNode).ensureCheckedOut();
-
- String oldNodePath = oldNode.getPath();
- ImportMode importMode = wspFilter.getImportMode(oldNodePath);
- NodeStash recovery = new NodeStash(session, oldNodePath, importMode);
- recovery.stash();
-
- // ensure that existing binaries are not sourced from a property
- // that is about to be removed
- Map<String, DocViewSAXImporter.BlobInfo> blobs = binaries.get(oldNode.getPath());
- if (blobs != null) {
- for (DocViewSAXImporter.BlobInfo info : blobs.values()) {
- info.detach();
+ if (ni.uuid != null) {
+ if (idConflictPolicy == IdConflictPolicy.FAIL) {
+ try {
+ // does uuid already exist in the repo?
+ Node sameIdNode = session.getNodeByIdentifier(ni.uuid);
+ // edge-case: same node path -> uuid is kept
+ if (existingNode != null && existingNode.getPath().equals(sameIdNode.getPath())) {
+ log.debug("Node with existing identifier is being updated without modifying its uuid");
+ } else {
+ // uuid found in path covered by filter
+ if (isIncluded(sameIdNode, 0)) {
+ log.warn("Node identifier {} for to-be imported node {} already taken by {}, trying to release it.", ni.uuid, currentNode.getPath() + '/' + ni.label, sameIdNode.getPath());
+ removeReferences(sameIdNode);
Review comment:
The moved node may be referenced via id or by path. If the move operation is just silently reverted through package install the path references are broken!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] tripodsan commented on a change in pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
tripodsan commented on a change in pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#discussion_r694535900
##########
File path: vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
##########
@@ -809,89 +813,47 @@ private void handleAuthorizable(Node node, DocViewNode ni) throws RepositoryExce
private StackElement addNode(DocViewNode ni) throws RepositoryException, IOException {
final Node currentNode = stack.getNode();
- // find old node
- Node oldNode = null;
- Node node = null;
+ Node existingNode = null;
if ("".equals(ni.label)) {
// special case for root node update
- node = currentNode;
- } else if (ni.uuid == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
+ existingNode = currentNode;
} else {
- try {
- node = session.getNodeByUUID(ni.uuid);
- if (!node.getParent().isSame(currentNode)) {
- log.warn("Packaged node at {} is referenceable and collides with existing node at {}. Will create new UUID.",
- currentNode.getPath() + "/" + ni.label,
- node.getPath());
- ni.uuid = null;
- ni.props.remove(JcrConstants.JCR_UUID);
- ni.props.remove(JcrConstants.JCR_BASEVERSION);
- ni.props.remove(JcrConstants.JCR_PREDECESSORS);
- ni.props.remove(JcrConstants.JCR_SUCCESSORS);
- ni.props.remove(JcrConstants.JCR_VERSIONHISTORY);
- node = null;
- }
- } catch (ItemNotFoundException e) {
- // ignore
- }
- if (node == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
- } else {
- if (!node.getName().equals(ni.name)) {
- // if names mismatches => replace
- oldNode = node;
- node = null;
- }
-
+ if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
+ existingNode = currentNode.getNode(ni.label);
}
- }
- // if old node is not included in the package, ignore rewrite
- if (oldNode != null && !isIncluded(oldNode, oldNode.getDepth() - rootDepth)) {
- node = oldNode;
- oldNode = null;
- }
-
- // TODO: under which condition do we end up here?
- if (oldNode != null) {
- // check versionable
- new VersioningState(stack, oldNode).ensureCheckedOut();
-
- String oldNodePath = oldNode.getPath();
- ImportMode importMode = wspFilter.getImportMode(oldNodePath);
- NodeStash recovery = new NodeStash(session, oldNodePath, importMode);
- recovery.stash();
-
- // ensure that existing binaries are not sourced from a property
- // that is about to be removed
- Map<String, DocViewSAXImporter.BlobInfo> blobs = binaries.get(oldNode.getPath());
- if (blobs != null) {
- for (DocViewSAXImporter.BlobInfo info : blobs.values()) {
- info.detach();
+ if (ni.uuid != null) {
+ if (idConflictPolicy == IdConflictPolicy.FAIL) {
+ try {
+ // does uuid already exist in the repo?
+ Node sameIdNode = session.getNodeByIdentifier(ni.uuid);
+ // edge-case: same node path -> uuid is kept
+ if (existingNode != null && existingNode.getPath().equals(sameIdNode.getPath())) {
+ log.debug("Node with existing identifier is being updated without modifying its uuid");
+ } else {
+ // uuid found in path covered by filter
+ if (isIncluded(sameIdNode, 0)) {
+ log.warn("Node identifier {} for to-be imported node {} already taken by {}, trying to release it.", ni.uuid, currentNode.getPath() + '/' + ni.label, sameIdNode.getPath());
+ removeReferences(sameIdNode);
Review comment:
why removing the references? if the existing node is at the wrong place, it should be moved but the references should remain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] kwin commented on a change in pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#discussion_r694557448
##########
File path: vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
##########
@@ -809,89 +813,47 @@ private void handleAuthorizable(Node node, DocViewNode ni) throws RepositoryExce
private StackElement addNode(DocViewNode ni) throws RepositoryException, IOException {
final Node currentNode = stack.getNode();
- // find old node
- Node oldNode = null;
- Node node = null;
+ Node existingNode = null;
if ("".equals(ni.label)) {
// special case for root node update
- node = currentNode;
- } else if (ni.uuid == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
+ existingNode = currentNode;
} else {
- try {
- node = session.getNodeByUUID(ni.uuid);
- if (!node.getParent().isSame(currentNode)) {
- log.warn("Packaged node at {} is referenceable and collides with existing node at {}. Will create new UUID.",
- currentNode.getPath() + "/" + ni.label,
- node.getPath());
- ni.uuid = null;
- ni.props.remove(JcrConstants.JCR_UUID);
- ni.props.remove(JcrConstants.JCR_BASEVERSION);
- ni.props.remove(JcrConstants.JCR_PREDECESSORS);
- ni.props.remove(JcrConstants.JCR_SUCCESSORS);
- ni.props.remove(JcrConstants.JCR_VERSIONHISTORY);
- node = null;
- }
- } catch (ItemNotFoundException e) {
- // ignore
- }
- if (node == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
- } else {
- if (!node.getName().equals(ni.name)) {
- // if names mismatches => replace
- oldNode = node;
- node = null;
- }
-
+ if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
+ existingNode = currentNode.getNode(ni.label);
}
- }
- // if old node is not included in the package, ignore rewrite
- if (oldNode != null && !isIncluded(oldNode, oldNode.getDepth() - rootDepth)) {
- node = oldNode;
- oldNode = null;
- }
-
- // TODO: under which condition do we end up here?
- if (oldNode != null) {
- // check versionable
- new VersioningState(stack, oldNode).ensureCheckedOut();
-
- String oldNodePath = oldNode.getPath();
- ImportMode importMode = wspFilter.getImportMode(oldNodePath);
- NodeStash recovery = new NodeStash(session, oldNodePath, importMode);
- recovery.stash();
-
- // ensure that existing binaries are not sourced from a property
- // that is about to be removed
- Map<String, DocViewSAXImporter.BlobInfo> blobs = binaries.get(oldNode.getPath());
- if (blobs != null) {
- for (DocViewSAXImporter.BlobInfo info : blobs.values()) {
- info.detach();
+ if (ni.uuid != null) {
+ if (idConflictPolicy == IdConflictPolicy.FAIL) {
+ try {
+ // does uuid already exist in the repo?
+ Node sameIdNode = session.getNodeByIdentifier(ni.uuid);
+ // edge-case: same node path -> uuid is kept
+ if (existingNode != null && existingNode.getPath().equals(sameIdNode.getPath())) {
+ log.debug("Node with existing identifier is being updated without modifying its uuid");
+ } else {
+ // uuid found in path covered by filter
+ if (isIncluded(sameIdNode, 0)) {
+ log.warn("Node identifier {} for to-be imported node {} already taken by {}, trying to release it.", ni.uuid, currentNode.getPath() + '/' + ni.label, sameIdNode.getPath());
+ removeReferences(sameIdNode);
Review comment:
IMHO if a node with a conflicting id already exists in the repo at a different path, one should either
- not touch it (if not contained in filter rules)
- remove it and its references (if contained in the filter rules)
-
Just assuming that this node should be moved because its id is the same is very dangerous as conflicts may occur for totally unrelated nodes (although unlikely)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] sonarcloud[bot] commented on pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#issuecomment-904566453
Kudos, SonarCloud Quality Gate passed! 
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [17 Code Smells](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] sonarcloud[bot] removed a comment on pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#issuecomment-904566453
Kudos, SonarCloud Quality Gate passed! 
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [17 Code Smells](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] kwin commented on a change in pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#discussion_r694567558
##########
File path: vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
##########
@@ -809,89 +813,47 @@ private void handleAuthorizable(Node node, DocViewNode ni) throws RepositoryExce
private StackElement addNode(DocViewNode ni) throws RepositoryException, IOException {
final Node currentNode = stack.getNode();
- // find old node
- Node oldNode = null;
- Node node = null;
+ Node existingNode = null;
if ("".equals(ni.label)) {
// special case for root node update
- node = currentNode;
- } else if (ni.uuid == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
+ existingNode = currentNode;
} else {
- try {
- node = session.getNodeByUUID(ni.uuid);
- if (!node.getParent().isSame(currentNode)) {
- log.warn("Packaged node at {} is referenceable and collides with existing node at {}. Will create new UUID.",
- currentNode.getPath() + "/" + ni.label,
- node.getPath());
- ni.uuid = null;
- ni.props.remove(JcrConstants.JCR_UUID);
- ni.props.remove(JcrConstants.JCR_BASEVERSION);
- ni.props.remove(JcrConstants.JCR_PREDECESSORS);
- ni.props.remove(JcrConstants.JCR_SUCCESSORS);
- ni.props.remove(JcrConstants.JCR_VERSIONHISTORY);
- node = null;
- }
- } catch (ItemNotFoundException e) {
- // ignore
- }
- if (node == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
- } else {
- if (!node.getName().equals(ni.name)) {
- // if names mismatches => replace
- oldNode = node;
- node = null;
- }
-
+ if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
+ existingNode = currentNode.getNode(ni.label);
}
- }
- // if old node is not included in the package, ignore rewrite
- if (oldNode != null && !isIncluded(oldNode, oldNode.getDepth() - rootDepth)) {
- node = oldNode;
- oldNode = null;
- }
-
- // TODO: under which condition do we end up here?
- if (oldNode != null) {
- // check versionable
- new VersioningState(stack, oldNode).ensureCheckedOut();
-
- String oldNodePath = oldNode.getPath();
- ImportMode importMode = wspFilter.getImportMode(oldNodePath);
- NodeStash recovery = new NodeStash(session, oldNodePath, importMode);
- recovery.stash();
-
- // ensure that existing binaries are not sourced from a property
- // that is about to be removed
- Map<String, DocViewSAXImporter.BlobInfo> blobs = binaries.get(oldNode.getPath());
- if (blobs != null) {
- for (DocViewSAXImporter.BlobInfo info : blobs.values()) {
- info.detach();
+ if (ni.uuid != null) {
+ if (idConflictPolicy == IdConflictPolicy.FAIL) {
+ try {
+ // does uuid already exist in the repo?
+ Node sameIdNode = session.getNodeByIdentifier(ni.uuid);
+ // edge-case: same node path -> uuid is kept
+ if (existingNode != null && existingNode.getPath().equals(sameIdNode.getPath())) {
+ log.debug("Node with existing identifier is being updated without modifying its uuid");
+ } else {
+ // uuid found in path covered by filter
+ if (isIncluded(sameIdNode, 0)) {
+ log.warn("Node identifier {} for to-be imported node {} already taken by {}, trying to release it.", ni.uuid, currentNode.getPath() + '/' + ni.label, sameIdNode.getPath());
+ removeReferences(sameIdNode);
Review comment:
In any case references are only removed if they are contained in the filter rules, so this does not destroy anything which would not be overwritten anyhow later in the package installation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] kwin commented on a change in pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#discussion_r694557448
##########
File path: vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
##########
@@ -809,89 +813,47 @@ private void handleAuthorizable(Node node, DocViewNode ni) throws RepositoryExce
private StackElement addNode(DocViewNode ni) throws RepositoryException, IOException {
final Node currentNode = stack.getNode();
- // find old node
- Node oldNode = null;
- Node node = null;
+ Node existingNode = null;
if ("".equals(ni.label)) {
// special case for root node update
- node = currentNode;
- } else if (ni.uuid == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
+ existingNode = currentNode;
} else {
- try {
- node = session.getNodeByUUID(ni.uuid);
- if (!node.getParent().isSame(currentNode)) {
- log.warn("Packaged node at {} is referenceable and collides with existing node at {}. Will create new UUID.",
- currentNode.getPath() + "/" + ni.label,
- node.getPath());
- ni.uuid = null;
- ni.props.remove(JcrConstants.JCR_UUID);
- ni.props.remove(JcrConstants.JCR_BASEVERSION);
- ni.props.remove(JcrConstants.JCR_PREDECESSORS);
- ni.props.remove(JcrConstants.JCR_SUCCESSORS);
- ni.props.remove(JcrConstants.JCR_VERSIONHISTORY);
- node = null;
- }
- } catch (ItemNotFoundException e) {
- // ignore
- }
- if (node == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
- } else {
- if (!node.getName().equals(ni.name)) {
- // if names mismatches => replace
- oldNode = node;
- node = null;
- }
-
+ if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
+ existingNode = currentNode.getNode(ni.label);
}
- }
- // if old node is not included in the package, ignore rewrite
- if (oldNode != null && !isIncluded(oldNode, oldNode.getDepth() - rootDepth)) {
- node = oldNode;
- oldNode = null;
- }
-
- // TODO: under which condition do we end up here?
- if (oldNode != null) {
- // check versionable
- new VersioningState(stack, oldNode).ensureCheckedOut();
-
- String oldNodePath = oldNode.getPath();
- ImportMode importMode = wspFilter.getImportMode(oldNodePath);
- NodeStash recovery = new NodeStash(session, oldNodePath, importMode);
- recovery.stash();
-
- // ensure that existing binaries are not sourced from a property
- // that is about to be removed
- Map<String, DocViewSAXImporter.BlobInfo> blobs = binaries.get(oldNode.getPath());
- if (blobs != null) {
- for (DocViewSAXImporter.BlobInfo info : blobs.values()) {
- info.detach();
+ if (ni.uuid != null) {
+ if (idConflictPolicy == IdConflictPolicy.FAIL) {
+ try {
+ // does uuid already exist in the repo?
+ Node sameIdNode = session.getNodeByIdentifier(ni.uuid);
+ // edge-case: same node path -> uuid is kept
+ if (existingNode != null && existingNode.getPath().equals(sameIdNode.getPath())) {
+ log.debug("Node with existing identifier is being updated without modifying its uuid");
+ } else {
+ // uuid found in path covered by filter
+ if (isIncluded(sameIdNode, 0)) {
+ log.warn("Node identifier {} for to-be imported node {} already taken by {}, trying to release it.", ni.uuid, currentNode.getPath() + '/' + ni.label, sameIdNode.getPath());
+ removeReferences(sameIdNode);
Review comment:
IMHO if a node with a conflicting id already exists in the repo at a different path, one should either
- not touch it (if not contained in filter rules)
- remove it and its references (if contained in the filter rules)
Just assuming that this node should be moved because its id is the same is very dangerous as conflicts may occur for totally unrelated nodes (although unlikely)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] sonarcloud[bot] commented on pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#issuecomment-904745258
Kudos, SonarCloud Quality Gate passed! 
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list) [89.3% Coverage](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] sonarcloud[bot] removed a comment on pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#issuecomment-904410481
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [17 Code Smells](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] kwin commented on pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
kwin commented on pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#issuecomment-904384391
The default behaviour changed: https://github.com/apache/jackrabbit-filevault/pull/159/files#diff-805491a0bcebe271c6e12fa9b2a97e452effc7963d619203e2e2cc616d954639.
But IMHO in a way which is safer than before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] kwin merged pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
kwin merged pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] tripodsan commented on a change in pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
tripodsan commented on a change in pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#discussion_r694565566
##########
File path: vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
##########
@@ -809,89 +813,47 @@ private void handleAuthorizable(Node node, DocViewNode ni) throws RepositoryExce
private StackElement addNode(DocViewNode ni) throws RepositoryException, IOException {
final Node currentNode = stack.getNode();
- // find old node
- Node oldNode = null;
- Node node = null;
+ Node existingNode = null;
if ("".equals(ni.label)) {
// special case for root node update
- node = currentNode;
- } else if (ni.uuid == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
+ existingNode = currentNode;
} else {
- try {
- node = session.getNodeByUUID(ni.uuid);
- if (!node.getParent().isSame(currentNode)) {
- log.warn("Packaged node at {} is referenceable and collides with existing node at {}. Will create new UUID.",
- currentNode.getPath() + "/" + ni.label,
- node.getPath());
- ni.uuid = null;
- ni.props.remove(JcrConstants.JCR_UUID);
- ni.props.remove(JcrConstants.JCR_BASEVERSION);
- ni.props.remove(JcrConstants.JCR_PREDECESSORS);
- ni.props.remove(JcrConstants.JCR_SUCCESSORS);
- ni.props.remove(JcrConstants.JCR_VERSIONHISTORY);
- node = null;
- }
- } catch (ItemNotFoundException e) {
- // ignore
- }
- if (node == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
- } else {
- if (!node.getName().equals(ni.name)) {
- // if names mismatches => replace
- oldNode = node;
- node = null;
- }
-
+ if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
+ existingNode = currentNode.getNode(ni.label);
}
- }
- // if old node is not included in the package, ignore rewrite
- if (oldNode != null && !isIncluded(oldNode, oldNode.getDepth() - rootDepth)) {
- node = oldNode;
- oldNode = null;
- }
-
- // TODO: under which condition do we end up here?
- if (oldNode != null) {
- // check versionable
- new VersioningState(stack, oldNode).ensureCheckedOut();
-
- String oldNodePath = oldNode.getPath();
- ImportMode importMode = wspFilter.getImportMode(oldNodePath);
- NodeStash recovery = new NodeStash(session, oldNodePath, importMode);
- recovery.stash();
-
- // ensure that existing binaries are not sourced from a property
- // that is about to be removed
- Map<String, DocViewSAXImporter.BlobInfo> blobs = binaries.get(oldNode.getPath());
- if (blobs != null) {
- for (DocViewSAXImporter.BlobInfo info : blobs.values()) {
- info.detach();
+ if (ni.uuid != null) {
+ if (idConflictPolicy == IdConflictPolicy.FAIL) {
+ try {
+ // does uuid already exist in the repo?
+ Node sameIdNode = session.getNodeByIdentifier(ni.uuid);
+ // edge-case: same node path -> uuid is kept
+ if (existingNode != null && existingNode.getPath().equals(sameIdNode.getPath())) {
+ log.debug("Node with existing identifier is being updated without modifying its uuid");
+ } else {
+ // uuid found in path covered by filter
+ if (isIncluded(sameIdNode, 0)) {
+ log.warn("Node identifier {} for to-be imported node {} already taken by {}, trying to release it.", ni.uuid, currentNode.getPath() + '/' + ni.label, sameIdNode.getPath());
+ removeReferences(sameIdNode);
Review comment:
the most common use case, is probably a rename/move of a referenceable node due to restructuring of a folder hierarchy. removing and recreating the node at the new place should be avoided. also, the references to it should remain. this is the idea of MOVE_EXISTING.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] sonarcloud[bot] commented on pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#issuecomment-904410481
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [17 Code Smells](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] sonarcloud[bot] commented on pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#issuecomment-904425918
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [17 Code Smells](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] kwin commented on a change in pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#discussion_r694557448
##########
File path: vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
##########
@@ -809,89 +813,47 @@ private void handleAuthorizable(Node node, DocViewNode ni) throws RepositoryExce
private StackElement addNode(DocViewNode ni) throws RepositoryException, IOException {
final Node currentNode = stack.getNode();
- // find old node
- Node oldNode = null;
- Node node = null;
+ Node existingNode = null;
if ("".equals(ni.label)) {
// special case for root node update
- node = currentNode;
- } else if (ni.uuid == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
+ existingNode = currentNode;
} else {
- try {
- node = session.getNodeByUUID(ni.uuid);
- if (!node.getParent().isSame(currentNode)) {
- log.warn("Packaged node at {} is referenceable and collides with existing node at {}. Will create new UUID.",
- currentNode.getPath() + "/" + ni.label,
- node.getPath());
- ni.uuid = null;
- ni.props.remove(JcrConstants.JCR_UUID);
- ni.props.remove(JcrConstants.JCR_BASEVERSION);
- ni.props.remove(JcrConstants.JCR_PREDECESSORS);
- ni.props.remove(JcrConstants.JCR_SUCCESSORS);
- ni.props.remove(JcrConstants.JCR_VERSIONHISTORY);
- node = null;
- }
- } catch (ItemNotFoundException e) {
- // ignore
- }
- if (node == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
- } else {
- if (!node.getName().equals(ni.name)) {
- // if names mismatches => replace
- oldNode = node;
- node = null;
- }
-
+ if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
+ existingNode = currentNode.getNode(ni.label);
}
- }
- // if old node is not included in the package, ignore rewrite
- if (oldNode != null && !isIncluded(oldNode, oldNode.getDepth() - rootDepth)) {
- node = oldNode;
- oldNode = null;
- }
-
- // TODO: under which condition do we end up here?
- if (oldNode != null) {
- // check versionable
- new VersioningState(stack, oldNode).ensureCheckedOut();
-
- String oldNodePath = oldNode.getPath();
- ImportMode importMode = wspFilter.getImportMode(oldNodePath);
- NodeStash recovery = new NodeStash(session, oldNodePath, importMode);
- recovery.stash();
-
- // ensure that existing binaries are not sourced from a property
- // that is about to be removed
- Map<String, DocViewSAXImporter.BlobInfo> blobs = binaries.get(oldNode.getPath());
- if (blobs != null) {
- for (DocViewSAXImporter.BlobInfo info : blobs.values()) {
- info.detach();
+ if (ni.uuid != null) {
+ if (idConflictPolicy == IdConflictPolicy.FAIL) {
+ try {
+ // does uuid already exist in the repo?
+ Node sameIdNode = session.getNodeByIdentifier(ni.uuid);
+ // edge-case: same node path -> uuid is kept
+ if (existingNode != null && existingNode.getPath().equals(sameIdNode.getPath())) {
+ log.debug("Node with existing identifier is being updated without modifying its uuid");
+ } else {
+ // uuid found in path covered by filter
+ if (isIncluded(sameIdNode, 0)) {
+ log.warn("Node identifier {} for to-be imported node {} already taken by {}, trying to release it.", ni.uuid, currentNode.getPath() + '/' + ni.label, sameIdNode.getPath());
+ removeReferences(sameIdNode);
Review comment:
IMHO if a node with a conflicting id already exists in the repo at a different path, one should either
- not touch it (if not contained in filter rules)
- remove it and its references (if contained in the filter rules)
Just assuming that this node should be moved because its id is the same is very dangerous as conflicts may occur for totally unrelated nodes (although unlikely)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] sonarcloud[bot] removed a comment on pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#issuecomment-904425918
SonarCloud Quality Gate failed. 
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL) [17 Code Smells](https://sonarcloud.io/project/issues?id=apache_jackrabbit-filevault&pullRequest=159&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_jackrabbit-filevault&pullRequest=159&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-filevault] kwin commented on a change in pull request #159: JCRVLT-551 try to set UUID from package
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #159:
URL: https://github.com/apache/jackrabbit-filevault/pull/159#discussion_r694566705
##########
File path: vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXImporter.java
##########
@@ -809,89 +813,47 @@ private void handleAuthorizable(Node node, DocViewNode ni) throws RepositoryExce
private StackElement addNode(DocViewNode ni) throws RepositoryException, IOException {
final Node currentNode = stack.getNode();
- // find old node
- Node oldNode = null;
- Node node = null;
+ Node existingNode = null;
if ("".equals(ni.label)) {
// special case for root node update
- node = currentNode;
- } else if (ni.uuid == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
+ existingNode = currentNode;
} else {
- try {
- node = session.getNodeByUUID(ni.uuid);
- if (!node.getParent().isSame(currentNode)) {
- log.warn("Packaged node at {} is referenceable and collides with existing node at {}. Will create new UUID.",
- currentNode.getPath() + "/" + ni.label,
- node.getPath());
- ni.uuid = null;
- ni.props.remove(JcrConstants.JCR_UUID);
- ni.props.remove(JcrConstants.JCR_BASEVERSION);
- ni.props.remove(JcrConstants.JCR_PREDECESSORS);
- ni.props.remove(JcrConstants.JCR_SUCCESSORS);
- ni.props.remove(JcrConstants.JCR_VERSIONHISTORY);
- node = null;
- }
- } catch (ItemNotFoundException e) {
- // ignore
- }
- if (node == null) {
- if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
- node = currentNode.getNode(ni.label);
- }
- } else {
- if (!node.getName().equals(ni.name)) {
- // if names mismatches => replace
- oldNode = node;
- node = null;
- }
-
+ if (stack.checkForNode() && currentNode.hasNode(ni.label)) {
+ existingNode = currentNode.getNode(ni.label);
}
- }
- // if old node is not included in the package, ignore rewrite
- if (oldNode != null && !isIncluded(oldNode, oldNode.getDepth() - rootDepth)) {
- node = oldNode;
- oldNode = null;
- }
-
- // TODO: under which condition do we end up here?
- if (oldNode != null) {
- // check versionable
- new VersioningState(stack, oldNode).ensureCheckedOut();
-
- String oldNodePath = oldNode.getPath();
- ImportMode importMode = wspFilter.getImportMode(oldNodePath);
- NodeStash recovery = new NodeStash(session, oldNodePath, importMode);
- recovery.stash();
-
- // ensure that existing binaries are not sourced from a property
- // that is about to be removed
- Map<String, DocViewSAXImporter.BlobInfo> blobs = binaries.get(oldNode.getPath());
- if (blobs != null) {
- for (DocViewSAXImporter.BlobInfo info : blobs.values()) {
- info.detach();
+ if (ni.uuid != null) {
+ if (idConflictPolicy == IdConflictPolicy.FAIL) {
+ try {
+ // does uuid already exist in the repo?
+ Node sameIdNode = session.getNodeByIdentifier(ni.uuid);
+ // edge-case: same node path -> uuid is kept
+ if (existingNode != null && existingNode.getPath().equals(sameIdNode.getPath())) {
+ log.debug("Node with existing identifier is being updated without modifying its uuid");
+ } else {
+ // uuid found in path covered by filter
+ if (isIncluded(sameIdNode, 0)) {
+ log.warn("Node identifier {} for to-be imported node {} already taken by {}, trying to release it.", ni.uuid, currentNode.getPath() + '/' + ni.label, sameIdNode.getPath());
+ removeReferences(sameIdNode);
Review comment:
The moved node may be referenced via id and by path at the same time. If the move operation is just silently reverted through package install the path references are broken!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org