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

Redirect/remap/fallback support in interceptors is not adequate #1081

Open
dmlloyd opened this issue Jan 8, 2024 · 0 comments
Open

Redirect/remap/fallback support in interceptors is not adequate #1081

dmlloyd opened this issue Jan 8, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@dmlloyd
Copy link
Contributor

dmlloyd commented Jan 8, 2024

Problem

The interceptor chain is linear. Each interceptor has the opportunity to return an answer for a requested property or to pass the request on to the next interceptor in the chain.

For many use cases this is sufficient. But, for use cases such as:

  • Property expansion
  • Compatibility redirects from one name to another
  • Compatibility defaults based on complex conditions

support is inadequate. The cause of this inadequacy comes down to the relationship between desired fallback behaviors and existing interceptors such as sources and default values.

When establishing a potentially complex compatibility behavior, the desired behavior is generally one of providing a value for a "new" property based on the value given for a "old" or "legacy" property or group of properties.

We can approximate this behavior today by choosing an early position in the interceptor chain, requesting all properties relevant to the original request, and finally performing (potentially complex) comparison and/or combination on the results. However, there are several flaws with this approach, not the least of which is that the interactions between default values and redirection can cause wrong answers unless extra care is taken in each case.

A better solution for compatibility redirection would be to interpose a config source directly before the default config source or interceptor which can provide a "pre-default" value for a "new" property based on the value(s) of the corresponding "legacy" property/properties. The reason we do not use this approach is because there is no way for such a source or interceptor to re-enter the configuration API from the "start" of the interceptor/sources chain, thus preventing it from knowing, at this point in the chain, whether there is a "legacy" value that can provide a "pre-default" (or "compatibility") resolution for the "new" property.

Proposed solution

Modify the config interceptor context interface to add a ConfigValue restart(String name) method, which queries the interceptor chain from the beginning.

Redirection interceptors could be placed near the end of the chain, just before the default values source. If the new name of the renamed property is queried, the restart method can be used to determine what value, if any, is present for the property's old name (note that the old name must not have a default value in the defaults source in this case). If the old property name is present, then its value is transformed (if desired) and returned. If it is not present, the query proceeds to the next source as usual. No complex arbitration logic is needed.

Risks

It is possible to craft an interceptor which causes an infinite loop by continually restarting with the same name, or with a circular sequence of redirections. We could, if necessary, mitigate this by keeping a thread-local reentry count on the restart implementation, with a relatively low cap like 20.

Assumptions

Sources and interceptors are currently divided into two separate spaces, with all sources compressed into a single interceptor. Default values are presently implemented using a config source. As such, it's not convenient to interpose a config interceptor between the "real" physical config sources and the "virtual" default values config sources.

In the long term, there should not be a separation between interceptors and config sources. This is one of several outstanding API issues that we must address. But for the near term, we can solve this problem by reserving ConfigSource implementations for use only by physical configuration sources, and move default value handling into an interceptor. In this way it is trivial to establish an interceptor between all "real" config sources and default/compatibility sources (of which there may be many, thanks to this general approach).

dmlloyd added a commit to dmlloyd/smallrye-config that referenced this issue Jan 8, 2024
radcortez pushed a commit that referenced this issue Jan 10, 2024
* Introduce `restart` method for interceptors

First step in solving #1081

* Add recursion limiter
@radcortez radcortez added the enhancement New feature or request label Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants