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

XCOMMONS-2921: Add support for overwritting the configuration from the execution context #760

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mflorea
Copy link
Member

@mflorea mflorea commented Jan 31, 2024

Jira URL

https://jira.xwiki.org/browse/XCOMMONS-2921

Changes

Description

  • Implement a new "executionContext" configuration source
  • Add ConfigurationSource#removeProperty()
  • Add a helper to execute some code with modified configuration.

…e execution context

* Implement a new "executionContext" configuration source
* Add ConfigurationSource#removeProperty()
* Add a helper to execute some code with modified configuration.
Copy link
Member Author

@mflorea mflorea Jan 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not happy about this one, but I don't have a better option. I'm open to suggestions.

Copy link
Member

@tmortagne tmortagne Feb 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what we need is actually a configuration specific thing. To me, the real need is to pass a callable which is executed between an ExecutionContext set/unset and that's useful for various other use cases than just the configuration. You would then simply call the default ConfigutationSource in the callable you pass to the helper.

For example, it could simply be something like Execution#call(Map<String, Object> properties, Callable<V> callable)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, that proposal does not work well with custom storage in the ExecutionContext, which is most probably the case of the ExecutionContext ConfigurationSource.

@mflorea
Copy link
Member Author

mflorea commented Jan 31, 2024

Check xwiki/xwiki-platform#2834 for the usage.

Comment on lines +47 to +51
<dependency>
<groupId>org.xwiki.commons</groupId>
<artifactId>xwiki-commons-context</artifactId>
<version>${project.version}</version>
</dependency>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative was to put the code in xwiki-commons-context and add there the dependency on xwiki-commons-configuration-api, but @tmortagne noted that it's more probable for a module that requires the configuration API to also require the context API, than the other way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants