KeySet::lookup(String) returns Key instead of null on unsuccessful search [BUG JNA binding]

[tucek]

Steps to Reproduce the Problem

KeySet set = KeySet.create();
if (set.lookup("user:/unsetkey") != null) {
  throw new AssertionError();
}

Expected Result

null should be returned.

Actual Result

AssertionError is thrown

System Information

• Elektra Version: 0.9.5
• Operating System: Ubuntu focal on wsl2

Proposed solution

KeySet::lookup(...) should return Optional<Key> and would return Optional.empty() if Elektra.INSTANCE.ksLookup(ByName)? returns null.

Workaround for 0.9.5

KeySet set = KeySet.create();
// check whether Key pointer is actually null
if (set.lookup("user:/unsetkey").get() != null) {
  throw new AssertionError();
}

[mpranj]

Thank you for reporting and working on this.

I am not a user of the Java bindings, so take this with a grain of salt.
From what I see this is not a bug. Since the Key class wraps JNA pointers you can not compare the Key object to null. You have to call get() (which is your workaround) or even simply use the existing isNull() function.

if (!set.lookup("user:/unsetkey").isNull()) {
  throw new AssertionError();
}

That being said, improvements to the Java bindings/API are welcome and a consistent use of optionals could make sense. 👍

[tucek]

At least the current behavior contradicts the documentation of the function: @return Key if search successful, null otherwise - Therefore i would consider this a bug.

Apart from that i would consider returning a Key object which has to be checked for referencing a null pointer (and therefore being completely useless), a rather crude approach for a Java API.

I will update the Java API to use Optionals accordingly. I will also consider removing the Key::isNull and require Key to have a non-null pointer reference.

[mpranj]

At least the current behavior contradicts the documentation of the function: @return Key if search successful, null otherwise - Therefore i would consider this a bug.

You're right, I overlooked the docu. According to the docu a fix would be to just return null instead of new Key(null). That is irrelevant though, if you want to change the API.