MP-Config 1.2.1: Must ConfigSource#getProperties() return an immutable Map?

[Laird Nelson]

Hello; I was wondering if the MicroProfile-Config 1.2.1 specification requires that the Map returned by ConfigSource#getProperties() be immutable.  If so, I didn't find the language that says so.

Thanks,

Best,

Laird

[Emily Jiang]

It depends on the config source implementation. You can make it either immutable (static config source) or mutable (dynamic config source). 

Thanks

Emily

[Laird Nelson]

Thanks for your reply!  Must a mutable Map returned by ConfigSource#getProperties() be thread-safe?  If so, what should a caller synchronize on when, say, iterating over its keySet() or entrySet()?  I didn't find any language specifying this.

[Alasdair Nottingham]

Emily,

Doesn’t mutability depend on the perspective? I assumed the Map returned by getProperties doesn’t support put and remove operations, certainly I wouldn’t expect a remove to affect the backing store. Or are you saying that is valid?

Alasdair Nottingham

--
You received this message because you are subscribed to the Google Groups "Eclipse MicroProfile" group.
To unsubscribe from this group and stop receiving emails from it, send an email to microprofile...@googlegroups.com.
To post to this group, send email to microp...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/microprofile/7591fcc6-9064-41c6-b59f-1e788e0a3003%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Emily Jiang]

Alasdair,
I realised that my reply is about the values inside the map might be different from time to time for a dynamic config source, which is not about the mutable/ immutable map.

Laird,

In order not to cause any undesirable behaviour/exceptions, the map should be immutable. In the class-level javadoc, we have

A ConfigSource is always read-only, any potential updates of the configured values must be handled directly inside each ConfigSource.

Is this clear enough?

Thanks
Emily

[Laird Nelson]

On Tuesday, June 5, 2018 at 3:25:30 PM UTC-7, Emily Jiang wrote:

In order not to cause any undesirable behaviour/exceptions, the map should be immutable. In the class-level javadoc, we have

A ConfigSource is always read-only, any potential updates of the configured values must be handled directly inside each ConfigSource.

Is this clear enough?

I suppose, though really I'm a big fan of specifications on methods.

Do you know why this method exists, given that ConfigSource's functionality is entirely contained by getPropertyNames() and getValue(String)?

Thanks,

Best,

Laird

[Emily Jiang]

It is nature to have a map to hold all properties for each config sources. The default impl of getConfigNames directly collects the keys of the map. Without this, the config source needs to figure out the keys.

One thing I want to bring it up is that the method getProperties may not contain the fullset of the config properties as the backend config source might be too big e.g. zookeeper. We have an open issue to document it better, see issue https://github.com/eclipse/microprofile-config/issues/333. Please add your comments and thoughts there.

Thanks
Emily

[Gunnar Morling]

Hi,

Assuming you're going to create a similar method in the Config JSR API, could you consider to return a dedicated type that only exposes the read methods, e.g. ConfigValueMap? It'd be nice if Java had read-only collection interfaces, but until that's the case I think it's a good idea to return dedicated types exposing just the right API to avoid this kind of issue.

--Gunnar

[Emily Jiang]

Good suggestions, Gunnar! Please add your comment to the issue, so that it will not get lost. Yes, the PR will happen for both MP Config and Config JSR. we are trying to keep mp Config and Config JSR in sync.

Thanks

Emily

--
You received this message because you are subscribed to a topic in the Google Groups "Eclipse MicroProfile" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/microprofile/tvjgSR9qL2Q/unsubscribe.
To unsubscribe from this group and all its topics, send an email to microprofile+unsubscribe@googlegroups.com.

To post to this group, send email to microp...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/microprofile/20edb710-b580-482d-b7fa-808fa29b08ea%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Thanks
Emily
=================
Emily Jiang
eji...@apache.org

[Laird Nelson]

On Wednesday, June 6, 2018 at 12:32:36 PM UTC-7, Emily Jiang wrote:

It is nature to have a map to hold all properties for each config sources.

I'm not sure I understand this.

 

The default impl of getConfigNames directly collects the keys of the map. Without this, the config source needs to figure out the keys.

Yes, exactly.  Since (a) ConfigSource is an interface and (b) it exposes a getValue(String) method and (c) it exposes a getPropertyNames() method, then these are all that are required, right?  If a given ConfigSource implementation wanted to use a Map internally that is fine, of course, but there's no need to expose the Map as part of the public API.  Now you have to answer all sorts of questions about synchronization, thread safety, iteration order, backing data store issues, and the like.

(I understand of course that the Map is already exposed, so this is academic, but for posterity I think it is important to emphasize that the surface area of ConfigSource is much too broad.)

 

One thing I want to bring it up is that the method getProperties may not contain the fullset of the config properties as the backend config source might be too big e.g. zookeeper. We have an open issue to document it better, see issue https://github.com/eclipse/microprofile-config/issues/333. Please add your comments and thoughts there.

OK.

Best,

Laird

[Laird Nelson]

On Thursday, June 7, 2018 at 1:58:41 AM UTC-7, Gunnar Morling wrote:

Hi,

Assuming you're going to create a similar method in the Config JSR API, could you consider to return a dedicated type that only exposes the read methods, e.g. ConfigValueMap?

Exactly.  Or consider whether a type is actually needed here (given that there's already getValue(String) and getPropertyNames()).

 

It'd be nice if Java had read-only collection interfaces, but until that's the case I think it's a good idea to return dedicated types exposing just the right API to avoid this kind of issue.

I agree.

Best,

Laird