Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 342302

Summary:

Move AdapterFactory into application scope

Product:

[RT] RAP

Reporter:

Rüdiger Herrmann <ruediger.herrmann>

Component:

RWT

Assignee:

Project Inbox <rap-inbox>

Status:

RESOLVED FIXED

QA Contact:

Severity:

enhancement

Priority:

CC:

fr.appel

Version:

unspecified

Flags:

rsternberg: review+
fr.appel: review+

Target Milestone:

1.4 RC1

Hardware:

All

OS:

All

Whiteboard:

Bug Depends on:

Bug Blocks:

341761

Attachments:

Description	Flags
Patch that moves AdapterFactory/Manager to application scope	none
Synced patch #1 with HEAD	none

Description Rüdiger Herrmann

2011-04-08 10:43:07 EDT

Currently, the AdapterFactory (through its use in the AdapterManager) has session scope. Letting the AdapterFactory have application scope would have several advantages
* the LifeCycleAdapterFactory (currently the only implementation) could hold their buffers as instance fields (see bug 342131)
* the AdapterFactoryRegistry would become obsolete, AdapterFactories could be directly instantiated and registered at the AdapterManager
* future AdapterFactories that hold state, would have easier scope management as they live in the outermost scope. If another scope (i.e. session scope) is desired, this can be handled cleanly from the AdaptreFactory implementation.

It is very unlikely that this change will break clients as I highly doubt that anyone outside RWT uses this feature.

Comment 1 Rüdiger Herrmann

2011-04-25 08:24:02 EDT

Created attachment 193984 [details]
Patch that moves AdapterFactory/Manager to application scope

Changes:
* fomerly the AdapterFactoryRegistry buffered pairs of AdapterFactory and Adaptables. With this buffer it was able to prevented that a duplicate AdapterFactory - Adaptable pair was added. As AdapterFactories are now added directly at the AdapterManager, the AdapterFactoryRegistry has become obsolete. With removing the AdapterFactoryRegistry, the check for duplicates is gone too. I consider this a minor tradeoff as this check was incomplete anyway. A thorough check that ensures that no conficts occur for any added trio of AdapterFactory, its adapterList and Adaptables can still be addedd later.
* as AdapterManagerImpl has moved from session scope to application scope, it has to be thread safe
* removed AdapterManagerImpl#deregisterAdapters() as a) it was not used (except for a test) and b) the AdapterFactoryRegistry didn't allow either to deregister previously registered factories. In essence, this change does not disable any publicly available functionality.

TODO:
* Move AdapterManager_Test into correct (internal) package and rename to AdapterManagerImpl_Test
* restructuring the AdapterManagerImpl internally and other refactorings were left for later to make comparing the patch against HEAD easier
* remove AdapterManager interface, I don't see a reason for the internal class to have an interface. Postponed to keep the patch small.
* test that ensures thread safety of AdapterManagerImpl

Could anyone review these changes?

Comment 2 Rüdiger Herrmann

2011-05-02 05:34:54 EDT

Created attachment 194469 [details]
Synced patch #1 with HEAD

Comment 3 Rüdiger Herrmann

2011-05-02 11:15:50 EDT

Applied patch #2 to CVS HEAD

Comment 4 Rüdiger Herrmann

2011-05-02 11:42:05 EDT

Committed the following refactorings:
* moved AdapterManager_Test into org.eclipse.rwt.internal package and renamed it to AdapterManagerImpl_Test
* removed AdapterManager interface and renamed AdapterManagerImpl to AdapterManager

Comment 5 Rüdiger Herrmann

2011-05-02 13:55:14 EDT

Committed some more refactorings:
* removed AdapterFactoryCreator as its checks don't add much value in comparison to directly instantiating a AdapterFactory and getting an exception from ClassUtil#newInstance()
* removed AdapterManager#registerAdapterFactory(Class,Class), now the RWTServletcontextListener and EngineConfigWrapper create instances from AdapterFactory classes themselves
* extracted the code that maintains the map of adaptables and its AdapterFactories from AdapterManager into a class of its own: AdapterFactoryRegistry

Comment 6 Rüdiger Herrmann

2011-05-03 05:39:09 EDT

Restructured code that determines, finds and buffers AdapterFactories
Changes are in CVS HEAD