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

Review <Map> element in Jetty XML #5316

Closed
sbordet opened this issue Sep 23, 2020 · 2 comments
Closed

Review <Map> element in Jetty XML #5316

sbordet opened this issue Sep 23, 2020 · 2 comments

Comments

@sbordet
Copy link
Contributor

sbordet commented Sep 23, 2020

Jetty version
10.0.x

Description
The <Map> element is not used in Jetty XML files.

It cannot be customized to create Map implementations, it only creates HashMaps.

<Map> takes a list of <Entry> elements, but <Entry> takes a list of 2 <Item>, which is error prone (e.g. pass a number of items different from 2).

There is <Put> already that however is constrained to have a literal key in the name attribute (e.g. the key cannot be the result of a <SystemProperty>), which works with any Map implementations, for example java.util.Properties.

In summary, I think <Map> should be removed. Specific implementations can be created with <New>, and <Put> covers most cases. For the non covered cases, we can always use <Call>.

@sbordet sbordet added this to the 10.0.x milestone Sep 23, 2020
@gregw
Copy link
Contributor

gregw commented Sep 23, 2020

I don't think we can remove it without deprecating it for some time first.
Also I'd like to see Put enhanced to support:

  <Put>
    <Key>name</Key>
    <Value>value</Value>
  </Put>

Which is on trend for removing values from element parameters anyway!

If we can do that quickly.. then maybe we can just remove Map and have a configuration-10.dtd

BUT IT HAS TO BE DONE TODAY if it is to make it into 10.0.x

@sbordet sbordet removed this from the 10.0.x milestone Sep 23, 2020
@sbordet
Copy link
Contributor Author

sbordet commented Sep 23, 2020

Added class attribute and Class element to Map so that the implementation can be specified.

The other proposed changes were either backwards incompatible or too complex.

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

No branches or pull requests

2 participants