Skip to content

Commit

Permalink
Avoiding storing references to Imported<ResourceGenerator>
Browse files Browse the repository at this point in the history
  • Loading branch information
@gastaldi
gastaldi committed Feb 7, 2014
1 parent 0a280d8 commit 426f0c7
Showing 1 changed file with 1 addition and 5 deletions.
6 changes: 1 addition & 5 deletions resources/impl/src/main/java/org/jboss/forge/addon/resource/ResourceFactoryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ public class ResourceFactoryImpl implements ResourceFactory
@Inject
private FileResourceTransactionManager transactionManager;

private Imported<ResourceGenerator<?, ?>> instances;

@Override
@SuppressWarnings({ "unchecked", "rawtypes" })
public <E, T extends Resource<E>> T create(final Class<T> type, final E underlyingResource)
Expand All @@ -53,9 +51,7 @@ public <E, T extends Resource<E>> T create(final Class<T> type, final E underlyi
TreeMap<Class<?>, ResourceGenerator> generated = new TreeMap<>(
new RelatedClassComparator());

// FIXME Workaround for FORGE-1263
if (instances == null)
instances = (Imported) registry.getServices(ResourceGenerator.class);
Imported<ResourceGenerator> instances = registry.getServices(ResourceGenerator.class);

for (ResourceGenerator generator : instances)
{
Expand Down

5 comments on commit 426f0c7

@lincolnthree
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? It may result in performance issues. (Not certain... I need to do some profiling.) The Imported object should automatically refresh itself with new instances when new services become available/unavailable.

@gastaldi
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I know. I was experiencing some sporadic ClassCastExceptions when the instances were iterated and apparently the errors are gone with this change.

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented on 426f0c7 Feb 8, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gastaldi
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CCE was not due to classloader issues, but totally different classes. Here is one of them:

java.lang.ClassCastException: org.jboss.forge.addon.environment.impl.EnvironmentImpl_$$_javassist_7d1146f1-76f5-4455-873f-df536aa5c84e cannot be cast to org.jboss.forge.addon.resource.ResourceGenerator}}

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented on 426f0c7 Feb 8, 2014 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.