Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WorkbenchServiceRegistry: Use generics instead of raw types #836

Merged
merged 1 commit into from
Jun 20, 2023
+13 −14
Merged

WorkbenchServiceRegistry: Use generics instead of raw types #836

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions ...se.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/WorkbenchServiceRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -84,34 +85,34 @@ public boolean hasService(Class<?> api) {
}
};

private Map factories = new HashMap();
private final Map<String, ServiceFactoryHandle> factories = new HashMap<>();

static class ServiceFactoryHandle {
AbstractServiceFactory factory;
WeakHashMap serviceLocators = new WeakHashMap();
Copy link
Contributor

Choose a reason for hiding this comment

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

was there a good reason to use WeakHashMap instead of a Set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't know. As the Name says it is weak -i.e. it can be GCed. probably on purpose

Copy link
Contributor

Choose a reason for hiding this comment

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

And now it can't be GCed any more. Is that an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why could'nt it? its still a WeakHashMap under the hood

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. Sure. I didn't see that. Sorry for the confusion.

final Set<ServiceLocator> serviceLocators = Collections.newSetFromMap(new WeakHashMap<>());
String[] serviceNames;

ServiceFactoryHandle(AbstractServiceFactory factory) {
this.factory = factory;
}
}

public Object getService(Class key, IServiceLocator parentLocator, ServiceLocator locator) {
ServiceFactoryHandle handle = (ServiceFactoryHandle) factories.get(key.getName());
public Object getService(Class<?> key, IServiceLocator parentLocator, ServiceLocator locator) {
ServiceFactoryHandle handle = factories.get(key.getName());
if (handle == null) {
handle = loadFromRegistry(key);
}
if (handle != null) {
Object result = handle.factory.create(key, parentLocator, locator);
if (result != null) {
handle.serviceLocators.put(locator, new Object());
handle.serviceLocators.add(locator);
return result;
}
}
return null;
}

private ServiceFactoryHandle loadFromRegistry(Class key) {
private ServiceFactoryHandle loadFromRegistry(Class<?> key) {
ServiceFactoryHandle result = null;
IConfigurationElement[] serviceFactories = getExtensionPoint().getConfigurationElements();
try {
Expand All @@ -134,7 +135,7 @@ private ServiceFactoryHandle loadFromRegistry(Class key) {
PlatformUI.getWorkbench().getExtensionTracker().registerObject(
serviceFactories[i].getDeclaringExtension(), handle, IExtensionTracker.REF_WEAK);

List serviceNames = new ArrayList();
List<String> serviceNames = new ArrayList<>();
for (IConfigurationElement configElement : serviceNameElements) {
String serviceName = configElement.getAttribute(IWorkbenchRegistryConstants.ATTR_SERVICE_CLASS);
if (factories.containsKey(serviceName)) {
Expand All @@ -145,7 +146,7 @@ private ServiceFactoryHandle loadFromRegistry(Class key) {
serviceNames.add(serviceName);
}
}
handle.serviceNames = (String[]) serviceNames.toArray(new String[serviceNames.size()]);
handle.serviceNames = serviceNames.toArray(new String[serviceNames.size()]);
result = handle;
}
}
Expand All @@ -161,7 +162,7 @@ private IExtensionPoint getExtensionPoint() {
}

public AbstractSourceProvider[] getSourceProviders() {
ArrayList providers = new ArrayList();
ArrayList<AbstractSourceProvider> providers = new ArrayList<>();
IExtensionPoint ep = getExtensionPoint();
for (IConfigurationElement configElement : ep.getConfigurationElements()) {
if (configElement.getName().equals(IWorkbenchRegistryConstants.TAG_SOURCE_PROVIDER)) {
Expand All @@ -176,14 +177,14 @@ public AbstractSourceProvider[] getSourceProviders() {
WorkbenchPlugin.log(status);
continue;
}
providers.add(sourceProvider);
providers.add((AbstractSourceProvider) sourceProvider);
processVariables(configElement.getChildren(IWorkbenchRegistryConstants.TAG_VARIABLE));
} catch (CoreException e) {
StatusManager.getManager().handle(e.getStatus());
}
}
}
return (AbstractSourceProvider[]) providers.toArray(new AbstractSourceProvider[providers.size()]);
return providers.toArray(AbstractSourceProvider[]::new);
}

private static final String[] supportedLevels = { ISources.ACTIVE_CONTEXT_NAME, ISources.ACTIVE_SHELL_NAME,
Expand Down Expand Up @@ -229,9 +230,7 @@ public void removeExtension(IExtension extension, Object[] objects) {
for (Object object : objects) {
if (object instanceof ServiceFactoryHandle) {
ServiceFactoryHandle handle = (ServiceFactoryHandle) object;
Set locatorSet = handle.serviceLocators.keySet();
ServiceLocator[] locators = (ServiceLocator[]) locatorSet
.toArray(new ServiceLocator[locatorSet.size()]);
ServiceLocator[] locators = handle.serviceLocators.toArray(ServiceLocator[]::new);
Arrays.sort(locators, (loc1, loc2) -> {
int l1 = loc1.getService(IWorkbenchLocationService.class).getServiceLevel();
int l2 = loc2.getService(IWorkbenchLocationService.class).getServiceLevel();
Expand Down