Community
Participate
Working Groups
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.
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?
Created attachment 194469 [details] Synced patch #1 with HEAD
Applied patch #2 to CVS HEAD
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
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
Restructured code that determines, finds and buffers AdapterFactories Changes are in CVS HEAD
