Skip to content

Commit

Permalink
Revert IFile.create(...REPLACE) - fixes #1433 (#1434)
Browse files Browse the repository at this point in the history 
Revert IFile.create(...REPLACE) - fixes #1433

Allows to use modifyRule for IFile.write() if the file already exits, so
that every IFile.setContents() can be replaced with IFile.create().

#1433
  • Loading branch information
@jukzi
jukzi committed Jun 19, 2024
1 parent 2b663cf commit e639885
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 30 deletions.
2 changes: 1 addition & 1 deletion ...se.core.resources/src/org/eclipse/core/internal/localstore/FileSystemResourceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,7 @@ private void prepareWrite(IFile target, IFileInfo fileInfo, int updateFlags, boo
throw new ResourceException(IResourceStatus.NOT_FOUND_LOCAL, target.getFullPath(), message, null);
}
} else {
if (!BitMask.isSet(updateFlags, IResource.REPLACE) && fileInfo.exists()) {
if (fileInfo.exists()) {
String message = NLS.bind(Messages.localstore_resourceExists, target.getFullPath());
throw new ResourceException(IResourceStatus.EXISTS_LOCAL, target.getFullPath(), message, null);
}
Expand Down
20 changes: 13 additions & 7 deletions ...rces/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/File.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public void create(InputStream content, int updateFlags, IProgressMonitor monito
final ISchedulingRule rule = workspace.getRuleFactory().createRule(this);
try {
workspace.prepareOperation(rule, subMonitor.newChild(1));
checkCreatable(updateFlags);
checkCreatable();
workspace.beginOperation(true);
IFileStore store = getStore();
IFileInfo localInfo = create(updateFlags, subMonitor.newChild(40), store);
Expand Down Expand Up @@ -172,7 +172,7 @@ public void create(byte[] content, int updateFlags, IProgressMonitor monitor) th
final ISchedulingRule rule = workspace.getRuleFactory().createRule(this);
try {
workspace.prepareOperation(rule, subMonitor.newChild(1));
checkCreatable(updateFlags);
checkCreatable();
workspace.beginOperation(true);
IFileStore store = getStore();
IFileInfo localInfo = create(updateFlags, subMonitor.newChild(40), store);
Expand Down Expand Up @@ -202,10 +202,8 @@ public void create(byte[] content, int updateFlags, IProgressMonitor monitor) th
}
}

private void checkCreatable(int updateFlags) throws CoreException {
if ((updateFlags & IResource.REPLACE) == 0) {
checkDoesNotExist();
}
private void checkCreatable() throws CoreException {
checkDoesNotExist();
Container parent = (Container) getParent();
ResourceInfo info = parent.getResourceInfo(false, false);
parent.checkAccessible(getFlags(info));
Expand All @@ -232,7 +230,7 @@ private IFileInfo create(int updateFlags, SubMonitor subMonitor, IFileStore stor
}
}
} else {
if (!BitMask.isSet(updateFlags, IResource.REPLACE) && localInfo.exists()) {
if (localInfo.exists()) {
// return an appropriate error message for case variant collisions
if (!Workspace.caseSensitive) {
String name = getLocalManager().getLocalName(store);
Expand Down Expand Up @@ -433,6 +431,10 @@ public void setContents(InputStream content, int updateFlags, IProgressMonitor m
checkAccessible(getFlags(info));
workspace.beginOperation(true);
IFileInfo fileInfo = getStore().fetchInfo();
if (BitMask.isSet(updateFlags, IResource.DERIVED)) {
// update of derived flag during IFile.write:
info.set(ICoreConstants.M_DERIVED);
}
internalSetContents(content, fileInfo, updateFlags, false, subMonitor.newChild(99));
} catch (OperationCanceledException e) {
workspace.getWorkManager().operationCanceled();
Expand Down Expand Up @@ -461,6 +463,10 @@ public void setContents(byte[] content, int updateFlags, IProgressMonitor monito
checkAccessible(getFlags(info));
workspace.beginOperation(true);
IFileInfo fileInfo = getStore().fetchInfo();
if (BitMask.isSet(updateFlags, IResource.DERIVED)) {
// update of derived flag during IFile.write:
info.set(ICoreConstants.M_DERIVED);
}
internalSetContents(content, fileInfo, updateFlags, false, subMonitor.newChild(99));
} catch (OperationCanceledException e) {
workspace.getWorkManager().operationCanceled();
Expand Down
37 changes: 17 additions & 20 deletions resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/resources/IFile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,13 +304,6 @@ public interface IFile extends IResource, IEncodedStorage, IAdaptable {
* with a value of <code>true</code> immediately after creating the resource.
* </p>
* <p>
* The {@link IResource#KEEP_HISTORY} update flag indicates that the history of this file is kept if any
* </p>
* <p>
* The {@link IResource#REPLACE} update flag indicates that this resource is overridden if already existing.
*
* </p>
* <p>
* The {@link IResource#TEAM_PRIVATE} update flag indicates that this resource
* should immediately be set as a team private resource. Specifying this flag
* is equivalent to atomically calling {@link IResource#setTeamPrivateMember(boolean)}
Expand Down Expand Up @@ -338,9 +331,7 @@ public interface IFile extends IResource, IEncodedStorage, IAdaptable {
* @param source an input stream containing the initial contents of the file,
* or <code>null</code> if the file should be marked as not local
* @param updateFlags bit-wise or of update flag constants
* ({@link IResource#FORCE}, {@link IResource#DERIVED},
* {@link IResource#KEEP_HISTORY}, {@link IResource#REPLACE},
* and {@link IResource#TEAM_PRIVATE})
* ({@link IResource#FORCE}, {@link IResource#DERIVED}, and {@link IResource#TEAM_PRIVATE})
* @param monitor a progress monitor, or <code>null</code> if progress
* reporting is not desired
* @exception CoreException if this method fails. Reasons include:
Expand Down Expand Up @@ -400,9 +391,7 @@ public default void create(byte[] content, boolean force, boolean derived, IProg
*
* @param content the content as byte array
* @param createFlags bit-wise or of flag constants ({@link IResource#FORCE},
* {@link IResource#DERIVED},
* {@link IResource#KEEP_HISTORY},
* {@link IResource#REPLACE}, and
* {@link IResource#DERIVED}, and
* {@link IResource#TEAM_PRIVATE})
* @param monitor a progress monitor, or <code>null</code> if progress
* reporting is not desired
Expand All @@ -416,17 +405,19 @@ public default void create(byte[] content, int createFlags, IProgressMonitor mon
/**
* Creates the file and sets the file content. If the file already exists in
* workspace its content is replaced. Shortcut for calling
* {@link #create(byte[], int, IProgressMonitor)} with flags depending on the
* boolean parameters given and {@link IResource#REPLACE}.
* {@link #setContents(byte[], boolean, boolean, IProgressMonitor)} or
* {@link #create(byte[], boolean, boolean, IProgressMonitor)} if the file does
* not {@link #exists()}.
*
* @param content the new content bytes. Must not be null.
* @param force a flag controlling how to deal with resources that are not
* in sync with the local file system
* @param derived Specifying this flag is equivalent to atomically calling
* {@link IResource#setDerived(boolean)} immediately after
* creating the resource or non-atomically setting the
* derived flag before setting the content of an already
* existing file
* creating the resource or atomically setting the derived
* flag before setting the content of an already existing
* file if derived==true. A value of false will not update
* the derived flag of an existing file.
* @param keepHistory a flag indicating whether or not store the current
* contents in the local history if the file did already
* exist
Expand All @@ -438,8 +429,14 @@ public default void create(byte[] content, int createFlags, IProgressMonitor mon
public default void write(byte[] content, boolean force, boolean derived, boolean keepHistory,
IProgressMonitor monitor) throws CoreException {
Objects.requireNonNull(content);
create(content, (force ? IResource.FORCE : 0) | (derived ? IResource.DERIVED : 0)
| (keepHistory ? IResource.KEEP_HISTORY : 0) | IResource.REPLACE, monitor);
if (exists()) {
int updateFlags = (derived ? IResource.DERIVED : IResource.NONE)
| (force ? IResource.FORCE : IResource.NONE)
| (keepHistory ? IResource.KEEP_HISTORY : IResource.NONE);
setContents(content, updateFlags, monitor);
} else {
create(content, force, derived, monitor);
}
}

/**
Expand Down
44 changes: 42 additions & 2 deletions ...ests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/IFileTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.core.resources.IContainer;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IFileState;
Expand All @@ -53,7 +54,9 @@
import org.eclipse.core.resources.IResourceChangeEvent;
import org.eclipse.core.resources.IResourceDelta;
import org.eclipse.core.resources.IResourceStatus;
import org.eclipse.core.resources.IWorkspace;
import org.eclipse.core.resources.IWorkspaceDescription;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IProgressMonitor;
Expand Down Expand Up @@ -497,23 +500,60 @@ public void testWrite() throws CoreException {
boolean setDerived = i % 2 == 0;
boolean deleteBefore = (i >> 1) % 2 == 0;
boolean keepHistory = (i >> 2) % 2 == 0;
boolean oldDerived1 = false;
if (deleteBefore) {
derived.delete(false, null);
} else {
oldDerived1 = derived.isDerived();
}
assertEquals(!deleteBefore, derived.exists());
FussyProgressMonitor monitor = new FussyProgressMonitor();
AtomicInteger changeCount = new AtomicInteger();
ResourcesPlugin.getWorkspace().addResourceChangeListener(event -> changeCount.incrementAndGet());
derived.write(("updateOrCreate" + i).getBytes(), false, setDerived, keepHistory, monitor);
assertEquals("not atomic", 1, changeCount.get());
monitor.assertUsedUp();
assertEquals(setDerived, derived.isDerived());
if (deleteBefore) {
assertEquals(setDerived, derived.isDerived());
} else {
assertEquals(oldDerived1 || setDerived, derived.isDerived());
}
assertFalse(derived.isTeamPrivateMember());
assertTrue(derived.exists());

IFileState[] history1 = derived.getHistory(null);
changeCount.set(0);
derived.write(("update" + i).getBytes(), false, false, keepHistory, null);
boolean oldDerived2 = derived.isDerived();
assertEquals(oldDerived2, derived.isDerived());
assertEquals("not atomic", 1, changeCount.get());
IFileState[] history2 = derived.getHistory(null);
assertEquals(keepHistory ? 1 : 0, history2.length - history1.length);
assertEquals((keepHistory && !oldDerived2) ? 1 : 0, history2.length - history1.length);
}
}

@Test
public void testWriteRule() throws CoreException {
IFile resource = projects[0].getFile("derived.txt");
createInWorkspace(projects[0]);
IWorkspace workspace = ResourcesPlugin.getWorkspace();
resource.delete(false, null);
AtomicInteger changeCount = new AtomicInteger();
ResourcesPlugin.getWorkspace().addResourceChangeListener(event -> changeCount.incrementAndGet());
workspace.run(pm -> {
resource.write(("create").getBytes(), false, false, false, null);
}, workspace.getRuleFactory().createRule(resource), IWorkspace.AVOID_UPDATE, null);
assertTrue(resource.exists());
assertEquals("not atomic", 1, changeCount.get());
// test that modifyRule can be used for IFile.write() if the file already exits:
changeCount.set(0);
workspace.run(pm -> {
resource.write(("replace").getBytes(), false, false, false, null);
}, workspace.getRuleFactory().modifyRule(resource), IWorkspace.AVOID_UPDATE, null);
assertTrue(resource.exists());
assertEquals("not atomic", 1, changeCount.get());
}

@Test
public void testDeltaOnCreateDerived() throws CoreException {
IFile derived = projects[0].getFile("derived.txt");
Expand Down

0 comments on commit e639885

Please sign in to comment.