New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with JNA integration #2235

Closed
Piankero opened this Issue Sep 12, 2018 · 8 comments

Comments

Projects
None yet
2 participants
@Piankero
Contributor

Piankero commented Sep 12, 2018

I am trying to write a java application which interacts with kdb via JNA. I tried to make a set command fail for a setting with a check/enum on it.

Here is my setup (and really no other key below user):

kdb set user/user/test c
kdb setmeta user/user/test check/enum "a, b, c"

In my java application I load this keyset from above (only these keys!) and try to pass it to the enum plugin:

Key key = Key.create("user/user");
KeySet set = .. loaded KeySet via JNA binding
Plugin plugin = INSTANCE.elektraPluginOpen("enum", null,
                set.get(), key.get());     //.get() returns the pointer to the keyset 
set.rewind();
int returnCode = plugin.kdbSet.invoke(plugin, set.get(), key.get());

I came up to a few Questions until now:

  1. I already managed to call kdbset commands but I am not able to fetch Error Messages. I receive -1 or 1 but do not have a textual message. Your PLUGIN documentation mentions ELEKTRA_SET_ERROR which should write out textual messages to the parentKey. You documentation does not mention though where that parentKey actually is. Passing a key("user/user" in the example above) to that parameter does not work and leaves the key empty. Currently the above example returns -1 which should not be the case (since c is a valid enum out of a, b, c). When deleting the meta check/enum I receive the correct 1 value.

Also this does not work just in kdb:

kdb mount test.conf /test dump enum
kdb setmeta /test/enum check/enum "a, b, c"
kdb set /test/enum "d"
#> Error message telling me that "d" is not in enum
kdb lsmeta /test
#> Did not find key           ##Shouldn't this give me the error message?!
  1. Why do I have to call set.rewind() before invoking kdbSet? Otherwise I always get 1 as return result.

  2. In your implementation you have the following definition of e.g. kdbGet:
    typedef int (*kdbGetPtr) (Plugin * handle, KeySet * returned, Key * parentKey);

What is the Plugin * handle for?

  1. In your implementation you have this line:
    Plugin * elektraPluginOpen (const char * name, KeySet * modules, KeySet * config, Key * errorKey)

What are those modules for?

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Sep 12, 2018

Contributor

Simply pass on the parentKey you got. The name of the parentKey must be the mountpoint, the string must be the config's filename.

Shouldn't this give me the error message?!

No, only writing to config files rejects invalid configs. Otherwise you would not be able to do anything after invalid entries are in the files.

Seems like the enum plugin does not rewind by itself. KDB always rewinds, so you should rewind to. Please update this information in the description of how the plugin system works.

The plugin handle can be used to store data and to get the plugin's configuration. For more information see src/include/kdbplugin.h (elektraPluginGetConfig elektraPluginSetData elektraPluginGetData)

The modules could be used to load any module, currently the module loader is only used to load plugins. For more info see src/libs/loader/

Contributor

markus2330 commented Sep 12, 2018

Simply pass on the parentKey you got. The name of the parentKey must be the mountpoint, the string must be the config's filename.

Shouldn't this give me the error message?!

No, only writing to config files rejects invalid configs. Otherwise you would not be able to do anything after invalid entries are in the files.

Seems like the enum plugin does not rewind by itself. KDB always rewinds, so you should rewind to. Please update this information in the description of how the plugin system works.

The plugin handle can be used to store data and to get the plugin's configuration. For more information see src/include/kdbplugin.h (elektraPluginGetConfig elektraPluginSetData elektraPluginGetData)

The modules could be used to load any module, currently the module loader is only used to load plugins. For more info see src/libs/loader/

@Piankero

This comment has been minimized.

Show comment
Hide comment
@Piankero

Piankero Sep 13, 2018

Contributor

Thank you, there were indeed some additional metakeys! It looks like the enum plugin does not provide a useful error/reason metakey. The network plugin does though.

Here are is the key + all meta values:

user/user: /home/wespe/.config/default.ecf
	Meta [error: number description ingroup module file line function reason]
	Meta [error/configfile: /home/wespe/.config/default.ecf]
	Meta [error/description: Validation failed]
	Meta [error/file: /build/elektra-JyCBlP/elektra-0.8.14/src/plugins/enum/enum.c]
	Meta [error/ingroup: plugin]
	Meta [error/line: 101]
	Meta [error/module: enum]
	Meta [error/mountpoint: user/user]
	Meta [error/number: 121]
	Meta [error/reason: Validation failed]

There should be a good error message though as I can see this codesnippet:
ELEKTRA_SET_ERRORF (121, parentKey, "Validation of key \"%s\" with string \"%s\" failed.", keyName (key), keyString (key));
The difference I could see in the code is that the network plugin uses ELEKTRA_SET_ERROR while enum uses ELEKTRA_SET_ERRORF. Also I do not know why the enum plugin actually fails because again I gave a valid key:

user/user/test: a
	Meta [check/enum: a, b, c]

Just for curiousity: What happens if multiple errors occur? How do the metakeys look then?

KDB always rewinds, so you should rewind to.

You mean I should not need to rewind?

Thanks for the clarification.

Contributor

Piankero commented Sep 13, 2018

