Skip to content

Endpoints that expose the context hierarchy should not assume it is linear #11019

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

Closed
wilkinsona opened this issue Nov 14, 2017 · 3 comments
Closed
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

A number of the endpoints produce JSON that reflects the application context hierarchy. For example, the configprops endpoint has a parent field in its response that contains the configuration properties of the parent context. This parent field may itself have a parent field that describes its parent's configuration properties.

This approach assumes that any context hierarchy is linear and that then response starts at the bottom (leaf) of the hierarchy. If there are two sibling contexts with the same parent then the current response format doesn't provide a good way to describe all of them.

We should change the response format of the relevant endpoints so that it doesn't make any assumptions about the structure of the context hierarchy.

@wilkinsona
Copy link
Member Author

It occurred to me that #11023 isn't strictly necessary to implement this. As the ID is only used to cross-reference contexts in the response, we could assign an arbitrary unique ID to each entry in the response that represents an application context.

@philwebb philwebb modified the milestones: 2.0.0.M7, 2.0.0.RC1 Nov 22, 2017
@wilkinsona wilkinsona added the status: blocked An issue that's blocked on an external project change label Jan 4, 2018
@wilkinsona wilkinsona removed the status: blocked An issue that's blocked on an external project change label Jan 11, 2018
@wilkinsona
Copy link
Member Author

There's rather more to this than I first realised.

When a bean in a context has the same name as a bean in one of its ancestors, Framework considers the bean to have been replaced. This means that, for any context in a linear hierarchy, bean names are guaranteed to be unique although the same name may refer to a different bean depending on which context in the hierarchy is queried.

When a context hierarchy isn't linear, for example because there are two child contexts that are siblings, both child contexts could contain a bean with the same name and one would not be a replacement for the other. As such, we want it to be possible for both of these beans to be included in an actuator endpoint's response.

We may also want two beans with the same name in a linear hierarchy to be returned in a response so that the replacement behaviour described above is apparent and can be diagnosed.

A number of actuator endpoints assume that bean names will be unique. That holds true today because the way that endpoints find beans and contexts is rather basic. That will change if/when #5312 is implemented.

Given that we're making breaking changes to the actuator endpoint response formats in 2.0, it would be good to make them as future-proof as possible. Part of that is not assuming that the hierarchy will be linear. As described above, a knock-on effect of non-linear hierarchies, is that bean names may no longer be unique. In addition to reworking how context hierarchies are represented, I think we should also rework how beans are represented so that their names are not used as keys in a map with a scope broader than a single context.

The following endpoints are affected in their current form:

  • Flyway
  • Liquibase

As the representation of the hierarchy is reviewed, we also need to consider the following endpoints:

  • Beans
  • Condition evaluation
  • Configuration properties

@wilkinsona wilkinsona self-assigned this Jan 15, 2018
@wilkinsona
Copy link
Member Author

wilkinsona commented Jan 16, 2018

I think the scheduled tasks endpoint should perhaps consider a hierarchy of contexts as well. Alternatively, it may be a variant of #5312 and the endpoint's response structure wouldn't change, and we'd introduce a mechanism to register ScheduledTaskHolder instances with the endpoint instead.

@wilkinsona wilkinsona reopened this Jan 16, 2018
@wilkinsona wilkinsona added for: team-attention An issue we'd like other members of the team to review and removed for: team-attention An issue we'd like other members of the team to review labels Jan 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants