Created attachment 123559 [details]
proposed api change along with impl

Created attachment 123560 [details]
change in projectui forPackage -> forModule

please review this small api change to NbPreferences

Y01 Summary is unlikely correct: <summary>UNC-safe File / URI interconversion</summary>

Y02 Adding new method into NbPreferences.Provider interface is likely not backward compatible and will fail the build. Try "ant check-sigtest" to verify. Do you need this method at all? I thought the logic of the preferencesForModule(root, clazz) is hardcoded in the API and does not need anything from the SPI...

(In reply to comment #4)
> Y01 Summary is unlikely correct: <summary>UNC-safe File / URI
> interconversion</summary>

sure, thanks. copy and paste error.

> 
> Y02 Adding new method into NbPreferences.Provider interface is likely not
> backward compatible and will fail the build. Try "ant check-sigtest" to verify.
> Do you need this method at all? I thought the logic of the
> preferencesForModule(root, clazz) is hardcoded in the API and does not need
> anything from the SPI...

not really, the logic itself if done inside the Provider. It's indeed not backward compatible, but unlikely to affect anyone. Unless someone uses a different storage backend?

There is no backward compatible solution really. If the Provider doesn't implement the new method, there's no way to return a correct value in NBPreferences. Eg. if we extend Provider with Provider2 and add the new method there, what will the behaviour be if the implementation only fullfils the Provider contract?

(In reply to comment #5)
> There is no backward compatible solution really. If the Provider doesn't
> implement the new method, there's no way to return a correct value in
> NBPreferences. Eg. if we extend Provider with Provider2 and add the new method
> there, what will the behaviour be if the implementation only fullfils the
> Provider contract?

I guess we could achieve the same without introducing Provider2 (and without changing Provider incompatibly). Here is my attempt:

    public static Preferences forModule(Preferences from, Class cls) {
        Preferences tmp = forModule(cls);
        StringBuilder path = new StringBuilder();
        while (tmp != root()) {
            path.insert(0, '/');
            path.insert(0, tmp.name());
            tmp = tmp.parent();
        }
        return from.node(path.toString());
    }

The Javadoc of the node(String) method says that the nodes are not guaranteed to be permanent until flush() is called on them. That is why we only need to make sure the "tmp" node is not going to be written the disk at all, if it was created and remained empty. That requires a test. If the test passes, then we may need no API at all, as the above forModule(from, cls) method is trivial and it has only one known use so far...

(In reply to comment #6)
> The Javadoc of the node(String) method says that the nodes are not guaranteed
> to be permanent until flush() is called on them. That is why we only need to
> make sure the "tmp" node is not going to be written the disk at all, if it was
> created and remained empty. That requires a test. If the test passes, then we
> may need no API at all, as the above forModule(from, cls) method is trivial and
> it has only one known use so far...

That's a rather shaky solution, just for sake of backward compatibility, let's just forget about then.

(In reply to comment #7)
> (In reply to comment #6)
> > The Javadoc of the node(String) method says that the nodes are not guaranteed
> > to be permanent until flush() is called on them. That is why we only need to
> > make sure the "tmp" node is not going to be written the disk at all, if it was
> > created and remained empty. That requires a test. If the test passes, then we
> > may need no API at all, as the above forModule(from, cls) method is trivial and
> > it has only one known use so far...
> 
> That's a rather shaky solution, just for sake of backward compatibility, let's
> just forget about then.

let me describe in more detail why I think it's unstable solution.

What are the alternatives
1. use the above mentioned workaround in project only without any additional API. Affects only IDE users (as only a minority of platform users uses projects) and we can count on the IDE's own implementation of Provider

2. change the Provider API in non backward compatible way (suggested by this report). Affects only a small minority of platform users (in the IDE, we would scream out loud of some module implemented it) and gives them a clear, simple way to upgrade.

ON the contrary, your proposal is nominally backward compatible but communicates the change in a very bad way. The minority of platforms users affected might get spurious, subtle problems undiscoverable without reading the platform's codebase.