From 255a1e4366c77affdb87154b8050b64f0b76dea0 Mon Sep 17 00:00:00 2001 From: Groboclown Date: Tue, 9 May 2017 14:41:16 -0500 Subject: [PATCH] Lots of miscellaneous bug fixes, and additional logging to help pin down some connection issues. --- CHANGES.md | 14 ++-- plugin/META-INF/plugin.xml | 12 ++- plugin/plugin.iml | 2 +- .../groboclown/idea/p4ic/P4Bundle.properties | 4 +- .../idea/p4ic/config/ClientConfig.java | 14 +++- .../idea/p4ic/config/ClientConfigSetup.java | 7 +- .../p4ic/config/P4ProjectConfigStack.java | 30 ++++--- .../idea/p4ic/config/ServerConfig.java | 10 +++ .../p4ic/config/part/MultipleDataPart.java | 41 +++++++++- .../exceptions/OfflineRevertException.java | 23 ++++++ .../ui/config/ResolvedPropertiesPanel.java | 10 +++ .../idea/p4ic/v2/server/P4Server.java | 77 +++++++++--------- .../idea/p4ic/v2/server/cache/sync/Cache.java | 8 +- .../server/cache/sync/ClientCacheManager.java | 8 +- .../sync/FileActionsServerCacheSync.java | 10 +++ .../cache/sync/WorkspaceServerCacheSync.java | 27 ++++--- .../connection/AuthenticatedServer.java | 12 ++- .../p4ic/v2/server/connection/ClientExec.java | 3 +- .../connection/ConnectionUIConfiguration.java | 4 +- .../v2/server/connection/ServerRunner.java | 24 +++--- .../p4ic/v2/server/util/FilePathUtil.java | 80 +++++++++++++++++-- 21 files changed, 314 insertions(+), 106 deletions(-) create mode 100644 plugin/src/net/groboclown/idea/p4ic/server/exceptions/OfflineRevertException.java diff --git a/CHANGES.md b/CHANGES.md index acaebca0..d50de7fa 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,7 +13,7 @@ * Change "maximum timeout" setting meaning. * The "maximum timeout" user setting hasn't been used since the 0.7 - release, so it now means the maximum SO socket time to live, which + release, so it now means the maximum socket time to live, which allows the user to avoid a potential issue with the underlying Perforce API. (#85) * Add lock timeout user setting. @@ -21,7 +21,13 @@ until the next one comes free. * Useful for users that have a very slow connection to the server. * Bug fixes. - * *In progress* + * Went back to actually using the "reconnect with each request" setting. + Before, this setting was ignored and all connections were reconnected + before being used. Users with slow connections should see a performance + boost with this disabled. + * Clarified error message when reverting files while working offline. + * Reduced number of false error messages when the plugin hasn't loaded the + client spec when requests are made. ## ::v0.8.5:: @@ -45,10 +51,6 @@ * Fixed the configuration panel width - an outer scroll pane confused the tab layout. The blank text fields shouldn't scroll offscreen anymore. * Fixed the SSL Fingerprint text field to correctly show the value. - * Went back to actually using the "reconnect with each request" setting. - Before, this setting was ignored and all connections were reconnected - before being used. Users with slow connections should see a performance - boost with this disabled. ## ::v0.8.4:: diff --git a/plugin/META-INF/plugin.xml b/plugin/META-INF/plugin.xml index 03596bcb..12ffc349 100644 --- a/plugin/META-INF/plugin.xml +++ b/plugin/META-INF/plugin.xml @@ -8,7 +8,14 @@
  1. 0.8.6
      -
    1. ...
      2. +
    2. Change "maximum timeout" setting meaning to now mean the maximum + socket timeout to live.
      3. +
    3. Add lock timeout user setting.
      4. +
    4. Went back to actually using the "reconnect with each request" setting, + so that it can reuse the existing connection.
      5. +
    5. Clarified error message when reverting files while working offline.
      6. +
    6. Reduced number of false error messages when the plugin hasn't loaded the + client spec when requests are made.
  2. 0.8.5 @@ -22,7 +29,6 @@
  3. Fixed the SSL Fingerprint text field to correctly show the value.
  4. Added user preference to change the socket SO timeout, to allow for fixing potential SocketTimeoutExceptions.
    5. -
  5. Went back to actually using the "reconnect with each request" setting.
@@ -30,7 +36,7 @@ [ Github ] | [ Open Issues ]

- Associate your IDEA project with Perforce through the built-in version control. + Associate your IDEA project with Perforce ("p4") through the built-in version control.

Limitations: diff --git a/plugin/plugin.iml b/plugin/plugin.iml index f4c536d5..d702675d 100644 --- a/plugin/plugin.iml +++ b/plugin/plugin.iml @@ -8,7 +8,7 @@ - + diff --git a/plugin/src/net/groboclown/idea/p4ic/P4Bundle.properties b/plugin/src/net/groboclown/idea/p4ic/P4Bundle.properties index 5d9c56cf..57143084 100644 --- a/plugin/src/net/groboclown/idea/p4ic/P4Bundle.properties +++ b/plugin/src/net/groboclown/idea/p4ic/P4Bundle.properties @@ -254,6 +254,7 @@ error.opened-action-status.invalid=The following Perforce files reported an unex error.config.invalid-roots.title=Client `{0}` Not In Project error.config.no-workspace-roots=The Perforce client files don't seem to be in the project. You may need to update your Perforce client settings. Additionally, the IDE Perforce settings must be refreshed. error.config.invalid-roots=The Perforce client files don't seem to be in the project. Your Perforce client has root directories {0}, but your project is configured with VCS directories {1}. You may need to update your Perforce client settings. Additionally, the IDE Perforce settings must be refreshed. +error.config.invalid-roots.exception=Workspace not synchronized. error.config.invalid-roots.yes=&Edit Configuration error.config.invalid-roots.no=&Cancel error.spec.unknown-open-action=Unknown open action {1} on file {0}. @@ -466,4 +467,5 @@ error.config.password.invalid=Password is incorrect. error.config.requires-password=Password required for server connection. configuration.tab.properties=Connection Properties user.prefs.socket-so-timeout=RPC SO Socket Ti&meout (in ms): -user.prefs.socket-so-timeout.tooltip=Increase this number if you see "SocketTimeoutException" problems. \ No newline at end of file +user.prefs.socket-so-timeout.tooltip=Increase this number if you see "SocketTimeoutException" problems. +error.revert.offline=Cannot revert files while offline. \ No newline at end of file diff --git a/plugin/src/net/groboclown/idea/p4ic/config/ClientConfig.java b/plugin/src/net/groboclown/idea/p4ic/config/ClientConfig.java index a183cb58..b36ac961 100644 --- a/plugin/src/net/groboclown/idea/p4ic/config/ClientConfig.java +++ b/plugin/src/net/groboclown/idea/p4ic/config/ClientConfig.java @@ -16,8 +16,6 @@ import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.vfs.VirtualFile; -import com.perforce.p4java.env.PerforceEnvironment; -import net.groboclown.idea.p4ic.P4Bundle; import net.groboclown.idea.p4ic.config.part.DataPart; import net.groboclown.idea.p4ic.v2.server.cache.ClientServerRef; import org.jetbrains.annotations.NotNull; @@ -26,14 +24,14 @@ import java.util.Collection; import java.util.Collections; import java.util.HashSet; -import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; /** * Stores information regarding a server configuration and the specific client/workspace in that * server. */ -public class ClientConfig { +public final class ClientConfig { private static final Logger LOG = Logger.getInstance(ClientConfig.class); @@ -41,6 +39,9 @@ public class ClientConfig { // is still viewable in a debugger. private static final char SEP = (char) 0x2202; + private static final AtomicInteger COUNT = new AtomicInteger(0); + + private final int configVersion; private final Project project; private final Set rootDirs; private final ServerConfig serverConfig; @@ -71,6 +72,7 @@ private ClientConfig(@NotNull Project project, @NotNull ServerConfig serverConfi " does not match data config " + ConfigPropertiesUtil.toProperties(data)); } */ + this.configVersion = COUNT.incrementAndGet(); this.project = project; this.rootDirs = Collections.unmodifiableSet(new HashSet(clientProjectBaseDirectories)); @@ -102,6 +104,10 @@ private ClientConfig(@NotNull Project project, @NotNull ServerConfig serverConfi this.clientServerRef = new ClientServerRef(serverConfig.getServerName(), clientName); } + public int getConfigVersion() { + return this.configVersion; + } + /** * * @return unique ID for this client, which is shared for all clients with the diff --git a/plugin/src/net/groboclown/idea/p4ic/config/ClientConfigSetup.java b/plugin/src/net/groboclown/idea/p4ic/config/ClientConfigSetup.java index 474c3276..d3bab9b3 100644 --- a/plugin/src/net/groboclown/idea/p4ic/config/ClientConfigSetup.java +++ b/plugin/src/net/groboclown/idea/p4ic/config/ClientConfigSetup.java @@ -23,13 +23,14 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; public final class ClientConfigSetup { private final ClientConfig config; private final Collection configProblems; private final DataPart source; - public ClientConfigSetup(@Nullable ClientConfig config, @Nullable Collection configProblems, + ClientConfigSetup(@Nullable ClientConfig config, @Nullable Collection configProblems, @NotNull DataPart source) { this.config = config; this.source = source; @@ -40,6 +41,10 @@ public ClientConfigSetup(@Nullable ClientConfig config, @Nullable Collection partList = dirToParts.get(partRoot); @@ -204,23 +206,27 @@ private Map convertToClients( for (Map.Entry> entry : parts.entrySet()) { @Nullable VirtualFile previous = entry.getKey(); - @Nullable - VirtualFile current = previous == null - ? null - : previous.getParent(); - while (current != null && ! current.equals(previous) && EqualUtil.isEqual(projectRoot, current)) { - if (parts.containsKey(current)) { - for (DataPart dataPart : parts.get(current)) { - if (! entry.getValue().contains(dataPart)) { - entry.getValue().add(dataPart); + if (previous == null || (projectRoot != null && FilePathUtil.isSameOrUnder(previous, projectRoot))) { + previous = projectRoot; + } + if (previous != null) { + for (FilePath parent : FilePathUtil.getTreeTo(previous, projectRoot)) { + // Put all the parent's data parts into the child, as lower priority + // items (at the end of the child's list). + List parentPartList = parts.get(parent.getVirtualFile()); + if (parentPartList != null) { + List childParts = entry.getValue(); + for (DataPart dataPart : parentPartList) { + if (! childParts.contains(dataPart)) { + childParts.add(dataPart); + } } } } - previous = current; - current = previous.getParent(); } } + // We now have the complete list of client servers, mapped to // their section in the project tree. We now need to organize these // into shared ClientConfig objects, while maintaining their diff --git a/plugin/src/net/groboclown/idea/p4ic/config/ServerConfig.java b/plugin/src/net/groboclown/idea/p4ic/config/ServerConfig.java index f0e4777e6..362df21f 100644 --- a/plugin/src/net/groboclown/idea/p4ic/config/ServerConfig.java +++ b/plugin/src/net/groboclown/idea/p4ic/config/ServerConfig.java @@ -23,6 +23,7 @@ import java.io.File; import java.util.*; +import java.util.concurrent.atomic.AtomicInteger; /** @@ -42,6 +43,9 @@ public final class ServerConfig { // is still viewable in a debugger. private static final char SEP = (char) 0x2202; + private static final AtomicInteger COUNT = new AtomicInteger(0); + + private final int configVersion; private final P4ServerName serverName; private final String username; private final File authTicket; @@ -110,6 +114,7 @@ private ServerConfig(@NotNull DataPart part) { if (! isValid(part)) { throw new IllegalStateException("Did not check validity before creating"); } + this.configVersion = COUNT.incrementAndGet(); assert part.hasServerNameSet(); this.serverName = part.getServerName(); @@ -138,6 +143,11 @@ private ServerConfig(@NotNull DataPart part) { } + public int getConfigVersion() { + return this.configVersion; + } + + @NotNull public P4ServerName getServerName() { return serverName; diff --git a/plugin/src/net/groboclown/idea/p4ic/config/part/MultipleDataPart.java b/plugin/src/net/groboclown/idea/p4ic/config/part/MultipleDataPart.java index 0e19e968..51519f82 100644 --- a/plugin/src/net/groboclown/idea/p4ic/config/part/MultipleDataPart.java +++ b/plugin/src/net/groboclown/idea/p4ic/config/part/MultipleDataPart.java @@ -27,12 +27,16 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; public class MultipleDataPart implements DataPart { private static final Logger LOG = Logger.getInstance(MultipleDataPart.class); + private static final AtomicInteger COUNT = new AtomicInteger(0); + private final VirtualFile root; private final List parts; + private final int index = COUNT.incrementAndGet(); public MultipleDataPart(@Nullable VirtualFile root, @NotNull List parts) { this.root = root; @@ -71,14 +75,14 @@ public int hashCode() { @Override public String toString() { - return "MultipleDataPart(" + root + ")"; + return "MultipleDataPart(" + root + "):" + index; } @NotNull @Override public Collection getConfigProblems() { if (LOG.isDebugEnabled()) { - LOG.debug("Config for MultipleDataPart under " + root); + LOG.debug(this + ": finding config problems"); } Set problems = new HashSet(); @@ -126,6 +130,9 @@ public boolean hasServerNameSet() { public P4ServerName getServerName() { for (DataPart part : parts) { if (part.hasServerNameSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using server name from " + part); + } return part.getServerName(); } } @@ -147,6 +154,9 @@ public boolean hasClientnameSet() { public String getClientname() { for (DataPart part : parts) { if (part.hasClientnameSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using client name from " + part); + } return part.getClientname(); } } @@ -168,6 +178,9 @@ public boolean hasUsernameSet() { public String getUsername() { for (DataPart part : parts) { if (part.hasUsernameSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using user name from " + part); + } return part.getUsername(); } } @@ -189,6 +202,9 @@ public boolean hasPasswordSet() { public String getPlaintextPassword() { for (DataPart part : parts) { if (part.hasPasswordSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using plaintext password from " + part); + } return part.getPlaintextPassword(); } } @@ -210,6 +226,9 @@ public boolean hasAuthTicketFileSet() { public File getAuthTicketFile() { for (DataPart part : parts) { if (part.hasAuthTicketFileSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using auth ticket file from " + part); + } return part.getAuthTicketFile(); } } @@ -231,6 +250,9 @@ public boolean hasTrustTicketFileSet() { public File getTrustTicketFile() { for (DataPart part : parts) { if (part.hasTrustTicketFileSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using trust ticket file from " + part); + } return part.getTrustTicketFile(); } } @@ -252,6 +274,9 @@ public boolean hasServerFingerprintSet() { public String getServerFingerprint() { for (DataPart part : parts) { if (part.hasServerFingerprintSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using server fingerprint from " + part); + } return part.getServerFingerprint(); } } @@ -273,6 +298,9 @@ public boolean hasClientHostnameSet() { public String getClientHostname() { for (DataPart part : parts) { if (part.hasClientHostnameSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using client hostname from " + part); + } return part.getClientHostname(); } } @@ -294,6 +322,9 @@ public boolean hasIgnoreFileNameSet() { public String getIgnoreFileName() { for (DataPart part : parts) { if (part.hasIgnoreFileNameSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using ignore file name from " + part); + } return part.getIgnoreFileName(); } } @@ -315,6 +346,9 @@ public boolean hasDefaultCharsetSet() { public String getDefaultCharset() { for (DataPart part : parts) { if (part.hasDefaultCharsetSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using default charset from " + part); + } return part.getDefaultCharset(); } } @@ -336,6 +370,9 @@ public boolean hasLoginSsoSet() { public File getLoginSso() { for (DataPart part : parts) { if (part.hasLoginSsoSet()) { + if (LOG.isDebugEnabled()) { + LOG.debug(this + ": using login sso from " + part); + } return part.getLoginSso(); } } diff --git a/plugin/src/net/groboclown/idea/p4ic/server/exceptions/OfflineRevertException.java b/plugin/src/net/groboclown/idea/p4ic/server/exceptions/OfflineRevertException.java new file mode 100644 index 00000000..4f9b0a81 --- /dev/null +++ b/plugin/src/net/groboclown/idea/p4ic/server/exceptions/OfflineRevertException.java @@ -0,0 +1,23 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.groboclown.idea.p4ic.server.exceptions; + +import net.groboclown.idea.p4ic.P4Bundle; + +public class OfflineRevertException extends P4DisconnectedException { + public OfflineRevertException() { + super(P4Bundle.message("error.revert.offline")); + } +} diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/ResolvedPropertiesPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/ResolvedPropertiesPanel.java index b935dbc5..449a3c6e 100644 --- a/plugin/src/net/groboclown/idea/p4ic/ui/config/ResolvedPropertiesPanel.java +++ b/plugin/src/net/groboclown/idea/p4ic/ui/config/ResolvedPropertiesPanel.java @@ -125,6 +125,8 @@ public ComputedConfigResults runBackgroundProcess() { return results; } + LOG.debug("Refreshing last configuration"); + lastConfig.refresh(); final ComputedConfigResults results = loadConfigResults(lastConfig); @@ -263,9 +265,13 @@ void setRequestConfigurationLoadListener( private ComputedConfigResults loadConfigResults(@NotNull final P4ProjectConfig projectConfig) { ComputedConfigResults results = new ComputedConfigResults(); results.problemMessages = new ArrayList(projectConfig.getConfigProblems()); + if (LOG.isDebugEnabled()) { + LOG.debug("Base project config problems: " + results.problemMessages); + } Collection configs = projectConfig.getClientConfigSetups(); if (configs.isEmpty()) { + LOG.debug("No client configs in setup."); results.problemMessages.add(createNoClientConfigProblem()); } for (ClientConfigSetup configSetup : configs) { @@ -275,6 +281,10 @@ private ComputedConfigResults loadConfigResults(@NotNull final P4ProjectConfig p ConfigProblem problem = ConnectionUIConfiguration.checkConnection(config, ServerConnectionManager.getInstance(), false); if (problem != null) { + if (LOG.isDebugEnabled()) { + LOG.debug("Config setup " + configSetup.getSource() + " has problem " + + problem); + } results.problemMessages.add(problem); } // We can have a connection without a client, for testing purposes, diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/P4Server.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/P4Server.java index d8facb08..bd97364e 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/P4Server.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/P4Server.java @@ -69,7 +69,7 @@ * The owner of this object needs to be aware of config changes; those * signal that the server instances are no longer valid. * It should listen to {@link net.groboclown.idea.p4ic.v2.events.BaseConfigUpdatedListener#TOPIC_NORMAL} events, which - * are generated by {@link net.groboclown.idea.p4ic.config.P4ConfigProject}. + * are generated by {@link net.groboclown.idea.p4ic.config.P4ProjectConfigComponent}. *

* The owner should also only save the state for valid server objects. *

@@ -110,11 +110,6 @@ public IntegrateFile(@Nullable ClientServerRef sourceClient, @NotNull FilePath s this.targetFile = targetFile; } - @Nullable - public ClientServerRef getSourceClient() { - return sourceClient; - } - @NotNull public FilePath getSourceFile() { return sourceFile; @@ -176,7 +171,7 @@ public boolean isWorkingOffline() { return !isValid() || connection.isWorkingOffline(); } - public boolean isSameSource(@Nullable ClientConfig pcs) { + boolean isSameSource(@Nullable ClientConfig pcs) { return source.equals(pcs); } @@ -236,7 +231,7 @@ int getFilePathMatchDepth(@NotNull FilePath file) throws InterruptedException { * * @return the actual client root directories used by the workspace, * split by parent directories. - * @throws InterruptedException + * @throws InterruptedException if the underlying query is interrupted */ @NotNull public List> getRoots() throws InterruptedException { @@ -250,17 +245,26 @@ public List> getRoots() throws InterruptedException { * wider than what should be used. * * @return project-based roots - * @throws InterruptedException + * @throws InterruptedException if the underlying query is cancelled. */ + @NotNull private List getProjectClientRoots() throws InterruptedException { return connection.cacheQuery(new CacheQuery>() { @Override public List query(@NotNull final ClientCacheManager mgr) throws InterruptedException { List roots = mgr.getClientRoots(project, alertManager); - if (roots.isEmpty() && isWorkingOnline()) { + if (roots == null && isWorkingOnline()) { LOG.debug("working online; no roots known for client, so refreshing list"); connection.query(project, mgr.createWorkspaceRefreshQuery()); roots = mgr.getClientRoots(project, alertManager); + if (roots == null) { + // TODO determine if this means we should tell the user. + LOG.info("Attempted to reload the workspace information, but the workspace roots list is " + + "still empty"); + } + } + if (roots == null) { + return Collections.emptyList(); } return roots; } @@ -327,23 +331,29 @@ public List query(@NotNull final P4Exec2 exec, } + @NotNull public Map mapSpecsToPath(@NotNull final Collection specs) throws InterruptedException { if (specs.isEmpty()) { return Collections.emptyMap(); } - return connection.query(project, new ServerQuery>() { - @Nullable - @Override - public Map query(@NotNull final P4Exec2 exec, - @NotNull final ClientCacheManager cacheManager, - @NotNull final ServerConnection connection, - @NotNull final SynchronizedActionRunner runner, - @NotNull final AlertManager alerts) - throws InterruptedException { - return cacheManager.mapSpecsToPath(specs); - } - }); + Map ret = + connection.query(project, new ServerQuery>() { + @NotNull + @Override + public Map query(@NotNull final P4Exec2 exec, + @NotNull final ClientCacheManager cacheManager, + @NotNull final ServerConnection connection, + @NotNull final SynchronizedActionRunner runner, + @NotNull final AlertManager alerts) + throws InterruptedException { + return cacheManager.mapSpecsToPath(specs); + } + }); + if (ret == null) { + return Collections.emptyMap(); + } + return ret; } @@ -547,16 +557,8 @@ public void revertFiles(@NotNull final List files, List try { unreverted.removeAll(revertFilesOffline(files)); if (!unreverted.isEmpty()) { - LOG.warn("Could not offline revert " + unreverted); - // TODO this is a terrible message to send to users. Make it user friendly. - exceptions.add(new P4DisconnectedException()); + exceptions.add(new OfflineRevertException()); } - //if (!unreverted.isEmpty()) { - // alerts.addWarning(vcs.getProject(), - // P4Bundle.message("revert.offline", server.getClientServerRef()), - // P4Bundle.message("revert.offline", server.getClientServerRef()), - // null, unreverted.toArray(new FilePath[unreverted.size()])); - //} } catch (InterruptedException e) { LOG.warn(e); exceptions.add(new VcsInterruptedException(e)); @@ -572,7 +574,7 @@ public void revertFiles(@NotNull final List files, List * @return the files that were actually reverted. */ @NotNull - public Collection revertFilesOffline(@NotNull final List files) throws InterruptedException { + private Collection revertFilesOffline(@NotNull final List files) throws InterruptedException { if (files.isEmpty()) { return Collections.emptyList(); } @@ -591,7 +593,7 @@ public Collection query(@NotNull final ClientCacheManager mgr) throws * @param files files to revert. */ @NotNull - public MessageResult> revertFilesOnline(@NotNull final List files) + private MessageResult> revertFilesOnline(@NotNull final List files) throws InterruptedException, P4DisconnectedException { if (files.isEmpty()) { return new MessageResult>( @@ -866,7 +868,7 @@ public Boolean query(@NotNull final ClientCacheManager mgr) throws InterruptedEx * * @param spec file spec to read * @return the file contents, or null if it does not exist. - * @throws P4DisconnectedException + * @throws P4DisconnectedException disconnected */ @Nullable public String loadFileAsStringOnline(@NotNull final FilePath file, @NotNull final IFileSpec spec) @@ -1222,8 +1224,8 @@ public Void query(@NotNull final ClientCacheManager mgr) throws InterruptedExcep * correct. If we have a P4FileSyncState, then that is used to * determine the MD5. * - * @param files - * @return + * @param files list of files idea thinks are different. + * @return actually different files */ @NotNull public List getVirtualFilesDifferentThanServerHaveVersionOnline( @@ -1324,7 +1326,7 @@ private List getFilesDifferentThanServerOnline(@NotNull final Map ret = new ArrayList(syncState.size()); for (Entry entry : syncState.entrySet()) { final VirtualFile vf = entry.getKey(); - String vfMd5 = null; + String vfMd5; try { vfMd5 = readMd5(entry.getKey()); } catch (IOException e) { @@ -1349,7 +1351,6 @@ private List getFilesDifferentThanServerOnline(@NotNull final Map ex = new Ref(); fsMd5 = connection.query(project, new ServerQuery() { @Nullable @Override diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/Cache.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/Cache.java index 5dfe9f7f..4efbc8f2 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/Cache.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/Cache.java @@ -39,7 +39,13 @@ * {@link CacheFrontEnd} objects. */ interface Cache { - @NotNull + /** + * + * @param project project + * @param alerts alert manager + * @return null if the workspace hasn't synchronized yet. + */ + @Nullable List getClientRoots(@NotNull Project project, @NotNull AlertManager alerts); diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/ClientCacheManager.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/ClientCacheManager.java index 7c6b6df4..277ea6dd 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/ClientCacheManager.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/ClientCacheManager.java @@ -176,13 +176,15 @@ public PendingUpdateState deleteChangelist(final int changeListId) { return changeLists.deleteChangelist(changeListId); } - @NotNull + @Nullable public List getClientRoots(@NotNull Project project, @NotNull AlertManager alerts) { return workspace.getClientRoots(project, alerts); } public boolean hasClientRoots(@NotNull Project project) { - + if (project.isDisposed()) { + return false; + } return workspace.getSimpleClientRoots(project) != null; } @@ -350,7 +352,7 @@ public ClientServerRef getClientServerId() { private class CacheImpl implements Cache { - @NotNull + @Nullable @Override public List getClientRoots(@NotNull final Project project, @NotNull AlertManager alerts) { return workspace.getClientRoots(project, alerts); diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/FileActionsServerCacheSync.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/FileActionsServerCacheSync.java index 46012de6..0f241dbb 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/FileActionsServerCacheSync.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/FileActionsServerCacheSync.java @@ -44,6 +44,7 @@ import net.groboclown.idea.p4ic.v2.server.cache.sync.AbstractServerUpdateAction.ExecutionStatus; import net.groboclown.idea.p4ic.v2.server.connection.*; import net.groboclown.idea.p4ic.v2.server.util.FilePathUtil; +import net.groboclown.idea.p4ic.v2.ui.alerts.InvalidRootsHandler; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -762,6 +763,15 @@ private boolean isValidUpdateAction(IFileSpec spec) { */ private List getClientRootSpecs(@NotNull Project project, @NotNull AlertManager alerts) throws VcsException { final List roots = cache.getClientRoots(project, alerts); + if (roots == null) { + // We need to synchronize the workspace. + VcsException invalidRootsException = + new VcsException(P4Bundle.getString("error.config.invalid-roots.exception")); + // invalidRootsException.fillInStackTrace(); + alerts.addCriticalError(new InvalidRootsHandler(project, Collections.emptyList(), + cache.getClientServerId(), invalidRootsException), invalidRootsException); + return Collections.emptyList(); + } return FileSpecUtil.makeRootFileSpecs(roots.toArray(new VirtualFile[roots.size()])); } diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/WorkspaceServerCacheSync.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/WorkspaceServerCacheSync.java index f2fdd415..aa612647 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/WorkspaceServerCacheSync.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/cache/sync/WorkspaceServerCacheSync.java @@ -39,7 +39,6 @@ import net.groboclown.idea.p4ic.v2.server.util.FilePathUtil; import net.groboclown.idea.p4ic.v2.ui.alerts.ClientNameMismatchHandler; import net.groboclown.idea.p4ic.v2.ui.alerts.InvalidClientHandler; -import net.groboclown.idea.p4ic.v2.ui.alerts.InvalidRootsHandler; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -233,16 +232,26 @@ P4ClientFileMapping getClientMappingFor(final FilePath file) { return fileRepo.getByLocation(file); } - @NotNull + /** + * + * @param project project + * @param alerts alert manager + * @return null if the client roots have not been synchronized, or if the list of + * workspace roots is empty. + */ + @Nullable List getClientRoots(@NotNull Project project, @NotNull AlertManager alerts) { + if (project.isDisposed()) { + // Special case to avoid error messages on a disposed client. + return Collections.emptyList(); + } final List ret = getSimpleClientRoots(project); - if (ret == null) { + if (ret == null || ret.isEmpty()) { // no root found. - invalidRootsException.fillInStackTrace(); - alerts.addCriticalError(new InvalidRootsHandler(project, - cachedServerWorkspace.getRoots(), - cache.getClientServerId(), invalidRootsException), invalidRootsException); - return Collections.emptyList(); + // There are some circumstances where this is fine, and we don't need to + // raise an error. Indeed, all the callers know about checking for + // an empty list. + return null; } return ret; } @@ -481,7 +490,7 @@ protected void innerLoadServerCache(@NotNull final P4Exec2 exec, @NotNull final exec.getProject(), P4Bundle.message("warning.client.updated.title", getCachedClientName()), P4Bundle.message("warning.client.updated", getCachedClientName()), - null, FilePathUtil.getFilePathsFsrStrings(roots)); + null, FilePathUtil.getFilePathsForStrings(roots)); } cache.refreshServerState(exec, alerts); } diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/AuthenticatedServer.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/AuthenticatedServer.java index ffd4f9d4..121b42e3 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/AuthenticatedServer.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/AuthenticatedServer.java @@ -94,12 +94,15 @@ class AuthenticatedServer { public static class ServerConnection { private final Project project; + private final ClientConfig clientConfig; private final IOptionsServer server; private final ServerAuthenticator.AuthenticationStatus authStatus; - public ServerConnection(@NotNull Project project, @Nullable IOptionsServer server, + public ServerConnection(@NotNull Project project, @NotNull ClientConfig config, + @Nullable IOptionsServer server, @NotNull ServerAuthenticator.AuthenticationStatus authStatus) { this.project = project; + this.clientConfig = config; this.server = server; this.authStatus = authStatus; if (authStatus.isAuthenticated() && server == null) { @@ -107,6 +110,11 @@ public ServerConnection(@NotNull Project project, @Nullable IOptionsServer serve } } + @NotNull + public ClientConfig getClientConfig() { + return clientConfig; + } + @Nullable public IOptionsServer getServer() { return server; @@ -180,7 +188,7 @@ ServerConnection checkoutServer() } finally { connectLock.unlock(); } - return new ServerConnection(project, retServer, retAuthStatus); + return new ServerConnection(project, config, retServer, retAuthStatus); } void checkinServer(@NotNull IOptionsServer server) throws P4JavaException, InterruptedException { diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ClientExec.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ClientExec.java index bf82502d..187f83fb 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ClientExec.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ClientExec.java @@ -345,7 +345,8 @@ public void loginFailure(@NotNull final P4LoginException e) throws VcsException, } @Override - public void loginRequiresPassword(@NotNull P4PasswordException cause) + public void loginRequiresPassword(@NotNull P4PasswordException cause, + ClientConfig clientConfig) throws VcsException, CancellationException { AlertManager.getInstance().addCriticalError( new LoginFailedHandler(project, connectedController, config.getServerConfig(), cause), cause); diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ConnectionUIConfiguration.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ConnectionUIConfiguration.java index 0d61967f..bbfaed1e 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ConnectionUIConfiguration.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ConnectionUIConfiguration.java @@ -214,7 +214,7 @@ public void loginFailure(@NotNull P4LoginException e) @Override public void loginRequiresPassword( - @NotNull P4PasswordException cause) + @NotNull P4PasswordException cause, ClientConfig clientConfig) throws VcsException, CancellationException { LOG.info("Login requires password"); problems.add(cause); @@ -225,7 +225,7 @@ public void loginRequiresPassword( // refresh. This is pulled out from the LoginFailedHandler class. // FIXME This just sits around and waits for the configuration dialog - // to go away. + // to go away... sometimes. ApplicationManager.getApplication().invokeLater(new Runnable() { @Override public void run() { diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ServerRunner.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ServerRunner.java index 01014875..de63c0c3 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ServerRunner.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/connection/ServerRunner.java @@ -21,9 +21,9 @@ import com.perforce.p4java.exception.*; import com.perforce.p4java.server.IOptionsServer; import net.groboclown.idea.p4ic.P4Bundle; +import net.groboclown.idea.p4ic.config.ClientConfig; import net.groboclown.idea.p4ic.server.VcsExceptionUtil; import net.groboclown.idea.p4ic.server.exceptions.*; -import net.groboclown.idea.p4ic.v2.server.authentication.PasswordManager; import net.groboclown.idea.p4ic.v2.server.authentication.ServerAuthenticator; import org.jetbrains.annotations.NotNull; @@ -72,7 +72,8 @@ interface ErrorVisitor { void loginFailure(@NotNull P4LoginException e) throws VcsException, CancellationException; - void loginRequiresPassword(@NotNull P4PasswordException cause) + void loginRequiresPassword(@NotNull P4PasswordException cause, + ClientConfig clientConfig) throws VcsException, CancellationException; void disconnectFailure(P4DisconnectedException e) @@ -163,14 +164,20 @@ static IOptionsServer getServerFor(@NotNull AuthenticatedServer.ServerConnection throw new HandledVcsException(problem); } if (status.isPasswordRequired()) { - // TODO shoulnd't need to do this casting; instead, use the real problem. + // TODO shouldn't need to do this casting; instead, use the real problem. final P4PasswordException problem; if (status.getProblem() != null && status.getProblem() instanceof P4PasswordException) { problem = (P4PasswordException) status.getProblem(); } else { problem = new P4LoginRequiresPasswordException(new P4JavaException()); } - errorVisitor.loginRequiresPassword(problem); + // TODO currently investigating ways to prevent a persistent pestering for the password. + // If it's the same client and server ID, then that means it's easy to skip (keep a record + // of which configs has been asked; but that might be a memory issue). Otherwise, this gets + // trickier. + LOG.info("Password is required for connection ID " + servCon.getClientConfig().getConfigVersion() + "." + + servCon.getClientConfig().getServerConfig().getConfigVersion() + ": " + problem.getMessage()); + errorVisitor.loginRequiresPassword(problem, servCon.getClientConfig()); throw new HandledVcsException(problem); } if (status.isSessionExpired()) { @@ -306,7 +313,6 @@ private static T p4RunFor(@NotNull P4Runner runner, @NotNull final Connec throw errorVisitor.sslFingerprintError(e); } - if (isSSLHandshakeProblem(e)) { if (isUnlimitedStrengthEncryptionInstalled()) { // config not invalid @@ -434,14 +440,6 @@ private static T p4RunFor(@NotNull P4Runner runner, @NotNull final Connec private static T retry(final P4Runner runner, final Connection conn, final ErrorVisitor errorVisitor, final int retryCount, final Exception e) throws VcsException { - /* TODO understand the right behavior here, with the new login flow - if (retryCount > 1) { - P4WorkingOfflineException ex = new P4WorkingOfflineException(e); - errorVisitor.disconnectFailure(ex); - throw ex; - } - return p4RunFor(runner, conn, errorVisitor, retryCount + 1); - */ P4WorkingOfflineException ex = new P4WorkingOfflineException(e); errorVisitor.disconnectFailure(ex); throw ex; diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/util/FilePathUtil.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/util/FilePathUtil.java index f21fa02c..070efd34 100644 --- a/plugin/src/net/groboclown/idea/p4ic/v2/server/util/FilePathUtil.java +++ b/plugin/src/net/groboclown/idea/p4ic/v2/server/util/FilePathUtil.java @@ -71,6 +71,14 @@ public static FilePath getFilePath(@NotNull final VirtualFile f) { } } + @Nullable + public static FilePath getNullableFilePath(@Nullable final VirtualFile f) { + if (f == null) { + return null; + } + return getFilePath(f); + } + @NotNull public static Collection getFilePathsForVirtualFiles(@NotNull Collection virtualFiles) { List ret = new ArrayList(virtualFiles.size()); @@ -83,7 +91,7 @@ public static Collection getFilePathsForVirtualFiles(@NotNull Collecti } @NotNull - public static Collection getFilePathsFsrStrings(@NotNull List paths) { + public static Collection getFilePathsForStrings(@NotNull List paths) { List ret = new ArrayList(paths.size()); for (String path: paths) { if (path != null) { @@ -109,19 +117,77 @@ public static List toStringList(@Nullable Collection files) { return ret; } - public static boolean isSameOrUnder(@NotNull FilePath parent, @NotNull FilePath child) { + /** + * Break apart the path so that it contains the complete directory chain. The + * First element returned is the path object passed in. + * + * @param path + * @return list of path directories. + */ + @NotNull + public static List getTree(@NotNull FilePath path) { + List ret = new ArrayList(); FilePath prev; - FilePath next = child; + FilePath next = path; do { - if (parent.equals(next)) { - return true; - } + ret.add(next); prev = next; next = next.getParentPath(); } while (next != null && ! next.equals(prev)); - return false; + return ret; + } + + /** + * Break apart the path into parent directories, up to and including the {@literal parent}. + * If the {@literal parent} is never reached, then the complete tree is returned. + * + * @param path + * @param parent + * @return + */ + @NotNull + public static List getTreeTo(@NotNull FilePath path, @Nullable FilePath parent) { + List ret = new ArrayList(); + FilePath prev; + FilePath next = path; + do { + ret.add(next); + prev = next; + next = next.getParentPath(); + } while (next != null && ! next.equals(prev) && ! prev.equals(parent)); + return ret; } + public static List getTreeTo(@NotNull VirtualFile path, @Nullable VirtualFile parent) { + return getTreeTo(getFilePath(path), getNullableFilePath(parent)); + } + + /** + * Tests if {@literal child} is a child (a sub-directory or sub-file) or the same directory as + * {@literal parent} + * + * @param parent base directory for comparison. + * @param child file or directory to check against parent. + * @return true if child is the same directory as parent, a sub-directory of parent, or a file in parent. + */ + public static boolean isSameOrUnder(@NotNull FilePath parent, @NotNull FilePath child) { + List paths = getTreeTo(child, parent); + return ! paths.isEmpty() && parent.equals(paths.get(paths.size() - 1)); + } + + /** + * Tests if {@literal child} is a child (a sub-directory or sub-file) or the same directory as + * {@literal parent} + * + * @param parent base directory for comparison. + * @param child file or directory to check against parent. + * @return true if child is the same directory as parent, a sub-directory of parent, or a file in parent. + */ + public static boolean isSameOrUnder(@NotNull VirtualFile parent, @NotNull VirtualFile child) { + return isSameOrUnder(getFilePath(parent), getFilePath(child)); + } + + /** * * @param project