I spoke too soon.
I had overlooked that AggregatedUserLayoutManager also uses
GuidGenerator, in its implementation of IUserLayoutManager
getCacheKey(), which *is* used in the composition of the UserInstance
cache key.
So I scale back the proposal to elimination of the unused IUserLayout
method but not elimination of GuidGenerator (at least not in this context).
http://www.ja-sig.org/issues/browse/UP-1805
That brings me back to the synchronization bug in GuidGenerator. Is
there any reason layout manager cache keys need to be *globally*
unique? Or would a serial number do? Since the use for
IUserLayoutManager cache keys is to compose part of a UserInstance cache
key, it seems that a serial number changing with every change to the
state of *that* user layout manager instance would be sufficient, and
cheaper and simpler than generating a GUID.
private String constructCacheKey(IPerson person,String rootNodeId) throws
PortalException {
StringBuffer sbKey = new StringBuffer(1024);
sbKey.append(rootNodeId).append(",");
sbKey.append(uPreferencesManager.getUserPreferences().getCacheKey());
sbKey.append(uPreferencesManager.getUserLayoutManager().getCacheKey());
return sbKey.toString();
}
IUserLayout declares a method getCacheKey() that isn't called anywhere.
In AggregatedLayouts, this is implemented via a uPortal-custom
GuidGenerator class, in which I think I've found a threadsafety bug:
http://www.ja-sig.org/issues/browse/UP-1804
In SimpleLayout, getCacheKey always returns null.
Since the only use of GuidGenerator is for ALM implementation of
getCacheKey(), and since nothing calls getCacheKey(), my proposal at
this point is to eliminate all of these -- GuidGenerator, the
getCacheKey() method declaration on IUserLayout, the getCacheKey()
implementation in SimpleLayout, and the getCacheKey() implementation
in ALM.
http://www.ja-sig.org/issues/browse/UP-1805
Eliminating classes and interface methods in a patches branch is
normally not allowed in consideration of "backwards compatibility".
Given that nothing is calling this code and if anything did call this
code, it would get IDs of questionable uniqueness, how do we uPortal
developers feel about making an exception here and eliminating this
baggage in the active patches branches as well as /trunk?
Andrew
--
You are currently subscribed to [email protected] as: [EMAIL
PROTECTED]
To unsubscribe, change settings or access archives, see
http://www.ja-sig.org/wiki/display/JSG/uportal-dev
