[CM T0] extend src/bindings/jna/hello/src/main/java/HelloElektra.java

[markus2330]

src/bindings/jna/hello/src/main/java/HelloElektra.java should include examples from all parts of the API

Open for triage: which API parts are not covered yet? Please propose useful examples to be implemented.

Proposed examples:

• [ ] Create keys of all types supported by the Java binding
• [x] Check whether the keys are already available in the key database
  • If not, save them to the key database
  • If they are available, load their values
• [ ] Use KeySet iterator to output loaded values
• [x] Use Key name iterator iterator to output name
• [x] Update meta data on a key
• [x] Use Key iterator to dump meta data
• [ ] Use SortedSet interface methods (e.g. to combine loaded and initial KeySets)

You are encouraged to use the Java Streams API for working with iterators. Any notes on experiences with the Java binding API are very welcome. Please just comment here on your experience.

[JakobWonisch]

[FLOSS H1]

Some of the mentioned use cases seem to already be implemented:

• [x] Use KeySet iterator to output loaded values
• [X] Use Key name iterator iterator to output name

Additional use cases would be the following:

• [ ] Create a new KeySet from an existing one using a cut point
• [ ] Set binary and string values outside of Key.create (ties into the first already proposed example)

KeySet.create in this example passes size 10, which, as explained here has no effect, since every value greater than 0 and less than 16 is automatically adjusted to 16.

[leothetryhard]

We (@lukashartl and @leothetryhard) want to work on this issue. I compiled elektra with -DBINDINGS="ALL;-DEPRECATED" which to my understanding includes jna. I tried running the HelloElektra.java by following the JNA docs and executing ./gradlew hello:run in /src/bindings/jna. However the build fails with the following error:

Can you point me towards what I'm missing?

[kodebach]

Could you please try again... The error basically means that Gradle couldn't find the dependency com.thoughtworks.xstream (which is required by the gradle-versions plugin, we use). As far as I can tell, the dependency is available in the standard repository and for me ./gradlew hello:run just worked. Maybe it was a weird HTTP timeout or something similar...

If the error persists, I'll look into it again

[leothetryhard]

Yes it works now. pluings.gradle.org must have been down when I first tried. Thank you!

[leothetryhard]

[CM T3] Update: There are new issues when trying to run HelloJava. When executing ./gradlew hello:run the build fails in example 5:

The line of concern is Key k = ks.lookup(key).orElseThrow(AssertionError::new); My colleague @lukashartl was able to reproduce the problem. Has there been recent changes to .lookup()? Or maybe is it something trivial we are overlooking? Help would be appreciated @markus2330 @kodebach in order to continue with the work.

[kodebach]

Honestly, I'm not sure how this example ever could've worked. It doesn't call kdbSet at any point and doesn't insert a key into ks, yet the example expects ks.lookup(key) to always find a key.

@tucek Seems you added the .orElseThrow(AssertionError::new) in https://github.com/ElektraInitiative/libelektra/commit/053d20bb1f7393a287dd0c352bfb8d30d58aa900. Is there a reason why you expect this lookup to always find something? I would in fact expect that the Optional will be empty most of the time.

@leothetryhard I would suggest changing

			Key k = ks.lookup (key).orElseThrow (AssertionError::new);
			System.out.println (k.getString ());

into

			ks.lookup (key).ifPresent (k -> System.out.println (k.getString ()));

unless @tucek has another suggestion

[leothetryhard]

#4378 implemented:

• [x] Check whether the keys are already available in the key database

• If not, save them to the key database

• If they are available, load their values

• [x] Update meta data on a key

• [x] Use Key iterator to dump meta data

[tucek]

Honestly, I'm not sure how this example ever could've worked. It doesn't call kdbSet at any point and doesn't insert a key into ks, yet the example expects ks.lookup(key) to always find a key.

@tucek Seems you added the .orElseThrow(AssertionError::new) in 053d20b. Is there a reason why you expect this lookup to always find something? I would in fact expect that the Optional will be empty most of the time.

@leothetryhard I would suggest changing

			Key k = ks.lookup (key).orElseThrow (AssertionError::new);
			System.out.println (k.getString ());

into

			ks.lookup (key).ifPresent (k -> System.out.println (k.getString ()));

unless @tucek has another suggestion

@kodebach i strongly think it worked back in 891f8024ccfbec9d8ba4c4a33a8745c16f95b9ac - Since ks is being created line 25 (example 3) with key and not being cleared. Should get in line 44 clear ks before returning the result?

EDIT: Of course this does not mean, it is a perfect example ;)

[kodebach]

I'm not 100% sure without looking at the code, whether or not the current kdbGet would clear the key. In the new-backend branch IIRC I took the approach that after kdbGet the part below parentKey matches exactly what is persisted in the KDB. Otherwise, you'd need to manually clear the KeySet before calling kdbGet to make sure you actually get the persisted config, if e.g. a key was removed by another process between two kdbGet calls.

