Java SDK - bucket.lookupIn.exists bug?


#1

Hi,

I am encountering a bug in the Java SDK when trying to check if a subdocument exists. My code is the following (in Vert.x async and RXJava) :

asyncBucket
        .lookupIn(id)
        .exists(pathToCheck)
        .execute()
        .subscribe(
            lookupDocumentFragment -> handleSuccessResult(lookupDocumentFragment.exists(pathToCheck), asyncHandler),
            e -> handleFailureResult(e, asyncHandler)
        );

When I try this code with a path that doesn’t exist, I get this LookupDocumentFragment result :
EXIST(config.sink){value=false)
In this, config.sink is my path. It doesn’t exist, and, as we can see, the value is false. However, lookupDocumentFragment.exists(pathToCheck) returns “true” anyway.

I checked the SDK source code, and I think the bugs comes from DocumentFragment.java, more specifically the “exists” method which does this check :

if (path.equals(result.path()) && !(result.value() instanceof Exception)) {
            return true;
} 

In our case, path.equals(result.path()) is obviously true, but so is !(result.value() instanceof Exception)) as result.value() is a Boolean set to “false”. So exists returns true while it should return result.value().

Can somebody reproduce this behavior and confirm my analysis ?

Regargs,


#2

Hi @impolitePanda, you’re absolutely right, this is a bug and the cause is as you describe. I’ve logged issue https://issues.couchbase.com/browse/JCBC-1204 and will get this resolved. Apologies that you came across this issue, and thank you for submitting the code example and diagnosis.


#3

Hi @graham.pople! Thanks a lot for the news. For now we implemented our own fix (basically rewriting the exists method instead of calling it) but we would sure prefer to use the official one instead :slight_smile:


#4

Good to hear you have a workaround :slight_smile:


#5

Actually @impolitePanda, after further code analysis I believe I was a bit hasty in declaring this a bug. The code is actually working as intended. For an exists() subdoc operation, the client needs to check the content() of the DocumentFragment result - false will indicate that the path did not exist. DocumentFragment::exists() will return true to indicate that content() can be successfully called, e.g. it indicates that the result exists, rather than the path exists. It’s this way because DocumentFragment is also used for other subdoc operations, such as gets, and the exists() operation makes more sense in those contexts to indicate if the content is present.

Does that make sense?


#6

@graham.pople, I think I understand what you are saying but wouldn’t that mean that the “exists” method name is ambiguous ?
I mean, I used the method to check if a path exists in my doc and it returns true while the path didn’t exist which is, to me, counterintuitive with the “exists” name.
And, apparently, it’s used in a very different way within other methods of the SDK.

And if I understand your comment correctly, it would mean that my code would look more like this, then ?

lookupDocumentFragment.exists(pathToCheck) ? lookupDocumentFragment.content(pathToCheck, Boolean.class) : false

#7

@impolitePanda, yes that code is the correct way to check if the path exists.

On the subdoc API, exists() means “does this path exist”. But on the returned DocumentFragment, you may have grouped multiple subdoc operations together, and need to check which individual subdoc result exists(). And the content() contains the actual result of the subdoc operation, e.g. whether the path exists or not.

I agree with you that that there’s some ambiguity here, and in fact it caught me out too, hence the creation of a bug. Unfortunately we can’t alter it without it being a breaking change.


#8

@graham.pople, Ok, understood :slight_smile: Especially with the impossibility of creating a breaking change.
Could I perhaps ask you to transform the bug report into a documentation update task (javadoc and Sub-Document API) to maybe minimize this ambiguity ?

In any case, thanks a bunch for your help!


#9

@impolitePanda yes I was already planning on doing so, the documentation definitely wants to make this clear. Thanks for your understanding :slight_smile:

Update: https://issues.couchbase.com/browse/DOC-3640 created