Skip to content

Commit

Permalink
LibraryManager didn't Correctly Handle Closing Projects
Browse files Browse the repository at this point in the history 
When closing project the library manager tried to resolve the
dependencies of the closed project which resulted in a broken
typelibrary instance. On project reopening this led to broken types.

Furthermore as part of this work we could remove a workspace job from
the resouce change listener making it cleaner and hopefully more stable.
  • Loading branch information
@azoitl @oberlehner
azoitl authored and oberlehner committed Jan 29, 2025
1 parent 9ec7331 commit f85e780
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 66 deletions.
14 changes: 10 additions & 4 deletions ...clipse.fordiac.ide.library/src/org/eclipse/fordiac/ide/library/LibraryChangeListener.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2024 Primetals Technologies Austria GmbH
* Copyright (c) 2024, 2025 Primetals Technologies Austria GmbH
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
Expand All @@ -22,7 +22,6 @@
import org.eclipse.core.resources.IResourceDelta;
import org.eclipse.core.resources.IResourceDeltaVisitor;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.fordiac.ide.model.typelibrary.TypeLibraryManager;
import org.eclipse.fordiac.ide.model.typelibrary.TypeLibraryTags;
import org.eclipse.fordiac.ide.ui.FordiacLogHelper;

Expand All @@ -45,7 +44,8 @@ public void resourceChanged(final IResourceChangeEvent event) {
private final IResourceDeltaVisitor visitor = delta -> {
switch (delta.getResource().getType()) {
case IResource.PROJECT:
return delta.getKind() != IResourceDelta.REMOVED; // ignore deleted projects
// ignore closed or deleted projects
return !(isProjectClosed(delta) || delta.getKind() == IResourceDelta.REMOVED);
case IResource.FILE:
// check manifest files
// information on previous linked status is not available on delete
Expand All @@ -56,7 +56,7 @@ public void resourceChanged(final IResourceChangeEvent event) {
|| ((file.isLinked() || delta.getKind() == IResourceDelta.REMOVED)
&& (delta.getKind() & MASK) != 0))) {
final IProject project = file.getProject();
LibraryManager.INSTANCE.startResolveJob(project, TypeLibraryManager.INSTANCE.getTypeLibrary(project));
LibraryManager.INSTANCE.startResolveJob(project);

}
return false;
Expand All @@ -73,6 +73,12 @@ public void resourceChanged(final IResourceChangeEvent event) {
return true;
};

private static boolean isProjectClosed(final IResourceDelta delta) {
return (delta.getKind() != IResourceDelta.CHANGED
&& (delta.getFlags() & IResourceDelta.OPEN) == IResourceDelta.OPEN
&& !delta.getResource().isAccessible());
}

private static boolean isTypeLibraryFolder(final IContainer container) {
return container instanceof IFolder && container.getParent() instanceof IProject
&& (TypeLibraryTags.STANDARD_LIB_FOLDER_NAME.equals(container.getName())
Expand Down
27 changes: 14 additions & 13 deletions ...s/org.eclipse.fordiac.ide.library/src/org/eclipse/fordiac/ide/library/LibraryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ public void importLibrary(final IProject project, final TypeLibrary typeLibrary,
addLibraryChangeListener();
// dependency resolution
if (resolve && imported) {
startResolveJob(project, typeLibrary);
startResolveJob(project);
}
}

Expand Down Expand Up @@ -599,28 +599,26 @@ public Map<String, List<LibraryRecord>> getExtractedLibraries() {
* <p>
* Will download/import libraries as needed through background jobs
*
* @param project selected project
* @param typeLibrary {@link TypeLibrary} to use
* @param project selected project
*/
public void checkManifestFile(final IProject project, final TypeLibrary typeLibrary) {
public void checkManifestFile(final IProject project) {
final Manifest manifest = ManifestHelper.getContainerManifest(project);
if (manifest == null || !ManifestHelper.isProject(manifest)) {
return;
}

ManifestHelper.sortAndSaveManifest(manifest);

startResolveJob(project, typeLibrary);
startResolveJob(project);
}

/**
* Start background job that resolves transitive library dependencies of
* {@link IProject}
*
* @param project selected project
* @param typeLibrary {@link TypeLibrary} to use
* @param project selected project
*/
public void startResolveJob(final IProject project, final TypeLibrary typeLibrary) {
public void startResolveJob(final IProject project) {
if (resolvingProjects.contains(project)) {
return;
}
Expand All @@ -630,7 +628,10 @@ public void startResolveJob(final IProject project, final TypeLibrary typeLibrar

@Override
public IStatus runInWorkspace(final IProgressMonitor monitor) throws CoreException {
resolveDependencies(project, typeLibrary);
if (project.isAccessible()) {
// only resolve if we have an accessible project
resolveDependencies(project, TypeLibraryManager.INSTANCE.getTypeLibrary(project));
}
return Status.OK_STATUS;
}

Expand Down Expand Up @@ -752,8 +753,8 @@ public void resolveDependencies(final IProject project, final TypeLibrary typeLi

checkLibChanges();

// remove when no longer needed
moveLinksToVirtualFolders(project, typeLibrary);
// TODO this is for migrating old projects: remove when no longer needed
moveLinksToVirtualFolders(project);

if (projectManifest.getDependencies() == null) {
return;
Expand Down Expand Up @@ -963,7 +964,7 @@ private ResolveNode resolveDependency(final String symbolicName, final VersionRa
* @param project selected project
* @param typeLibrary {@link TypeLibrary} to use
*/
private void moveLinksToVirtualFolders(final IProject project, final TypeLibrary typeLibrary) {
private void moveLinksToVirtualFolders(final IProject project) {
if (project.getFolder(TypeLibraryTags.STANDARD_LIB_FOLDER_NAME).exists()) {
return;
}
Expand Down Expand Up @@ -1103,7 +1104,7 @@ void stopEventBroker() {
private final EventHandler handler = event -> {
final Object data = event.getProperty(IEventBroker.DATA);
if (data instanceof final TypeLibrary typeLibrary) {
checkManifestFile(typeLibrary.getProject(), typeLibrary);
checkManifestFile(typeLibrary.getProject());
}
};

Expand Down
58 changes: 10 additions & 48 deletions ...rg/eclipse/fordiac/ide/systemmanagement/changelistener/FordiacResourceChangeListener.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2012, 2024 TU Wien ACIN, Profactor GmbH, fortiss GmbH,
* Copyright (c) 2012, 2025 TU Wien ACIN, Profactor GmbH, fortiss GmbH,
* Johannes Kepler University Linz,
* Primetals Technologies Austria GmbH,
* Martin Erich Jobst
Expand All @@ -20,7 +20,6 @@
*******************************************************************************/
package org.eclipse.fordiac.ide.systemmanagement.changelistener;

import java.text.MessageFormat;
import java.util.Collection;
import java.util.HashSet;
import java.util.Scanner;
Expand Down Expand Up @@ -147,12 +146,9 @@ private boolean handleResourceChanged(final IResourceDelta delta) {
}
break;
case IResource.PROJECT:
if (testFlags(delta, IResourceDelta.OPEN)) {
if (delta.getResource().isAccessible()) {
handleProjectAdd(delta);
} else {
handleProjectRemove(delta);
}
if (testFlags(delta, IResourceDelta.OPEN) && !delta.getResource().isAccessible()) {
// this is the odd way of Eclipse Platform telling us a project was closed
handleProjectRemove(delta);
return false;
}
break;
Expand Down Expand Up @@ -220,17 +216,10 @@ private boolean handleResourceCopy(final IResourceDelta delta) {
return false;
}

return switch (delta.getResource().getType()) {
case IResource.FILE -> {
if (delta.getResource().getType() == IResource.FILE) {
handleFileCopy(delta);
yield true;
}
case IResource.PROJECT -> {
handleProjectAdd(delta);
yield false;
}
default -> true;
};
return true;
}

private void handleFileDelete(final IResourceDelta delta) {
Expand Down Expand Up @@ -270,9 +259,8 @@ private void handleFileCopy(final IResourceDelta delta) {
final TypeEntry entry = typeLib.createTypeEntry(file);
if (null != entry && containedTypeNameIsDifferent(file)) {
// we only need to update the type entry if the file content is different from
// the file name
// this happens when a type is copied into a new project or when a project is
// opened or imported
// the file name this happens when a type is copied into a new project or when a
// project is opened or imported
updateTypeEntry(file, entry);
return;
}
Expand All @@ -284,9 +272,8 @@ private void handleFileCopy(final IResourceDelta delta) {
}
} else if (!file.equals(typeEntryForFile.getFile())) {
// After a file has been copied and the copied file is not the same as the
// founded type entry
// the file and the resulting type must be renamed with a unique name put it in
// the rename list
// founded type entry the file and the resulting type must be renamed with a
// unique name put it in the rename list
filesToRename.add(new FileToRenameEntry(file, typeEntryForFile));
}
}
Expand Down Expand Up @@ -510,31 +497,6 @@ public IStatus runInWorkspace(final IProgressMonitor monitor) throws CoreExcepti
job.schedule();
}

private void handleProjectAdd(final IResourceDelta delta) {
final IProject project = delta.getResource().getProject();
ResourcesPlugin.getWorkspace().removeResourceChangeListener(this);

final WorkspaceJob job = new WorkspaceJob(MessageFormat
.format(Messages.FordiacResourceChangeListener_UpdateTypeLibForNewProject, project.getName())) {
@Override
public IStatus runInWorkspace(final IProgressMonitor monitor) {
try {
// ensure dirty workspaces are cleaned before any type library is loaded
project.refreshLocal(IResource.DEPTH_INFINITE, monitor);
} catch (final CoreException e) {
FordiacLogHelper.logError(e.getMessage(), e);
}
ResourcesPlugin.getWorkspace().addResourceChangeListener(FordiacResourceChangeListener.this);
return Status.OK_STATUS;
}
};
job.setUser(false);
job.setSystem(true);
job.setPriority(Job.INTERACTIVE); // give this job the highest possible priority
job.setRule(project);
job.schedule();
}

private static void handleProjectRemove(final IResourceDelta delta) {
final IProject project = delta.getResource().getProject();
closeAllProjectRelatedEditors(project);
Expand Down
2 changes: 1 addition & 1 deletion ....fordiac.ide.test.library/src/org/eclipse/fordiac/ide/test/library/LibraryImportTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ void testCheckManifest() throws Exception {
ManifestHelper.addDependency(manifest, ManifestHelper.createRequired(TEST01, "[1.0.0-2.0.0)")); //$NON-NLS-1$
ManifestHelper.saveManifest(manifest);

LibraryManager.INSTANCE.checkManifestFile(project, TypeLibraryManager.INSTANCE.getTypeLibrary(project));
LibraryManager.INSTANCE.checkManifestFile(project);
Job.getJobManager().join(LibraryManager.FAMILY_FORDIAC_LIBRARY, null);

manifest = ManifestHelper.getContainerManifest(project);
Expand Down

0 comments on commit f85e780

Please sign in to comment.