geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jarek Gawor <jga...@gmail.com>
Subject Re: svn commit: r1127388 - in /geronimo/server/trunk/plugins: jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/ myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/
Date Wed, 25 May 2011 14:29:10 GMT
Since we are tracking 2 states now (ACTIVE or STARTING) and if a
bundle does not have any faces configuration, returning null in
addingBundle() would cause addingBundle() to be called twice for the
same bundle. So the bundle could be scanned twice needlessly... which
probably would happen in most cases anyway. Returning non-null
guarantees that the bundle is scanned only once. That also means that
the BundleTracker is tracking every bundle but that is not a problem
in this case.

Jarek

On Wed, May 25, 2011 at 1:59 AM, Ivan <xhhsld@gmail.com> wrote:
> Do we need to always return a non null value in the addingBundle
> method ? It looks to me that if no related files found in the target
> bundle, it is better to return null to tell the tracker to ignore it.
> Thanks.
>
> 2011/5/25  <gawor@apache.org>:
>> Author: gawor
>> Date: Wed May 25 05:25:34 2011
>> New Revision: 1127388
>>
>> URL: http://svn.apache.org/viewvc?rev=1127388&view=rev
>> Log:
>> GERONIMO-5938, GERONIMO-5976: Scan bundles for tag libs a little sooner to prevent
a potential race condition with WAB extender
>>
>> Modified:
>>    geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java
>>    geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java
>>    geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java
>>    geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java
>>
>> Modified: geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java
>> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java?rev=1127388&r1=1127387&r2=1127388&view=diff
>> ==============================================================================
>> --- geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java
(original)
>> +++ geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/Activator.java
Wed May 25 05:25:34 2011
>> @@ -33,7 +33,7 @@ public class Activator implements Bundle
>>         TldRegistryImpl tldRegistry = new TldRegistryImpl(context);
>>         tldRegistryRegistration = context.registerService(TldRegistry.class.getName(),
tldRegistry, null);
>>
>> -        tldBundleTracker = new BundleTracker(context, Bundle.ACTIVE, tldRegistry);
>> +        tldBundleTracker = new BundleTracker(context, Bundle.STARTING | Bundle.ACTIVE,
tldRegistry);
>>         tldBundleTracker.open();
>>     }
>>
>>
>> Modified: geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java
>> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java?rev=1127388&r1=1127387&r2=1127388&view=diff
>> ==============================================================================
>> --- geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java
(original)
>> +++ geronimo/server/trunk/plugins/jasper/geronimo-jasper/src/main/java/org/apache/geronimo/jasper/internal/TldRegistryImpl.java
Wed May 25 05:25:34 2011
>> @@ -109,22 +109,12 @@ public class TldRegistryImpl implements
>>     @Override
>>     public Object addingBundle(Bundle bundle, BundleEvent event) {
>>         Collection<TldProvider.TldEntry> tlds = scanBundle(bundle);
>> -        if (tlds.isEmpty()) {
>> -            return null;
>> -        } else {
>> -            map.put(bundle, tlds);
>> -            return bundle;
>> -        }
>> +        map.put(bundle, tlds);
>> +        return bundle;
>>     }
>>
>>     @Override
>>     public void modifiedBundle(Bundle bundle, BundleEvent event, Object object)
{
>> -        Collection<TldProvider.TldEntry> tlds = scanBundle(bundle);
>> -        if (tlds.isEmpty()) {
>> -            map.remove(bundle);
>> -        } else {
>> -            map.put(bundle, tlds);
>> -        }
>>     }
>>
>>     @Override
>> @@ -134,7 +124,7 @@ public class TldRegistryImpl implements
>>
>>     private Collection<TldProvider.TldEntry> scanBundle(Bundle bundle) {
>>         ServiceReference reference = bundleContext.getServiceReference(PackageAdmin.class.getName());
>> -        PackageAdmin packageAdmin = (PackageAdmin) bundle.getBundleContext().getService(reference);
>> +        PackageAdmin packageAdmin = (PackageAdmin) bundleContext.getService(reference);
>>         try {
>>             BundleResourceFinder resourceFinder = new BundleResourceFinder(packageAdmin,
bundle, "META-INF/", ".tld");
>>             TldResourceFinderCallback callback = new TldResourceFinderCallback();
>>
>> Modified: geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java
>> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java?rev=1127388&r1=1127387&r2=1127388&view=diff
>> ==============================================================================
>> --- geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java
(original)
>> +++ geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/Activator.java
Wed May 25 05:25:34 2011
>> @@ -62,7 +62,7 @@ public class Activator implements Bundle
>>         // register this as a service
>>         registryRegistration = context.registerService(ConfigRegistry.class.getName(),
registry, null);
>>
>> -           bt = new BundleTracker(context, Bundle.ACTIVE, new ConfigBundleTrackerCustomizer(this,
context.getBundle(), registry));
>> +           bt = new BundleTracker(context, Bundle.STARTING | Bundle.ACTIVE,
new ConfigBundleTrackerCustomizer(this, context.getBundle(), registry));
>>            bt.open();
>>        }
>>
>>
>> Modified: geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java
>> URL: http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java?rev=1127388&r1=1127387&r2=1127388&view=diff
>> ==============================================================================
>> --- geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java
(original)
>> +++ geronimo/server/trunk/plugins/myfaces/geronimo-myfaces/src/main/java/org/apache/geronimo/myfaces/config/resource/osgi/ConfigBundleTrackerCustomizer.java
Wed May 25 05:25:34 2011
>> @@ -55,7 +55,8 @@ public class ConfigBundleTrackerCustomiz
>>         if (bundle.equals(registryBundle)) {
>>             return null;
>>         }
>> -        return registry.addBundle(bundle) ? Boolean.TRUE : null;
>> +        registry.addBundle(bundle);
>> +        return bundle;
>>     }
>>
>>     @Override
>> @@ -64,7 +65,7 @@ public class ConfigBundleTrackerCustomiz
>>     }
>>
>>     @Override
>> -    public void removedBundle(Bundle bundle, BundleEvent event, Object object)
{
>> +    public void removedBundle(Bundle bundle, BundleEvent event, Object object)
{
>>         // have the registry process this
>>         registry.removeBundle(bundle, object);
>>     }
>>
>>
>>
>
>
>
> --
> Ivan
>

Mime
View raw message