Bug 136333

Summary: Fallback implementation of AuxiliaryConfiguration
Product: projects Reporter: Jesse Glick <jglick>
Component: Generic InfrastructureAssignee: Jesse Glick <jglick>
Status: RESOLVED FIXED QA Contact: issues <issues.netbeans.org>
Priority: P3 CC: abadea, apireviews, dkonecny, jlahoda, mkleint
Version: 6.xKeywords: API, API_REVIEW_FAST
Target Milestone: 6.x   
Hardware: All   
OS: All   
Issue Type: ENHANCEMENT Exception Report:
Attachments: Proposed core patch
Rewritten patch

Description Jesse Glick 2008-06-03 03:00:15 UTC
Currently AuxiliaryConfiguration is used improperly as both an API and SPI, and if it is not found in project lookup,
there is no fallback implementation for callers.
Comment 1 Jesse Glick 2008-06-03 03:29:57 UTC
Created attachment 62297 [details]
Proposed core patch
Comment 2 Jesse Glick 2008-06-03 03:34:40 UTC
Please review the proposed API enhancement. The patch to commit would also include:

- spec version update, apichanges.xml, @since

- warning in AC Javadoc not to look for it in project lookup directly but to use PU instead

- removal of ACI in contrib/autoproject.core (from which this is taken, in turn taken from some old code mkleint wrote...)

- fixes to all modules in main & contrib to replace

AC ac = prj.getLookup().lookup(AC.class);
if (ac != null) { ... }


AC ac = PU.getAC(prj);

and usage of the new spec version from projectapi correspondingly.
Comment 3 Milos Kleint 2008-06-03 08:09:40 UTC
MK1: I don't think you can implement the shared/public Aux Configuration via fileobject attribute. It will write
something down, but not in sharable manner, effectively breaking the contract.
Comment 4 Jesse Glick 2008-06-03 15:47:07 UTC
To MK1: of course it will not be shared in this case, but there is nothing to be done; the project provides no mechanism
to store this information sharably. Storing the information in the userdir is generally better than not storing it at
all. Looking at actual usages, most code assumes AC != null without even checking, and all calls to pCF(...,true) I can
find would either be OK if the data were not shared, or anyway make other assumptions about project structure (e.g. that
it is a j2seproject). Possible resolutions:

1. [preferred] Add a clarification to the Javadoc of PU.gAC that shared=true represents a best effort. Hypothetical code
which must fail if config cannot be stored sharably could continue to look for AC in lookup.

2. Add a boolean param to make calls to the fallback impl with shared=true throw an UnsupportedOperationException. Don't
like it much as it clutters the API with a confusing extra parameter, and does not provide any real value over #1.

3. Make the impl create some special file in the project dir, e.g. ".nbconfig.xml". I do not like this as it violates
the general principle in the project system of giving the project type complete control over its own disk
representation. Especially for autoproject it is desirable not to put any NetBeans-related junk in the source directory
unless preauthorized to do so.
Comment 5 Jan Lahoda 2008-06-03 17:38:11 UTC
Re MK1: I still think that providing non-sharable sharable settings without explicit request is not a very good idea.
What about something like PU.gAC(Project, boolean allowUnsharable), which would return null if not available (somewhat
similar to option 2)? BTW: I am not sure if conversion getLookup().lookup(AC.class) to PU.gAC would be correct for the
spellchecker module.

JL01: wouldn't it be good to make implementation of AC easier, so the project types themselves could easily provide a
fully-featured AC? (Something like support method: createAC(File shared, File priv), if priv==null, the settings would
be in the userdir.)

JL02: is there any estimate on the performance impact of storing "big" XMLs in attributes of FileObjects (both on the
performance of the AC itself and on the performance of calls to FO.getAttribute() on other FOs)?

JL03: when the IDE is upgraded (and a new userdir used), will the settings from the older version survive the upgrade?
If yes, and the project type would provide an AC in its lookup in the newer version, the old settings would be lost, right?
Comment 6 David Konecny 2008-06-03 23:26:34 UTC
What's the motivation behind this issue? I thought that all project types must have AU in lookup and that's why clients
do not have to check for null. I would rather clarify Javadoc in this respect then providing a fallback.

If it is difficult to implement AU then I agree with JL01 that helper implementation could be provided somewhere but it
would be up to project type to decide on suitability of it, e.g. AutoProjs might be fine with storing "sharable" data

Apart from that adding PU.getAC is good idea.
Comment 7 Jesse Glick 2008-06-04 06:28:29 UTC
To MK1 from JL: I still don't see the advantage of your #2a (as I will call it) over the simpler #1. Regarding
spellchecker, if it is undesirable to change it then I won't. Conversions must be done on a case-by-case basis
regardless of which option is chosen.

JL01: what is the purpose? Most project types will already be using AntProjectHelper for this.

JL02: no estimate; this is really up to the masterfs implementation. Currently it is a simple implementation using
attributes.xml which if needed could be migrated e.g. to NbPreferences. Note that this API change should not change the
actual behavior of the IDE; in particular the fallback impl would not be used for Ant-based projects.

JL03: It is up to the installer whether to import attributes.xml. I personally always recommend all contents of the
userdir to be imported across IDE releases but other people may decide to omit some items. If the project type switches
to a custom AC then I guess any previously stored settings would not be read, unless it decided to read FileObject
attributes for compatibility. If this is considered a problem the API method could easily return a proxy AC which tries
to read from old storage as well as the custom AC.

To DK motivation: no, there is no requirement that a project type have *anything* in its lookup, much less an AC. Any
code which looks for anything in project lookup must explicitly handle the case that there is no instance.

The fallback impl would be used by autoproject, perhaps also Mevenide (replacing M2AuxilaryConfigImpl), and in general
any other third-party non-Ant-based specialized project types for which a custom AC has no special value, or for which
the author simply did not bother to implement it due to time constraints. Same motivation as for other ProjectUtils
methods: provide a safe (null-free) API for callers with a minimal default implementation.
Comment 8 Milos Kleint 2008-06-04 07:05:47 UTC
JG: Mevenide's M2AuxilaryConfigImpl is using fileobject attributes for private aux config only, the shared items are
written to ${basedir}/nb-configuration.xml file. So it's not likely to be abandoned.
Comment 9 Milos Kleint 2008-06-04 10:05:50 UTC
+1 on having a support class/factory somewhere that would implement a simple AUX for projects that don't bother writing
their own.
Comment 10 Andrei Badea 2008-06-04 12:21:17 UTC
I do not support the proposal that shared=true be a best-effort contract. In the Spring module we need to store the list
of the Spring XML files and, most importantly, the groups of such files. These metadata affect the behavior of the IDE.
For example, if a group is not defined, the code completion will not provide the results that the user expects. So,
these metadata inherently belong to a Spring project just as the list of Java source folders belongs to a plain Java SE
project. It should be possible to check them into a source-control system, and an already-configured Spring project
checked out from the SCC should work just like on the computer from where it was checked into the SCC.

It makes sense to me to require projects to have AuxiliaryConfiguration in their lookup. There are (or we hope there
will be) more people implementing support for various technologies than project types. So the contracts should be biased
toward support implementors, not project type implementors.

For autoproject, I would consider it entirely understandable if a NetBeans-specific file is created in the project
directory once the user presses OK in the customizer (or as the result of, say, an UI action which generates an
artifact). We should try to avoid creating such a file too early or when it isn't really needed. But not providing
contracts is not a good way to enforce that.
Comment 11 Jesse Glick 2008-06-04 21:14:50 UTC
Created attachment 62387 [details]
Rewritten patch
Comment 12 Jesse Glick 2008-06-04 21:29:26 UTC
Perhaps the newly attached patch will be more to people's liking. Significant changes:

- MK1 etc. should be addressed: file attributes are used only for private storage; for shared storage, .netbeans.xml is
used (format similar to private.xml)

- separate file attributes are used for separate config fragments

- XML stored to file attributes is serialized in a simplified form (more friendly to attributes.xml or any other future
storage): no pretty printing, no XML declaration

- JL03 should be addressed: will read fallback-type storage even if a project adds AC to lookup

- Javadoc for AC mentions not to look for it directly

Missing pieces beyond those mentioned previously: expanded test to cover now more complex behavior; proper namespace for
.netbeans.xml root element (will need a trivial XML schema to permit subelements to be validated); pCF could try to
alphabetize contents of .netbeans.xml (as in AntProjectHelper.pCF).
Comment 13 Andrei Badea 2008-06-05 13:56:41 UTC
The solution in the rewritten patch looks fine to me.
Comment 14 Jesse Glick 2008-06-09 23:42:14 UTC
main #f9df73321192, contrib #6cce8ed7e572
Comment 15 Quality Engineering 2008-06-11 15:57:38 UTC
Integrated into 'main-golden', available in NB_Trunk_Production #251 build
Changeset: http://hg.netbeans.org/main/rev/f9df73321192
User: Jesse Glick <jglick@netbeans.org>
Log: Issue #136333: use an API/SPI split to provide a fallback AuxiliaryConfiguration implementation.
By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo