[ 
https://issues.apache.org/jira/browse/JCRVLT-767?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Julian Reschke updated JCRVLT-767:
----------------------------------
    Description: 
In 
https://github.com/apache/jackrabbit-filevault/blob/931ceef98513167af3218b773d9213e123a2f52d/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java#L993-L1007

{noformat}
        if (identifier.isPresent() && 
!node.getIdentifier().equals(identifier.get()) && 
!"rep:root".equals(ni.getPrimaryType().orElse(""))) {
            long startTime = System.currentTimeMillis();
            String previousIdentifier = node.getIdentifier();
            log.debug("Node stashing for {} starting, existing identifier: {}, 
new identifier: {}, import mode: {}",
                    node.getPath(), previousIdentifier, identifier.get(), 
importMode);
{noformat}

However, Node.getIdentifer() will always be non-null - even for nodes without 
mix:referenceable.

This means that we go into stashing although we don't have to.

Changing the first line to 

{noformat}
        if (identifier.isPresent() && !identifier.get().equals(node.getUUID()) 
&& !"rep:root".equals(ni.getPrimaryType().orElse(""))) {
{noformat}

and FV tests still pass.

  was:
In 
https://github.com/apache/jackrabbit-filevault/blob/931ceef98513167af3218b773d9213e123a2f52d/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java#L993-L1007

{noformat}
        if (identifier.isPresent() && 
!node.getIdentifier().equals(identifier.get()) && 
!"rep:root".equals(ni.getPrimaryType().orElse(""))) {
            long startTime = System.currentTimeMillis();
            String previousIdentifier = node.getIdentifier();
            log.debug("Node stashing for {} starting, existing identifier: {}, 
new identifier: {}, import mode: {}",
                    node.getPath(), previousIdentifier, identifier.get(), 
importMode);
{noformat}

However, Node.getIdentifer() will always be non-null - even for nodes without 
mix:referenceable.

This means that we go into stashing although we don't have to.

Changing the first line to 

{noformat}
        if (identifier.isPresent() && 
!identifier.get().equals(node.getIdentifier()) && 
!"rep:root".equals(ni.getPrimaryType().orElse(""))) {
{noformat}

and FV tests still pass.


> vlt: potential incorrect identifier comparison
> ----------------------------------------------
>
>                 Key: JCRVLT-767
>                 URL: https://issues.apache.org/jira/browse/JCRVLT-767
>             Project: Jackrabbit FileVault
>          Issue Type: Bug
>          Components: vlt
>            Reporter: Julian Reschke
>            Priority: Minor
>
> In 
> https://github.com/apache/jackrabbit-filevault/blob/931ceef98513167af3218b773d9213e123a2f52d/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java#L993-L1007
> {noformat}
>         if (identifier.isPresent() && 
> !node.getIdentifier().equals(identifier.get()) && 
> !"rep:root".equals(ni.getPrimaryType().orElse(""))) {
>             long startTime = System.currentTimeMillis();
>             String previousIdentifier = node.getIdentifier();
>             log.debug("Node stashing for {} starting, existing identifier: 
> {}, new identifier: {}, import mode: {}",
>                     node.getPath(), previousIdentifier, identifier.get(), 
> importMode);
> {noformat}
> However, Node.getIdentifer() will always be non-null - even for nodes without 
> mix:referenceable.
> This means that we go into stashing although we don't have to.
> Changing the first line to 
> {noformat}
>         if (identifier.isPresent() && 
> !identifier.get().equals(node.getUUID()) && 
> !"rep:root".equals(ni.getPrimaryType().orElse(""))) {
> {noformat}
> and FV tests still pass.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to