Of course this does not mean, it is a perfect example ;)

Yes, even if it works. It is very confusing as a first example. Especially without a comment stating e.g. "this works because ...".

[tucek]

I think the best course of action is to improve the example by actually writing a key to KDB and then fetching it and removing it afterwards. do you agree?

[markus2330]

This would also tests many other things which you might actually to avoid to test :wink:

[tucek]

Could we then just try to read a key that is guaranteed to be already there? Would you have any candidates in mind?

[tucek]

I'm not 100% sure without looking at the code, whether or not the current kdbGet would clear the key. In the new-backend branch IIRC I took the approach that after kdbGet the part below parentKey matches exactly what is persisted in the KDB. Otherwise, you'd need to manually clear the KeySet before calling kdbGet to make sure you actually get the persisted config, if e.g. a key was removed by another process between two kdbGet calls.

Of course this does not mean, it is a perfect example ;)

Yes, even if it works. It is very confusing as a first example. Especially without a comment stating e.g. "this works because ...".

As far as i understand from looking up the code ksLookup just appends to the output keyset, therefore i don't understand why the test should not work...

[kodebach]

I assume you mean kdbGet and not ksLookup.... Yes, it seems kdbGet currently just appends (IMO wrong, but it is what it is). So I'd say, if the tests works just leave it the way it is.

[tucek]

I haven't yet tested it my self, but the @leothetryhard claims the test only works, if we do not assert the key is still in the keyset. If kdbGet only appends, this assertion should work...

[kodebach]

The docs say kdbGet only appends, but there could very well be a bug. There are definitely some ksClear calls in kdbGet, so it might be that the KeySet is cleared before the keys are appended.

https://github.com/ElektraInitiative/libelektra/blob/a5b8684ed10a939b94597e587730ae5b26b09128/src/libs/elektra/kdb.c#L1426

https://github.com/ElektraInitiative/libelektra/blob/a5b8684ed10a939b94597e587730ae5b26b09128/src/libs/elektra/kdb.c#L1476

[markus2330]

Could we then just try to read a key that is guaranteed to be already there? Would you have any candidates in mind?

system:/elektra/version/infos/licence contains BSD

There are definitely some ksClear calls in kdbGet, so it might be that the KeySet is cleared before the keys are appended.

Is this still the problem in the new-backend branch? We should clean it up, can you please create an issue? (@kodebach)

[kodebach]

Is this still the problem in the new-backend branch?

To quote myself from above:

In the new-backend branch IIRC I took the approach that after kdbGet the part below parentKey matches exactly what is persisted in the KDB. Otherwise, you'd need to manually clear the KeySet before calling kdbGet to make sure you actually get the persisted config, if e.g. a key was removed by another process between two kdbGet calls.

It's also described this way in the updated documentation in new-backend (some docs may be outdated).

[markus2330]

Yes, exactly, there was a "IIRC" in the sentence and there might be outdated docs.

I agree that kdbGet should only append, so ksClear (ks) in the code is fishy. If the problem might be in the new-backend branch, we need to investigate (create a new issue etc.). If it is gone in new-backend, we do not need to worry as time (once new-backend gets merged) will fix the issue.

To come to 1.0 we need to be more careful about such issues. Hints about important bugs must be taken seriously. And that the whole config of the user gets cleared (which might be persisted in a later kdbSet) is a very serious bug.

[kodebach]

that the whole config of the user gets cleared (which might be persisted in a later kdbSet) is a very serious bug

I don't think that this is what happens. Something like that would have been spotted pretty quickly. If ksClear is actually used to clear parts of ks (and not some temporary copy), then I'm sure these parts will then be replaced by the current contents of the KDB (that's what happens in new-backend).

In the new-backend branch we use ksClear exactly, because there may be a later kdbSet. Take this scenario for example:

1. Application A calls kdbGet with ks
2. Application B deletes a key from the KDB
3. Application A calls kdbGet again with the same ks (to get the new state of the KDB)
4. Application A does some modifications to ks and calls kdbSet to persist them

If kdbGet only appends, then the key that B deleted will be inserted again by kdbSet in 4. That's why in new-backend anything that would overlap with a mountpoint loaded by kdbGet is removed from ks. All other keys in ks remain, since they won't be persisted by kdbSet.

In the only appends version, a caller would have to pass an empty ks (i.e. call ksClear(ks) first) to ensure the state of the KDB is correctly reflected after kdbGet.

Note: Adding default fallbacks before calling kdbGet should still work in new-backend. You can add default:/ keys. These will never overlap with a mountpoint (since adding default:/ mountpoints is forbidden) and therefore won't be cleared (they also won't be persisted, but that's a good thing). If this doesn't work in new-backend right now, then that's a bug.

I agree that kdbGet should only append

I'm not sure that would be the best solution, but if you think it is better than the current approach in new-backend. Then we can change it.

[github-actions[bot]]

I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart:

[github-actions[bot]]

I closed this now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue. Thank you for your contributions :sparkling_heart: