Skip to content

Commit

Permalink
Fixes and deprecation removal.
Browse files Browse the repository at this point in the history
Added code to "fix" #206.  This attempts to get in front of the underlying issue, but even if it does, the code traps the error and generates a nicer message.

Removed some deprecated code listed in #205.  Some of this deprecation remains for compatibility.

Fixed the gradle-wrapper.properties to reference the right URL.
  • Loading branch information
groboclown committed Nov 14, 2019
1 parent 660cecd commit 83e0629
Show file tree
Hide file tree
Showing 30 changed files with 167 additions and 88 deletions.
8 changes: 8 additions & 0 deletions .idea/dictionaries/albrechtm.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion idea-p4server/api/gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=http\://services.gradle.org/distributions/gradle-4.8.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
*/
public abstract class ProjectConfigRegistry
implements ProjectComponent, Disposable {
public static final String COMPONENT_NAME = ProjectConfigRegistry.class.getName();
public static final Class<ProjectConfigRegistry> COMPONENT_CLASS = ProjectConfigRegistry.class;
public static final String COMPONENT_NAME = COMPONENT_CLASS.getName();

private static final Logger LOG = Logger.getInstance(ProjectConfigRegistry.class);

Expand All @@ -59,7 +60,7 @@ public abstract class ProjectConfigRegistry

@Nullable
public static ProjectConfigRegistry getInstance(@NotNull Project project) {
return (ProjectConfigRegistry) project.getComponent(COMPONENT_NAME);
return (ProjectConfigRegistry) project.getComponent(COMPONENT_CLASS);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

/**
* Keeps a mapping between IDE ChangeList objects and Perforce changelist ID objects.
* This is maintained per-project, and the underlying state must be peristed between
* This is maintained per-project, and the underlying state must be persisted between
* executions.
* <p>
* The mapping is intentionally simple, and based on the server. The plugin will store
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,20 @@
package net.groboclown.p4.server.api.messagebus;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public class ErrorEvent<E extends Throwable> extends AbstractMessageEvent {
private final E error;
private final String message;

public ErrorEvent(@NotNull E error) {
this.error = error;
this.message = error.getMessage();
}

public ErrorEvent(@NotNull E error, @Nullable String message) {
this.error = error;
this.message = message;
}

@NotNull
Expand All @@ -29,6 +37,6 @@ public E getError() {
}

public String getMessage() {
return error.getMessage();
return message;
}
}
2 changes: 1 addition & 1 deletion idea-p4server/gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=http\://services.gradle.org/distributions/gradle-4.8.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=http\://services.gradle.org/distributions/gradle-4.8.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ public void setOpenedChanges(ClientServerRef ref,
}

// Cache store completed refresh. Send a notification.
// Actually, don't. This can cause unintended recursion during VCS refresh.
//FileCacheUpdatedMessage.send(project).onFilesCacheUpdated(new FileCacheUpdatedMessage.FileCacheUpdateEvent(
// openedFiles.toArray(new P4LocalFile[0])));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ public void setChangelists(P4LocalChangelist... changelists) {
this.changelists.addAll(Arrays.asList(changelists));
}

@TestOnly
public void setChangelists(Collection<P4LocalChangelist> changelists) {
this.changelists.clear();
this.changelists.addAll(changelists);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,20 +114,21 @@ public RepositoryLocation getChangedRepositoryPath() {
@Override
public byte[] loadContent()
throws IOException, VcsException {
return getContent();
}

@Nullable
@Override
public byte[] getContent()
throws IOException, VcsException {
if (!loadedContent && loader != null) {
loadedContent = true;
content = loader.loadContentForRev(config, clientname, data.getDepotFileName(), data.getRevision());
}
return content;
}

@Nullable
//@Override
@Deprecated
public byte[] getContent()
throws IOException, VcsException {
return loadContent();
}

@NotNull
@Override
public VcsRevisionNumber getRevisionNumber() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public static VirtualFile getDirectory(@NotNull Project project, @NotNull VcsDir
}
VirtualFile ret = VcsUtil.getVirtualFile(dir);
if (ret == null) {
return project.getBaseDir();
return ProjectUtil.guessProjectBaseDir(project);
}
return ret;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class ProjectConfigRegistryImplTest {
@Test
void getRegisteredClientConfig() {
ProjectConfigRegistry registry = new ProjectConfigRegistryImpl(idea.getMockProject());
idea.registerProjectComponent(ProjectConfigRegistry.COMPONENT_NAME, registry);
idea.registerProjectComponent(ProjectConfigRegistry.COMPONENT_CLASS, registry);

assertSame(ProjectConfigRegistry.getInstance(idea.getMockProject()), registry);
}
Expand Down
2 changes: 1 addition & 1 deletion idea-test-core/gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=http\://services.gradle.org/distributions/gradle-4.8.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion integration-test/gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=http\://services.gradle.org/distributions/gradle-4.8.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion p4d-test-core/gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=http\://services.gradle.org/distributions/gradle-4.8.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion p4java/gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=http\://services.gradle.org/distributions/gradle-4.8.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion plugin-v3/gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=http\://services.gradle.org/distributions/gradle-4.8.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ private Answer<Pair<IdeChangelistMap, IdeFileMap>> refreshServerOpenedCache(Coll
// Null project cannot have anything to refresh.
throw new IllegalStateException("project not set for call");
}
// Keep a chain of promises, headed by `ret`. The type of the
// answer doesn't matter, because it's only being used to tie the
// series of promises together.
Answer<?> ret = Answer.resolve(null);

for (ClientConfigRoot clientRoot : clients) {
Expand All @@ -157,12 +160,14 @@ private Answer<Pair<IdeChangelistMap, IdeFileMap>> refreshServerOpenedCache(Coll
: null;

ret = ret.mapAsync((x) ->
Answer.background((sink) -> P4ServerComponent.syncQuery(project,
Answer.background((sink) -> P4ServerComponent.syncQuery(
project,
clientRoot.getClientConfig(),
new SyncListOpenedFilesChangesQuery(
root,
UserProjectPreferences.getMaxChangelistRetrieveCount(project),
UserProjectPreferences.getMaxFileRetrieveCount(project))).getPromise()
UserProjectPreferences.getMaxFileRetrieveCount(project))
).getPromise()
.whenCompleted((changesResult) -> {
// TODO is this a duplicate call for the updateListener's CacheListener?
updateListener.setOpenedChanges(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,17 @@ private boolean processModifiedFileChange(ClientConfig config, IdeChangelistMap
}
builder.processModifiedWithoutCheckout(file.getFilePath().getVirtualFile());
return false;
} else if (ChangeListManager.getInstance(project).getChangeList(localChangeList.getId()) == null) {
}
// See #206
// There's a weird situation where the getChangeList(id) succeeds, but
// the underlying call is to an equivalent of findChangeList(name), which
// fails. This might even be a race condition. From what I can tell, the
// ChangeListManagerImpl has a ChangeListWorker, which knows about this
// changelist, but the UpdatingChangeListBuilder has a worker which does not.
//
LocalChangeList ideChangelist =
ChangeListManager.getInstance(project).findChangeList(localChangeList.getName());
if (ideChangelist == null) {
// This can happen after submit, and is a sign that the cache is out of date.
LOG.info("Encountered deleted changelist " + localChangeList +
"; cache is probably out of date and needs a refresh.");
Expand Down Expand Up @@ -574,6 +584,9 @@ private void markIgnored(Set<FilePath> dirtyFiles, ChangelistBuilder builder) {

// TODO unversioned files should be susceptible to the P4IGNORE settings.

// This is deprecated in >v193, which introduced the new method
// that takes a FilePath. Earlier versions don't have that
// method.
builder.processUnversionedFile(dirtyFile.getVirtualFile());
}
}
Expand Down Expand Up @@ -619,7 +632,23 @@ private void associateChangeWithList(ChangelistBuilder builder, Change change, L
LOG.debug("Adding change " + change + " to IDE change list " + localChangeList);
}

builder.processChangeInList(change, localChangeList, P4Vcs.getKey());
// #206 can't figure out the reason why the ChangeListManagerImpl would know about the change,
// while the builder's worker wouldn't know it.
try {
builder.processChangeInList(change, localChangeList, P4Vcs.getKey());
} catch (Throwable err) {
if (err.getClass().equals(Throwable.class)) {
// Chances are this is related to #206.
InternalErrorMessage.send(project).unexpectedError(new ErrorEvent<>(
err,
P4Bundle.message("error.changelist.bad-cache", localChangeList.getName())
));
// fallback...
builder.processChange(change, P4Vcs.getKey());
} else {
throw err;
}
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ public class P4CheckinEnvironment implements CheckinEnvironment, CommitExecutor
}

@Nullable
@Override
//@Override
@Deprecated
public RefreshableOnComponent createAdditionalOptionsPanel(CheckinProjectPanel panel, PairConsumer<Object, Object> additionalDataConsumer) {
// #52 - we could be able to monitor panel.getCommitMessage(); to ensure
// that there's a message, and when there isn't, disable the submit
Expand Down Expand Up @@ -114,7 +115,8 @@ public String getCheckinOperationName() {
}

@Nullable
@Override
//@Override
@Deprecated
public List<VcsException> commit(List<Change> changes, String preparedComment) {
return commit(changes, preparedComment, (p) -> null, new HashSet<>());
}
Expand Down Expand Up @@ -249,7 +251,8 @@ public String getActionText() {
}

@NotNull
@Override
//@Override
@Deprecated
public CommitSession createCommitSession() {
final SubmitModel model = new SubmitModel(project);
return new CommitSession() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void doCheckout(@NotNull Project project, @Nullable final Listener listen
@Override
public void run(@NotNull ProgressIndicator progressIndicator) {
progressIndicator.setIndeterminate(true);
progressIndicator.startNonCancelableSection();
progressIndicator.start();
try {
LOG.info("Fetching files into " + rootPath);
FetchFilesResult r = P4ServerComponent
Expand All @@ -96,10 +96,8 @@ public void run(@NotNull ProgressIndicator progressIndicator) {
} catch (InterruptedException e) {
InternalErrorMessage.send(project).cacheLockTimeoutError(new ErrorEvent<>(
new VcsInterruptedException(e)));
progressIndicator.finishNonCancelableSection();
onCancel();
} catch (P4CommandRunner.ServerResultException e) {
progressIndicator.finishNonCancelableSection();
VcsNotifier.getInstance(project).notifyError(P4Bundle.getString("checkout.config.error.title"),
e.getMessage());
synchronized (sync) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import net.groboclown.p4plugin.ui.ColorUtil;
import net.groboclown.p4plugin.ui.config.P4ProjectConfigurable;
import net.groboclown.p4plugin.ui.vcsroot.P4VcsRootConfigurable;
import net.groboclown.p4plugin.util.RootSettingsUtil;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -229,13 +230,7 @@ public String getDisplayName() {
*/
@Override
public UnnamedConfigurable getRootConfigurable(VcsDirectoryMapping mapping) {
if (mapping.getRootSettings() == null) {
VirtualFile rootDir = VcsUtil.getVirtualFile(mapping.getDirectory());
if (rootDir == null) {
rootDir = ProjectUtil.guessProjectBaseDir(getProject());
}
mapping.setRootSettings(new P4VcsRootSettingsImpl(myProject, rootDir));
}
RootSettingsUtil.getFixedRootSettings(myProject, mapping);
return new P4VcsRootConfigurable(getProject(), mapping);
}

Expand Down Expand Up @@ -449,28 +444,6 @@ public DiffProvider getDiffProvider() {
}


/**
* Returns true if the specified file path is located under a directory which is managed by this VCS.
* This method is called only for directories which are mapped to this VCS in the project configuration.
*
* @param filePath the path to check.
* @return true if the path is managed by this VCS, false otherwise.
*/
@Deprecated
//@Override
public boolean fileIsUnderVcs(FilePath filePath) {
// This does not check for ignored files, because
// it's possible for a file to be in the depot, and thus not ignored,
// but in the ignore list. The ignore list is only for ignoring
// new files.
return filePath != null &&
! filePath.isDirectory() &&
// Warning: for deleted files, fp.getPath() can be different than the actual file!!!!
// use this instead: getIOFile().getAbsolutePath()
! filePath.getIOFile().getAbsolutePath().contains("...");
}


@Override
public FileStatus[] getProvidedStatuses() {
return PREFERRED_STATUS;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,14 @@ public void popState() {

}

@Override
// @Override
@Deprecated
public void startNonCancelableSection() {

}

@Override
// @Override
@Deprecated
public void finishNonCancelableSection() {

}
Expand Down
Loading

0 comments on commit 83e0629

Please sign in to comment.