Writable ConfigurationSource

Development
#1

Hi devs,

As part of https://forum.xwiki.org/t/replace-concept-of-user-in-new-user-api-and-more/6470 (see points 6, 7, 8), we need the ability to modify a configuration source. My specific need is to be able to modify a user’s properties without exposing Documents and XObjects/XProperties.

I see 2 options for modifying ConfigurationSource:

  • Option 1: setProperty() + save() methods. Each calls to setProperty will store the changes in a map in memory and the call to save() will persist them in one go.
     void setProperty(String key, Object value);
 void save();
  • Option 2: setProperties() accepting a Map. The persistence is done when the set method is called.
     void setProperties(Map<String, Object> properties);

ATM my preference goes to option 1 since it makes it simpler to use (if you need to do operations in between calls to setProperty for ex) and more clearly explains what’s happening. It’s also similar to what we do in the Document API.

WDYT?

#2

Hmm option 1 cannot work since most implementations of this Role are singleton components ATM. It would work for per thread ones though.

I’m starting implementing option 2. Please let me know what you think ASAP.

#3

Making ConfigurationSource writable is a big change and does not make sense for all implementations but it does make a lot easier some use cases. But we’ll have to decide what it means for bridges like DefaultConfigurationSource (seems to me an exception is in order like we do for ContextComponentManager, this is pretty much the same logic but for components).

#4

That’s what I thought but it’s not that big a change. First, it’s not going to break anything since it’s a new method and second we’re not going to implement it for all ConfigurationSource implementations (right now I’m planning to do it only for AbstractDocumentConfigurationSource). But yes it’s an API change.

Yes, Implementation that don’t support it will not implement the persistence methods:

    /**
     * @param properties the set of properties to persist
     * @throws ConfigurationSaveException when an error occurs during persistence
     * @since 12.4RC1
     */
    @Unstable
    default void setProperties(Map<String, Object> properties) throws ConfigurationSaveException
    {
        throw new UnsupportedOperationException("Set operation not supported");
    }

Exactly. It’ll just throw the UnsupportedOperationException from above. This is what I proposed/meant in item 8 of https://forum.xwiki.org/t/replace-concept-of-user-in-new-user-api-and-more/6470

Thanks

#5

There’s a small problem with option 2. I’m currently writing the typed API for UserProperties and since we have typed methods for each property, it means we’ll have to do a save for each, which isn’t great. Having a setProperty() + a save() like in option 1 would sure be much nicer.

Not sure yet what’s best…

#6

One solution is to add a save() method in UserProperties. The only downside is that it means that depending on the API you set (setProperties() vs setFirstName()/setEmail()/etc), you’ll need to call save() or not.

Do you prefer that or implementing option 1 and forcing component implementation that want to implement save() to not be singletons? Any other idea?

#7

We should start with making this logic specific to UserProperties IMO.

#8

Created: