Bug 560062 - Always load both artifact and meta repos
Also try to clean up after yourself when adding repos temporarily
(But see caveats in RepositoryTracker:init())
Change-Id: Ib970be0b63ccc54f457ba95a8dea023eb5c003da
Signed-off-by: Carsten Reckord <reckord@yatta.de>
diff --git a/org.eclipse.epp.mpc.ui/src/org/eclipse/epp/internal/mpc/ui/catalog/MarketplaceCatalog.java b/org.eclipse.epp.mpc.ui/src/org/eclipse/epp/internal/mpc/ui/catalog/MarketplaceCatalog.java
index 1a3ab6c..39fcb99 100644
--- a/org.eclipse.epp.mpc.ui/src/org/eclipse/epp/internal/mpc/ui/catalog/MarketplaceCatalog.java
+++ b/org.eclipse.epp.mpc.ui/src/org/eclipse/epp/internal/mpc/ui/catalog/MarketplaceCatalog.java
@@ -33,6 +33,7 @@
import org.eclipse.epp.internal.mpc.core.MarketplaceClientCore;
import org.eclipse.epp.internal.mpc.ui.MarketplaceClientUi;
import org.eclipse.epp.internal.mpc.ui.catalog.MarketplaceCategory.Contents;
+import org.eclipse.epp.internal.mpc.ui.operations.RepositoryTransactionHelper;
import org.eclipse.epp.internal.mpc.ui.util.ConcurrentTaskManager;
import org.eclipse.epp.mpc.core.model.ICategory;
import org.eclipse.epp.mpc.core.model.IMarket;
@@ -53,6 +54,7 @@
import org.eclipse.equinox.p2.query.IQuery;
import org.eclipse.equinox.p2.query.IQueryResult;
import org.eclipse.equinox.p2.query.QueryUtil;
+import org.eclipse.equinox.p2.repository.artifact.IArtifactRepositoryManager;
import org.eclipse.equinox.p2.repository.metadata.IMetadataRepository;
import org.eclipse.equinox.p2.repository.metadata.IMetadataRepositoryManager;
import org.eclipse.equinox.p2.ui.ProvisioningUI;
@@ -298,7 +300,17 @@
ConcurrentTaskManager executor = new ConcurrentTaskManager(installedCatalogItemsByUpdateUri.size(),
Messages.MarketplaceCatalog_checkingForUpdates);
- try {
+
+ ProvisioningSession session = ProvisioningUI.getDefaultUI().getSession();
+ IMetadataRepositoryManager metadataRepositoryManager = (IMetadataRepositoryManager) session
+ .getProvisioningAgent()
+ .getService(IMetadataRepositoryManager.SERVICE_NAME);
+ IArtifactRepositoryManager artifactRepositoryManager = (IArtifactRepositoryManager) session
+ .getProvisioningAgent()
+ .getService(IArtifactRepositoryManager.SERVICE_NAME);
+
+ try (RepositoryTransactionHelper repositories = new RepositoryTransactionHelper(metadataRepositoryManager,
+ artifactRepositoryManager)) {
final IProgressMonitor pm = new NullProgressMonitor() {
@Override
public boolean isCanceled() {
@@ -308,17 +320,30 @@
for (Map.Entry<URI, List<MarketplaceNodeCatalogItem>> entry : installedCatalogItemsByUpdateUri.entrySet()) {
final URI uri = entry.getKey();
final List<MarketplaceNodeCatalogItem> catalogItemsThisSite = entry.getValue();
+
+ //bug 560062 - add both artifact and metadata repo in case something breaks before we can clean up
+ repositories.addRepository(uri);
+
executor.submit(() -> {
- ProvisioningSession session = ProvisioningUI.getDefaultUI().getSession();
- IMetadataRepositoryManager manager = (IMetadataRepositoryManager) session.getProvisioningAgent()
- .getService(IMetadataRepositoryManager.SERVICE_NAME);
try {
for (MarketplaceNodeCatalogItem item1 : catalogItemsThisSite) {
if (Boolean.TRUE.equals(item1.getAvailable())) {
item1.setAvailable(null);
}
}
- IMetadataRepository repository = manager.loadRepository(uri, pm);
+ //bug 560062 - to be safe load the artifact repo as well, even though we don't need it here - and do it first because
+ //it's better to have artifact without meta than the other way around.
+ //see comments in RepositoryTransactionHelper.RepositoryTracker.init() for why we fully load artifact repos as well.
+ //TODO this needs a closer look - can we do without this without retriggering bug 560062? It's a serious performance killer...
+ artifactRepositoryManager.loadRepository(uri, pm);
+ if (pm.isCanceled()) {
+ return;
+ }
+ IMetadataRepository repository = metadataRepositoryManager.loadRepository(uri, pm);
+ if (pm.isCanceled()) {
+ return;
+ }
+
IQuery<IInstallableUnit> query = QueryUtil.createMatchQuery( //
"id ~= /*.feature.group/ && " + //$NON-NLS-1$
"properties['org.eclipse.equinox.p2.type.group'] == true ");//$NON-NLS-1$
diff --git a/org.eclipse.epp.mpc.ui/src/org/eclipse/epp/internal/mpc/ui/operations/RepositoryTransactionHelper.java b/org.eclipse.epp.mpc.ui/src/org/eclipse/epp/internal/mpc/ui/operations/RepositoryTransactionHelper.java
new file mode 100644
index 0000000..9e7eeb4
--- /dev/null
+++ b/org.eclipse.epp.mpc.ui/src/org/eclipse/epp/internal/mpc/ui/operations/RepositoryTransactionHelper.java
@@ -0,0 +1,198 @@
+/*******************************************************************************
+ * Copyright (c) 2018 The Eclipse Foundation and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v2.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * The Eclipse Foundation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.epp.internal.mpc.ui.operations;
+
+import java.net.URI;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+
+import org.eclipse.equinox.p2.repository.IRepositoryManager;
+import org.eclipse.equinox.p2.repository.artifact.IArtifactRepositoryManager;
+import org.eclipse.equinox.p2.repository.metadata.IMetadataRepositoryManager;
+
+public class RepositoryTransactionHelper implements AutoCloseable {
+
+ private final RepositoryTracker metadataRepositoryTracker;
+
+ private final RepositoryTracker artifactRepositoryTracker;
+
+ public RepositoryTransactionHelper(IMetadataRepositoryManager metadataRepositoryManager,
+ IArtifactRepositoryManager artifactRepositoryManager) {
+ metadataRepositoryTracker = metadataRepositoryManager == null ? null
+ : new RepositoryTracker(metadataRepositoryManager);
+ artifactRepositoryTracker = artifactRepositoryManager == null ? null
+ : new RepositoryTracker(artifactRepositoryManager);
+ }
+
+ public void init() {
+ if (metadataRepositoryTracker != null) {
+ metadataRepositoryTracker.init();
+ }
+ if (artifactRepositoryTracker != null) {
+ artifactRepositoryTracker.init();
+ }
+ }
+
+ public void addRepository(URI uri) {
+ //add artifact repo first, because if anything goes wrong it's better
+ //to have artifact without meta than the other way around
+ if (artifactRepositoryTracker != null) {
+ artifactRepositoryTracker.addRepository(uri);
+ }
+ if (metadataRepositoryTracker != null) {
+ metadataRepositoryTracker.addRepository(uri);
+ }
+ }
+
+ public void resetRepository(URI uri) {
+ if (metadataRepositoryTracker != null) {
+ metadataRepositoryTracker.resetRepository(uri);
+ }
+ if (artifactRepositoryTracker != null) {
+ artifactRepositoryTracker.resetRepository(uri);
+ }
+ }
+
+ public void resetAll() {
+ if (metadataRepositoryTracker != null) {
+ metadataRepositoryTracker.resetAll();
+ }
+ if (artifactRepositoryTracker != null) {
+ artifactRepositoryTracker.resetAll();
+ }
+ }
+
+ @Override
+ public void close() {
+ resetAll();
+ }
+
+ private enum CleanupAction {
+ REMOVE, DISABLE;
+ }
+
+ private static class RepositoryTracker implements AutoCloseable {
+ private static final int ENABLED_STATE_FLAGS = IRepositoryManager.REPOSITORIES_NON_LOCAL
+ | IRepositoryManager.REPOSITORIES_NON_SYSTEM;
+
+ private static final int DISABLED_STATE_FLAGS = IRepositoryManager.REPOSITORIES_DISABLED | ENABLED_STATE_FLAGS;
+
+ final IRepositoryManager<?> manager;
+
+ final Map<URI, CleanupAction> cleanupActions;
+
+ final Map<URI, Integer> initialState;
+
+ RepositoryTracker(IRepositoryManager<?> manager) {
+ this.manager = manager;
+ this.initialState = new HashMap<>();
+ this.cleanupActions = new LinkedHashMap<>();
+ init();
+ }
+
+ public void init() {
+ if (!cleanupActions.isEmpty()) {
+ throw new IllegalStateException();
+ }
+ initialState.clear();
+
+ /* TODO
+ *
+ * A lot of side effects are happening when repos are loaded - especially composites:
+ * - child repos get added, sometimes as system=true/enabled=false, or sometimes as system=false/enabled=false
+ * - repos might get enabled
+ * - repos might get converted from system=true to system=false, also changing the type from
+ * LocalMetadataRepository/simpleRepository to something else
+ *
+ * All of this makes it rather hopeless to try and revert the repo state completely...
+ * (For example, at the time of this writing, loading http://download.eclipse.org/mylyn/releases/latest
+ * also creates http://download.eclipse.org/mylyn/releases/3.25 and leaves it behind as system=false/enabled=false,
+ * adding it as a disabled repo to the visible repo list. It is hard to gauge if having those around in meta but not in
+ * artifact would be safe - see bug 560062 - so it's probably best to always fully load both repos)
+ *
+ * That's why we only track some of the known repos and only revert what we explicitly did.
+ *
+ * We could possibly add a listener for RepositoryEvent.ADDED and relatives, but we might get false positives if
+ * user does other repo things in parallel...
+ */
+ URI[] enabledRemoteRepos = manager.getKnownRepositories(ENABLED_STATE_FLAGS);
+ URI[] disabledRemoteRepos = manager.getKnownRepositories(DISABLED_STATE_FLAGS);
+
+ for (URI uri : enabledRemoteRepos) {
+ initialState.put(uri, ENABLED_STATE_FLAGS);
+ }
+ for (URI uri : disabledRemoteRepos) {
+ initialState.put(uri, DISABLED_STATE_FLAGS);
+ }
+ }
+
+ public void addRepository(URI uri) {
+ manager.addRepository(uri);
+ mergeAction(uri, getCleanupAction(uri));
+ }
+
+ private void mergeAction(URI uri, CleanupAction cleanupAction) {
+ if (cleanupAction != null) {
+ cleanupActions.put(uri, cleanupAction);
+ }
+ }
+
+ private CleanupAction getCleanupAction(URI uri) {
+ Integer state = initialState.get(uri);
+ if (state == null) {
+ //did not exist initially
+ return CleanupAction.REMOVE;
+ }
+ if ((state & IRepositoryManager.REPOSITORIES_DISABLED) != 0) {
+ //initially disabled
+ return CleanupAction.DISABLE;
+ }
+ return null;
+ }
+
+ public void resetRepository(URI uri) {
+ CleanupAction action = cleanupActions.remove(uri);
+ doResetRepository(uri, action);
+ }
+
+ private void doResetRepository(URI uri, CleanupAction action) {
+ if (action == null) {
+ return;
+ }
+ switch (action) {
+ case REMOVE:
+ manager.removeRepository(uri);
+ break;
+ case DISABLE:
+ manager.setEnabled(uri, false);
+ break;
+ default:
+ throw new UnsupportedOperationException(action.name());
+ }
+ }
+
+ public void resetAll() {
+ for (Entry<URI, CleanupAction> entry : cleanupActions.entrySet()) {
+ doResetRepository(entry.getKey(), entry.getValue());
+ }
+ cleanupActions.clear();
+ }
+
+ @Override
+ public void close() {
+ resetAll();
+ }
+ }
+}
