Adding default interface implementation for all Java Plugin functions except get

[tucek]

While reading up about the changes to the Java plugin facility, i stumbled across the necessity to implement all functions (open, get, set, error, close), even if a plugin does not support them.

I propose to alleviate this current requirement by adding default implementations for all Plugin interface methods except get. These default implementation would just throw UnsupportedOperationException and therefore would only have to be overridden if a Java plugin actually supports it.

In addition i would like to add static helper functionality to add the appropriate key to the contract for publishing the support of each specific plugin interface method. Something like: Plugin.publishGet(KeySet keySet), Plugin.publishSet(KeySet keySet), ...

Additionally the documentation and sample Java plugins would be updated.

[tucek]

@markus2330 should i proceed with providing a PR, or do we need to discuss this proposal first?

[kodebach]

These default implementation would just throw UnsupportedOperationException and therefore would only have to be overridden if a Java plugin actually supports it.

Sounds good. I'm actually not sure, why I didn't do this when creating the new API...

In addition i would like to add static helper functionality to add the appropriate key to the contract for publishing the support of each specific plugin interface method. Something like: Plugin.publishGet(KeySet keySet), Plugin.publishSet(KeySet keySet), ...

Actually, I initially had a version of the interface, where the published functions are defined separately, by an interface method that returns e.g. a EnumSet<PluginMethod>. We didn't go that route because it was too different from the C API, but I still think it would be a good idea.

My reasoning is this:

1. With a separate method and an enum for defining the published methods, you cannot create the necessary keys incorrectly.
2. By also putting the rest of the contract into a separate method, there is also no change of forgetting to add the contract or checking the parentKey incorrectly.
3. The get method could also have default implementation, which would better show that the actual get part is also optional and just the contract part is required.

Finally, there was/is a plan to also move the contract into a separate method in the C API (with similar benefits as above). But this will take more time and will definitely only happen in the new-backend branch. The Java API would simply be ahead for a certain amount of time.

---

If we do only add Plugin.publishGet(KeySet keySet), Plugin.publishSet(KeySet keySet), etc., we should probably also add a boolean Plugin.isContractKey(Key parentKey) to be used in the if in get.

[tucek]

I agree. So then the default implementation of get would handle calling an interface method KeySet getContract() (without default implementation). It would make use of a static interface function boolean Plugin.isContractKey(Key parentKey) and i would also add Plugin.publish*(KeySet keySet) functions referening them in the java doc of KeySet getContract().

The use of enums would not bring that much of a benefit, except we would also introduce an interface method EnumSet<PluginMethod> getPublishedMethods(), because the implementer would have to call it within the KeySet getContract() implementation anyways.

[kodebach]

So then the default implementation of get would handle calling an interface method KeySet getContract()

No the idea was that getContract() is called separately from the ProcessProtocol class at this point

https://github.com/ElektraInitiative/libelektra/blob/eac1c6a74cb75bb84ebfae2247b3cc531259ded8/src/bindings/jna/process/src/main/java/org/libelektra/process/ProcessProtocol.java#L41-L46

Similarly, a getPublishedMethods method would be called, the enum values would be translated into the correct keys and added to the contract.

I agree, without this additional change, adding a getContract and/or a getPublishedMethods method wouldn't do much good, since they need to be called manually anyways.

[markus2330]

Adding the default methods in the interface sounds useful to me. :sparkling_heart: As this is not an incompatible API change, it could be added straight away.

For the API changes I think its best if we write some decisions so that the API becomes coherent and follows the same principles everywhere. We shouldn't push these changes to master during the term anyway. Please don't make the changes too big, so that we actually can finish everything within your bachelor thesis.

[kodebach]

write some decisions so that the API becomes coherent and follows the same principles everywhere.

In theory a good idea. But the Java API is already a completely separate thing since it lives on the other side of the process plugin. So in this case, the protocol used by process is the public API. What the clients (currently only Java) do on their side of the protocol doesn't need coordination IMO. In fact, I think it is much better if the client side API is a good fit for the language than to follow some restrictions that are needed for a C API that isn't even used (directly).

The general operation of a plugin (get/set/etc.) should still be the same everywhere, but that isn't what we want to change here.

We shouldn't push these changes to master during the term anyway.

Yes, that is definitely true.

[tucek]

I agree. So the change should not be big at all. I will prepare the PR. When do you suggest to merge this change?

[markus2330]

On 15th of June are the last deadlines, from then we can merge. Decisions are very welcomed, it is not only consistency across the framework but also across generations of maintainers. The next person (co-)maintaining the Java-bindings also should know about the basic principles you used. If you don't like the decisions format, something like doc/DESIGN.md for Java would be fine, too.

[tucek]

@markus2330 I'm not familiar with the decisions format yet. IMHO I also don't think there is much of a decision to be made here, The Java binding should provide an easy and uncomplicated way of implementing Elektra plugins in Java. So I would prepare the following changes to reduce boiler plate and error proneness:

• Adding default implementations (throwing UnsupportedOperationException) for Plugin interface methods open, get, set, error, close.
• Adding two new interface methods (not having default implementations):
  • `KeySet getContract()´ - must return the contract information for the plugin (without the keys indicating which methods are published)
  • EnumSet<PluginMethod> getPublishedPluginMethods() - must return the methods to publish
• ProcessProtocol would then implement the check for PROCESS_CONTRACT_ROOT and return the combined contract from the methods introduced above.

@markus2330 @kodebach do you approve this change or is there a broader discussion to be held?

[markus2330]

I also don't think there is much of a decision to be made here

"here" maybe not but in the whole JNA binding some guidance would be useful. Consider it as a result of your thesis: what are they key points to make Elektra better accessible from Java?

EnumSet<PluginMethod> getPublishedPluginMethods() - must return the methods to publish

Is this needed? Is it for additional methods or for the Plugin interface?

Do you approve this change or is there a broader discussion to be held?

Default implementations is already approved.

[kodebach]

Is it for additional methods or for the Plugin interface?

Since the process plugin (and its protocol) don't allow additional methods, this can only be used for the standard methods.

Is this needed?

Since I had this idea, I'll clarify: It's not needed, but using the enum provides more type-safety. The idea was also to abstract some of the details of the contract KeySet, which people don't really need to know (e.g. the correct parentKey). However, since you need to provide the contract anyway for things like description and supported meta-keys, it might not be as good an idea as I thought at first. If we introduced a PluginContract class that abstracts the whole contract, it might be better. But that would be too much work right now.

So I'd actually say, we just add the default implementations and a new getContract method (without default).

[tucek]

If we do not want to add EnumSet getPublishedPluginMethods() (thereby reducing the possibility for the plugin author to make a mistake when publishing plugin methods), i would again propose to at least provide the following static function with the Plugin interface:

• Plugin.publishOpen(KeySet keySet)
• Plugin.publishGet(KeySet keySet)
• Plugin.publishSet(KeySet keySet)
• Plugin.publishError(KeySet keySet)
• Plugin.publishClose(KeySet keySet)

IMO the EnumSet<PluginMethod> getPublishedPluginMethods() approach is still better, because it separates the detail of having to publish methods for the integration via process to work, from the descriptions and other contract data specific to the plugin.

[tucek]

I will look into determining what default methods are being overridden by the Plugin implementation via reflection.

[tucek]

I'm removing me as assignee to, so this issue can be done via FLOSS course. If a solution with reflection is being considered, one should also think about the dependency that might create on java (jigsaw) modules.

[markus2330]

Yes, the dependencies would be triage for H1.

[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: