Skip to content
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

spec plugin does not remove added meta data on arrays #4961

Closed
atmaxinger opened this issue Jun 9, 2023 · 4 comments · Fixed by #4963
Closed

spec plugin does not remove added meta data on arrays #4961

atmaxinger opened this issue Jun 9, 2023 · 4 comments · Fixed by #4963
Labels

Comments

@atmaxinger
Copy link
Contributor

atmaxinger commented Jun 9, 2023

The metadata added to arrays by the spec plugin is not being removed.
This issue needs to be addressed in the next release, as it poses a significant obstacle for applications that rely on storage plugins that lack support for custom metadata, like TOML.
This also blocks progress on Opensesame.

Here is a reproducible example, you can add this function to testmod_spec:

static void test_hook_remove_spec_keys_from_array_should_succeed (void)
{
	printf ("test %s\n", __func__);

	TEST_BEGIN
	{
		KeySet * ks = ksNew (0, KS_END);

		ksAppendKey (ks, keyNew ("spec:/sw/libelektra/opensesame/#0/current/sensors/#/loc", KEY_META, "description", "location of the sensor", KEY_END));
		ksAppendKey (ks, keyNew ("system:/sw/libelektra/opensesame/#0/current/sensors/#0/loc", KEY_VALUE, "guests", KEY_END));

		int resultCopy = elektraSpecCopy (NULL, ks, parentKey, true);

		// check if meta keys were copied
		Key * realKey = ksLookupByName (ks, "system:/sw/libelektra/opensesame/#0/current/sensors/#0/loc", 0);
		succeed_if (realKey != NULL, "key must exist");
		succeed_if (keyGetMeta (realKey, "meta:/description") != NULL, "meta must exist after copying");

		int resultRemove = elektraSpecRemove (NULL, ks, parentKey);

		// check if meta keys were removed
		ksLookupByName (ks, "system:/sw/libelektra/opensesame/#0/current/sensors/#0/loc", 0);
		succeed_if (realKey != NULL, "key must exist");
		succeed_if (keyGetMeta (realKey, "meta:/description") == NULL, "meta must NOT exist after copying");

		TEST_CHECK (resultCopy == ELEKTRA_PLUGIN_STATUS_SUCCESS, "plugin should have succeeded");
		TEST_CHECK (resultRemove == ELEKTRA_PLUGIN_STATUS_SUCCESS, "plugin should have succeeded");

		ksDel (ks);
	}
	TEST_END
}
@markus2330
Copy link
Contributor

Thanks for reporting this problem! I agree this is important.

@tmakar can you maybe give an hint how/where to fix the problem?

Any volunteers to fix it?

@tmakar
Copy link
Contributor

tmakar commented Jun 9, 2023

Thanks for reporting this problem! I agree this is important.

@tmakar can you maybe give an hint how/where to fix the problem?

Any volunteers to fix it?

Whoever fixes this, the only place responsible for removal of metakeys is elektraSpecRemove. All the code resides in this function.

probably first checking if Key * specKey = ksLookup (returned, specName, 0); returns the correct spec key. If yes, then it makes sense to check the actual removal of the metakey in the if clause below the code above. Everything is in the elektraSpecRemove function.

tmakar added a commit to tmakar/libelektra that referenced this issue Jun 9, 2023
@tmakar tmakar linked a pull request Jun 9, 2023 that will close this issue
19 tasks
@tmakar
Copy link
Contributor

tmakar commented Jun 9, 2023

@atmaxinger @markus2330

I did the bugfix by myself now 😁
#4963

@markus2330
Copy link
Contributor

@tmakar thank you so much! ❤️ Looks very good, only one small memory bug that should be easy to fix.

atmaxinger added a commit that referenced this issue Jun 10, 2023
…-array

fix: remove metakeys from array keys correctly (#4961)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants