This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 134580

Summary: Preferences for Project
Product: projects Reporter: Jan Lahoda <jlahoda>
Component: Generic InfrastructureAssignee: Jan Lahoda <jlahoda>
Status: RESOLVED FIXED    
Severity: blocker CC: apireviews, dkonecny, jglick
Priority: P3 Keywords: API, API_REVIEW_FAST
Version: 6.x   
Hardware: All   
OS: All   
Issue Type: ENHANCEMENT Exception Reporter:
Bug Depends on: 191393    
Bug Blocks:    
Attachments: Proposed patch.
Sample project.xml.
Updated patch.
Updated patch.
Updated patch.

Description Jan Lahoda 2008-05-06 10:45:51 UTC
Currently, clients can use AuxiliaryConfiguration to store arbitrary (configuration) data in the project. From client's
point of view, the use of AuxiliaryConfiguration is non-trivial, especially if the client needs to store only "a few"
"primitive" values.

I think that method like:
Preferences ProjectUtils.getPreferences(Project p, Class c, boolean shared)
which would mimic NbPreferences.forModule would be useful for the clients. The returned Preferences would use
AuxiliaryConfiguration to store the data.

If this is considered to be a good idea, I will try contribute the change.
Comment 1 Milos Kleint 2008-05-07 14:05:27 UTC
sounds ok to me.
Comment 2 Jesse Glick 2008-05-13 02:29:02 UTC
Not bad, though I would actually rather see a matching SPI; for example, if the project provided an instance of
Preferences (i.e. a root node) in its Lookup, that would be used instead of AuxiliaryConfiguration. This would permit
more natural storage of simple key-value preferences in a .properties file. (For example, in project.properties or
private.properties with a "prefs." prefix to isolate them from Ant-related properties.)

Also note that a full implementation of Preferences is not trivial. You need to implement listening, subnodes, etc. - a
significant amount of code, as seen in our impl of NbPreferences. All this may be overkill for our needs. A slim
interface like

interface ProjectPreferences { // SPI, API would be matching final class
  String get(String key);
  void put(String key, String/*|null*/ value);
}

might suffice (callers would be expected to choose distinctive keys).
Comment 3 Jan Lahoda 2008-05-14 14:41:00 UTC
Re SPI I was thinking about such an SPI, but seemed quite complex to me. We would need to use AuxiliaryConfiguration as
the storaqe if the preferences SPI is not available anyway, IMO. And if a project would not implement the Preference SPI
for e.g. 6.5, and implemented it after 6.5, we couldn't simply use the Preferences SPI in post-6.5, as the project
wouldn't work in 6.5. We would need to wait for project upgrade. So seemed to me much simpler and acceptable for me to
use only AuxiliaryConfiguration as the storage (the preferences SPI could be added at any time in the future is necessary).

Impl. of Preferences does not seem so difficult to me - it seems to be enough to extend AbstractPreferences and
implement a few methods. The use of Preferences allows existing features to be configured on the project-level with
minimal changes.

Thanks for the comments.
Comment 4 Jesse Glick 2008-05-14 15:58:42 UTC
Regarding SPI, of course we would need a fallback implementation of Preferences (or whatever the interface is) to use AC
in case the project did not provide one. I do not understand your compatibility argument. The client calls something in
ProjectUtils and reliably gets an interface to r/w properties. PU automatically uses a project-specific backend if
available, otherwise AC. The client need not know the difference. Eventually we could probably deprecate AC.

"Impl. of Preferences does not seem so difficult to me" - well talk to Radek Matous who spent quite some time
implementing it for NbPreferences. The asynchronous nature of storage, plus change support, makes it tricky. A
simplified interface letting you get and set simple string keys (no hierarchical nodes) with no change support would be
far easier to implement.

For unshared properties, I would recommend simply using NbPreferences. You can just e.g. hash the project directory path
and include that in the key, to make a setting apply to just one project. (Does not carry over when the project is
moved, but this is usually desirable anyway, and nbproject/private/ gets deleted in a move too.)
Comment 5 Jan Lahoda 2008-05-19 16:40:12 UTC
Sorry for confusion on the compatibility: I meant compatibility from user's point of view. If a project is created in
6.5, opened in post-6.5 (not upgraded) and then opened in 6.5, it should work reasonably. Another possible problem is
support for inner nodes - this may be difficult to do inside one .properties file. I would rather start with a simple
implementation based solely on the AuxiliaryConfiguration, and introduce some kind of faster/better SPI later if necessary.

The reason why I am proposing this API is that features that use "global" Preferences can easily be changed to use
project-level Preferences. If there would be two separate API, it would be more difficult for these feature to be ported
from "global" to "project" settings level. I think that the benefit/cost ratio of Preferences is quite good.

Thanks for the suggestion regarding private properties - will try to do it this way.
Comment 6 Jesse Glick 2008-05-19 18:58:56 UTC
Support for inner nodes is one of the main reasons I do not suggest using j.u.p.Preferences; inner nodes are a
significant complication for which there is no real use case.
Comment 7 Jan Lahoda 2008-05-30 11:16:05 UTC
Re support for inner nodes: e.g. each Java Hint has its own Preferences node and could create its own sub-nodes.
Moreover, Preferences are a standard API, so reusing it seems OK to me.

I am attaching a patch that implements ProjectUtils.getPreferences. I would like to start an API review on adding this
method, unless there are objections.

A note on tests: the tests require an implementation of AuxiliaryConfiguration. I used J2SE Project for simplicity - I
will try to rewrite the test so that the dependency on J2SE Project is not necessary.
Comment 8 Jan Lahoda 2008-05-30 11:16:53 UTC
Created attachment 62158 [details]
Proposed patch.
Comment 9 Jesse Glick 2008-05-30 18:37:50 UTC
Regarding support for inner nodes: you don't really need subnodes. You can just use "foo.bar.baz" syntax. Compare
Firefox/Thunderbird settings storage, etc.

Regarding project upgrade compatibility: if the fallback SPI impl for shared properties used AC, the API impl would just
need to check in both AC and the project's custom SPI for a given key. I.e.:

package spi;
interface Impl {
    String get(String key, boolean shared);
    void put(String key, String value, boolean shared);
    // optionally also: Iterable<String> keys(boolean shared);
}
package api;
public static String get(Project project, String key, boolean shared) {
    Impl i = p.lookup.lookup(Impl);
    if (i != null) {
        String v = i.get(k, shared);
        if (v != null) {
            return v;
        }
    }
    AC ac = project.lookup.lookup(AC);
    if (ac != null) {
        String v = ACHelper.get(ac, key, shared);
        if (v != null) {
            return v;
        }
    }
    // otherwise ignore shared flag
    // FileObject.attribute best here; a future masterfs impl could in fact use NbPreferences internally:
    Object o = project.getProjectDirectory().getAttribute(key);
    return (o instanceof String) ? (String) o : null;
}
public static void put(Project project, String key, String value, boolean shared) {
    Impl i = p.lookup.lookup(Impl);
    if (i != null) {
        i.put(key, value, shared);
        return;
    }
    AC ac = project.lookup.lookup(AC);
    if (ac != null) {
        ACHelper.put(ac, key, value, shared);
        return;
    }
    project.getProjectDirectory().setAttribute(key, value);
}
// optionally also: public static Iterable<String> keys(Project project, boolean shared)

I would still prefer the option for the project to store simple key/value properties in a more natural way - in
project.properties, private.properties, etc. Going through AuxiliaryConfiguration is awkward, inefficient, and harder to
debug. Also will look extremely ugly for projects which use FileObject.setAttribute to implement AC. (I plan to submit
an API change to provide a fallback impl of AC using FO.attribute. Should be useful for Maven projects, autoproject,
etc.) I won't block this implementation which forces everything to go through AC, I just don't like it so much.

Anyway, review comments:

[JG01] The test dependency on j2seproject is illegal (will cause cycles building tests). Better to make a fake project
type with a trivial AC impl which just persists its DOM fragments in memory.

[JG02] Please use descriptive parameter names, not "p" etc.

[JG03] "(specified as using {@link Class} as in {@link org.openide.util.NbPreferences#forModule(java.lang.Class)})"
duplicates information in @param and can be deleted.

[JG04] "for which project should be the preferences returned" => "project for which preferences should be returned"

[JG05] getPreferences fails to document that it could return null. In such a case you could consider just delegating to
getPrivateSettings to fall back gracefully and simplify the caller's API.

[JG06] I'm not sure I get the point of the __idXXXX key. Is there some reason you cannot just return prefs.node(projectDir)?

[JG07] findCNBForClass could be made more efficient by copying the simple impl from NbPreferences.

[JG08] type="module" in the schema should perhaps be type="module-or-node"? maxOccurs=unbounded" is a syntax error.
Actually try validating some files (preferably in the unit test). The names of elements in the schema are very
confusing. What does a sample document actually look like?

[JG09] Does your Preferences impl fire changes? If so, it should be tested. If not, it should be documented.

[JG10] If the project does provide an AC, that should be used for both shared and private settings (passing the
appropriate boolean flag to AC methods).
Comment 10 Jan Lahoda 2008-06-02 11:01:21 UTC
I have added a simple SPI interface that allows efficient backstore for the properties and an impl provided by
AntProjectHelper.
Currently stores the data into nbproject/project.properties and nbproject/private/private.properties, but we may want to
introduce something like
nbproject/auxiliary.properties and nbproject/private/auxiliary.properties.

JG01: fixed
JG02: done
JG03: done
JG04: done
JG05: added a note on null value. I do not think it is correct to return non-sharable settings when sharable settings
are requested.
JG06: prefs.node(projectDir) could cause the path would be too long for the system. Preferences also have a limit on the
length of the key and node name.
Anyway, I have rewritten this to use FileObject.{g|s}etAttribute.
JG07: done (although I did not find a simplier way than to introduce a dep on Module System and loop through installed
modules).
JG08: fixed the problems in the schema. I am attaching a sample document. Didn't have time to write the test yet, but I
will try.
JG09: It does - the functionality is provided by java.util.prefs.AbstractPreferences. So the test would actually test
whether the j.u.p.AbstractPreferences
is implemented correctly.
JG10: done (although I understood the comment in #desc5 in such a way that the private properties should go through
NbPreferences in all cases).

Attaching a new patch.
Comment 11 Jan Lahoda 2008-06-02 11:02:53 UTC
Created attachment 62234 [details]
Sample project.xml.
Comment 12 Jan Lahoda 2008-06-02 11:04:08 UTC
Created attachment 62235 [details]
Updated patch.
Comment 13 Jan Lahoda 2008-06-02 11:07:01 UTC
I would like to ask API review for addition of:
org.netbeans.api.project.ProjectUtils.getPreferences(Project, Class, boolean)
org.netbeans.spi.project.AuxiliaryProperties
org.netbeans.spi.project.support.ant.AntProjectHelper.createAuxiliaryProperties()
Comment 14 Jesse Glick 2008-06-02 18:51:14 UTC
Regarding JG05: I still think it is better to fall back to nonsharable settings in this case. At least the configuration
will work for the current user - better than nothing. And what is the caller code supposed to do, anyway? Check for
null, and if null, .... try using nonsharable settings instead. Better to keep the caller code concise and simple. This
is consistent with the general API philosophy of Preferences - hope that the backing store is available but do not
burden the caller if there is a problem with it, under the reasonable assumption that preferences are not precious and
failure to store them will not cripple the application.


Regarding JG06: NbPreferences has no such length restrictions.


Regarding JG07: hmm, may be inefficient to loop this way. Do you really need to group things by module anyway? Is there
anything wrong with using simple package name, as in Preferences.forPackage?


[JG11] Module names should be code.name.base, not code-name-base which is used only in filenames.


[JG12] Will you use the APH-based implementation in j2seproject and similar? According to the patch, I guess you will -
but this contradicts the sample project.xml, doesn't it?


[JG13] ProjectUtils.getPreferences should mention its SPI and when it is used.


[JG14] What is the purpose of the CleaningWeakReference? Can't you just have a
WeakHashMap<Project,AuxiliaryConfigBasedPreferencesProvider>?


[JG15] AuxiliaryPropertiesImpl.java (in project.ant) appears to be missing from the patch.
Comment 15 Jan Lahoda 2008-06-02 20:23:31 UTC
Regarding JG05: if we auto-fallback to non-sharable settings, the client code cannot decide what to do (it could, for
example, show a warning to the user). Also, I am not sure if the users would consider their setting "not precious". If a
non-null value is required, we need to add a new method like isSharablePreferencesSupport(Project).

Regarding JG06: NbPreferences has no such length restrictions: only for property names (which is a clear violation of
the Preferences contract, and moreover the impl is incorrect). There is still limitation on node name length. Also the
path length issues still may occur.

Regarding JG12: I plan to use the APH-based impl. for j2seproject and apisupport project. I attached the project.xml as
a sample for only illustration.

Regarding JG14: such a map would leak.

Regarding JG15: Oops, attaching a patch that contains it.
Comment 16 Jan Lahoda 2008-06-02 20:24:17 UTC
Created attachment 62281 [details]
Updated patch.
Comment 17 Jesse Glick 2008-06-02 20:36:52 UTC
JG14: you mean because the value (ACBPP) refers to the key (Project). But your current code will leak, too, since the
key is held strongly by HashMap. I might instead suggest using a WeakHashMap as originally suggested but not keeping a
project field in ACBPP. (If you need to call saveProject in flush, keep the projectDirectory instead.)
Comment 18 Jan Lahoda 2008-06-02 21:05:03 UTC
JG14: the project should be removed from the map by the CleaningWeakReference (see testReclaimable). To use WeakHashMap,
the AuxiliaryConfiguration and AuxiliaryProperties couldn't be stored in the ACBPP (these are allowed to hold the
corresponding instance of Project, IMO). So it would be needed to lookup them from the project lookup each time they are
accessed (and the returned instances could vary in time).
Comment 19 Jesse Glick 2008-06-03 01:25:45 UTC
JG14: I see your point, but I think it would be accomplished more easily with a simple
WeakHashMap<Project,WeakReference<ACBPP>>: if the value gets collected, make a new one, but don't bother to remove
unused keys for Project's which still exist. You are trying to make a "biweak" map, which is impossible, so you have to
use one of various workarounds. See Java bug #6389107 for discussion.
Comment 20 Jan Lahoda 2008-06-06 14:21:34 UTC
JG05: I have removed the note about null return value - will rewrite the impl. to use
ProjectUtils.getAuxiliaryConfiguration once it is available.
JG11: if the cnb used dots, these would need to be escaped before they would be written into the properties file (dots
are used to separate nodes in the properties files), so I used dashes. Hopefully this is not a big problem.
JG13: done
JG14: Ok, I used (Weak)HashMap+WeakReference to replace CleaningWeakReference.

I am attaching an updated patch and will integrate on Monday, unless there are objections.
Comment 21 Jan Lahoda 2008-06-06 14:22:28 UTC
Created attachment 62467 [details]
Updated patch.
Comment 23 Jan Lahoda 2008-06-09 12:49:51 UTC
Integrated.
Comment 24 Jesse Glick 2008-06-09 20:26:36 UTC
Don't forget to update www/www/ns (in CVS) with the new schema (auxiliary-configuration-preferences.xsd). This is
necessary to permit files using it to be validated.
Comment 25 Jan Lahoda 2008-06-09 20:40:32 UTC
Thanks for reminder:
lahvac@lahvac-laptop:/tmp/www/www/ns/auxiliary-configuration-preferences$ cvs commit
RCS file: /cvs/www/www/ns/auxiliary-configuration-preferences/1.xsd,v
done
Checking in 1.xsd;
/cvs/www/www/ns/auxiliary-configuration-preferences/1.xsd,v  <--  1.xsd
initial revision: 1.1
done
Comment 26 David Konecny 2008-06-09 23:05:04 UTC
Do you mind fixing this also for web/j2ee project types: web.project, j2ee.clientproject, j2ee.earproject and
j2ee.ejbjarproject? It would be highly appreciated.
Comment 27 David Konecny 2008-06-15 21:47:49 UTC
Is it actually necessary to add something to a project type? You added helper.createAuxiliaryProperties() to J2SE
project but web/j2ee projects seem to work without that as ProjectUtils provide a fallback, right?
Comment 28 Jesse Glick 2008-06-16 15:58:34 UTC
Use of APH.cAP is preferable as it uses the *.properties storage which is more concise, does not trigger
build{,-impl.xml} regeneration, etc.
Comment 29 Jesse Glick 2008-06-16 15:59:57 UTC
[JG16] Please mention in Project.getLookup Javadoc that an impl of AuxiliaryProperties is encouraged, and in the Javadoc
for AP mention APH.cAP as an example implementation.
Comment 30 David Konecny 2008-06-18 03:31:24 UTC
Re. "Use of APH.cAP is preferable" - I fixed that for web/j2ee. Next time if you decide not to fix all project types it
would useful to at least file issue so that it is not forgotten and and somebody else can do. Thanks, -David.
Comment 31 Quality Engineering 2008-06-18 15:58:10 UTC
Integrated into 'main-golden', available in NB_Trunk_Production #267 build
Changeset: http://hg.netbeans.org/main/rev/b5b11c3cd9d9
User: David Konecny <dkonecny@netbeans.org>
Log: #134580 - Preferences for Web/J2EE Projects
Comment 32 Quality Engineering 2008-06-20 15:52:34 UTC
Integrated into 'main-golden', available in NB_Trunk_Production #271 build
Changeset: http://hg.netbeans.org/main/rev/07659e55df39
User: Tomas Mysik <tmysik@netbeans.org>
Log: Preferences for PHP Projects

Related to #134580 - Preferences for Web/J2EE Projects.