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

Improve the structure of the response from the /env endpoint #9864

Closed
wilkinsona opened this issue Jul 26, 2017 · 3 comments

Comments

@wilkinsona
Copy link
Member

commented Jul 26, 2017

Property sources in the environment are ordered, however the /env endpoint doesn't guarantee that a client will see this ordering. It uses a map for the property sources and, while the entries are ordered on the server side, there's no guarantee that a client will preserve that ordering. For example, the ordering of the keys with curl is:

  • profiles
  • server.ports
  • servletContextInitParams
  • systemProperties
  • systemEnvironment
  • applicationConfig: [classpath:/application.properties]

But the ordering of the keys with HTTPie is alphabetical:

  • applicationConfig: [classpath:/application.properties]
  • profiles
  • server.ports
  • servletContextInitParams
  • systemEnvironment
  • systemProperties

There's also a secondary problem that profiles isn't a property source (it's actually a list of the environment's active profiles) and if someone adds a property source named profiles the two will clash and the profiles property source will overwrite the list of the environment's active profiles.

To make it clear that the ordering of the property sources is significant I think it would be better if the response used a list rather than a map. This would also separate the property sources from the list of active profiles, avoiding the clash. Something like this:

{
    "activeProfiles": [],
    "propertySources": [
        {
            "name": "systemEnvironment",
            "properties": {
                //
            }
        },
        {
            "name": "systemProperties",
            "properties": {
                //
            }
        }
    ]
}
@spencergibb

This comment has been minimized.

Copy link
Member

commented Jul 26, 2017

That's very similar to the format returned from config server

@philwebb

This comment has been minimized.

Copy link
Member

commented Jul 26, 2017

We should also add the property origin information now that we have it.

@wilkinsona

This comment has been minimized.

Copy link
Member Author

commented Jul 28, 2017

To include the origin information, properties could become an array of property objects:

"properties": [
    {
        "name": "foo",
        "value": "one",
        "origin":""
    },
    {
        "name": "bar",
        "value": "two",
        "origin":""
    }
]

Alternatively, it could continue to be an object with the keys being the property names:

"properties": {
    "foo": {
        "value": "one",
        "origin": ""
    },
    "bar": {
        "value": "two",
        "origin": ""
    }
]

I have a slight preference for the latter as it reinforces the fact that property names are unique within a given source. Ordering isn't important so there's nothing to be gained by using an array.

@wilkinsona wilkinsona self-assigned this Aug 1, 2017

@wilkinsona wilkinsona closed this in 8f42007 Aug 1, 2017

wilkinsona added a commit that referenced this issue Aug 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.