Thank you, there were indeed some additional metakeys! It looks like the enum plugin does not provide a useful error/reason metakey. The network plugin does though.

Here are is the key + all meta values:

user/user: /home/wespe/.config/default.ecf
	Meta [error: number description ingroup module file line function reason]
	Meta [error/configfile: /home/wespe/.config/default.ecf]
	Meta [error/description: Validation failed]
	Meta [error/file: /build/elektra-JyCBlP/elektra-0.8.14/src/plugins/enum/enum.c]
	Meta [error/ingroup: plugin]
	Meta [error/line: 101]
	Meta [error/module: enum]
	Meta [error/mountpoint: user/user]
	Meta [error/number: 121]
	Meta [error/reason: Validation failed]

There should be a good error message though as I can see this codesnippet:
ELEKTRA_SET_ERRORF (121, parentKey, "Validation of key \"%s\" with string \"%s\" failed.", keyName (key), keyString (key));
The difference I could see in the code is that the network plugin uses ELEKTRA_SET_ERROR while enum uses ELEKTRA_SET_ERRORF. Also I do not know why the enum plugin actually fails because again I gave a valid key:

user/user/test: a
	Meta [check/enum: a, b, c]

Just for curiousity: What happens if multiple errors occur? How do the metakeys look then?

KDB always rewinds, so you should rewind to.

You mean I should not need to rewind?

Thanks for the clarification.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Sep 13, 2018

Contributor

The difference in the error message seems rather pointless, it would be good to improve these error messages.

Just for curiousity: What happens if multiple errors occur? How do the metakeys look then?

This is also a question of the plugin interface: there only can be one error. Iirc the first one wins, the others are appended as warnings. But please test if this is actually the case and document it. It was improved some time ago by @tom-wa.

You mean I should not need to rewind?

Within the plugins you do not need to rewind. If you implement a plugin loader and call other plugins, you need to rewind. So it is a precondition for the plugins that they get a rewinded keyset. Please document this.

Contributor

markus2330 commented Sep 13, 2018

The difference in the error message seems rather pointless, it would be good to improve these error messages.

Just for curiousity: What happens if multiple errors occur? How do the metakeys look then?

This is also a question of the plugin interface: there only can be one error. Iirc the first one wins, the others are appended as warnings. But please test if this is actually the case and document it. It was improved some time ago by @tom-wa.

You mean I should not need to rewind?

Within the plugins you do not need to rewind. If you implement a plugin loader and call other plugins, you need to rewind. So it is a precondition for the plugins that they get a rewinded keyset. Please document this.

@Piankero

This comment has been minimized.

Show comment
Hide comment
@Piankero

Piankero Sep 13, 2018

Contributor

When executing a wrong kdb set in the console I receive the following message:
Validation of key "system/test/enum" with string "d" failed.
When doing the same in JNA I just get this message under error/reason:
Validation failed.
Where is the more detailed explanation?
The network plugin gives a good error message in JNA. In the enum plugin I cannot even locate the wrong key.

I will make a PR this afternoon which explains the rewind :)
Also I will see if only the first error gets into the metakeys of the parentkey.

Contributor

Piankero commented Sep 13, 2018

When executing a wrong kdb set in the console I receive the following message:
Validation of key "system/test/enum" with string "d" failed.
When doing the same in JNA I just get this message under error/reason:
Validation failed.
Where is the more detailed explanation?
The network plugin gives a good error message in JNA. In the enum plugin I cannot even locate the wrong key.

I will make a PR this afternoon which explains the rewind :)
Also I will see if only the first error gets into the metakeys of the parentkey.

@Piankero

This comment has been minimized.

Show comment
Hide comment
@Piankero

Piankero Sep 13, 2018

Contributor

I improved the documentation in #2237.

Contributor

Piankero commented Sep 13, 2018

I improved the documentation in #2237.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Sep 13, 2018

Contributor

Thanks for creating the PR!

Where is the more detailed explanation?

Hard to say without an PR/more detailed information, it seems like there is a bug in your plugin. Maybe you set this error without explanation somewhere?

Contributor

markus2330 commented Sep 13, 2018

Thanks for creating the PR!

Where is the more detailed explanation?

Hard to say without an PR/more detailed information, it seems like there is a bug in your plugin. Maybe you set this error without explanation somewhere?

@Piankero

This comment has been minimized.

Show comment
Hide comment
@Piankero

Piankero Sep 17, 2018

Contributor

The enum plugin itself sets the error. So it defenitely does not come from me!

Contributor

Piankero commented Sep 17, 2018

The enum plugin itself sets the error. So it defenitely does not come from me!

@Piankero

This comment has been minimized.

Show comment
Hide comment
@Piankero

Piankero Sep 18, 2018

Contributor

Alright, after some long tinkering I found out my system (or tbh myself) mixed apt-get installations with make install installations. The error message now is correct. Probably it was an old version from apt-get which still had the bad enum error message set.

Nonetheless thanks for your help!
@markus2330 karma++

Contributor

Piankero commented Sep 18, 2018

Alright, after some long tinkering I found out my system (or tbh myself) mixed apt-get installations with make install installations. The error message now is correct. Probably it was an old version from apt-get which still had the bad enum error message set.

Nonetheless thanks for your help!
@markus2330 karma++

@Piankero Piankero closed this Sep 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment