From 835b505f16fa67e8ffe6a71040aed2a92dde0085 Mon Sep 17 00:00:00 2001 From: Groboclown Date: Mon, 2 Mar 2020 10:01:40 -0600 Subject: [PATCH] Partial fixes to #211, NPE fixes, and normalized path view. Bug #211 can only be partially fixed, due to a problem with the IDE (see the bug report for details). There were some additional NPEs discovered that came with the refactoring of the configuration passing, and with infrequently encoutnered logging. The connection view pane now has the root paths shown with the right path separator character on Windows. --- CHANGES.md | 14 +++++++ .../p4/server/api/ProjectConfigRegistry.java | 2 +- .../p4/server/api/config/part/ConfigPart.java | 8 ++++ .../api/config/part/ConfigPartAdapter.java | 6 +++ .../api/config/part/MultipleConfigPart.java | 16 ++++++++ .../p4/server/api/MockConfigPart.java | 9 ++++- .../config/part/ConfigPartAdapterTest.java | 15 +++++++- .../impl/ProjectConfigRegistryImpl.java | 12 ++---- .../config/PersistentRootConfigComponent.java | 7 +++- .../config/part/ClientNameConfigPart.java | 25 +++++++----- .../impl/config/part/EnvCompositePart.java | 12 ++++++ .../impl/config/part/FileConfigPart.java | 6 +++ .../config/part/RequirePasswordDataPart.java | 8 ++++ .../part/ServerFingerprintDataPart.java | 8 ++++ .../impl/config/part/SimpleDataPart.java | 6 +++ .../impl/config/part/WinRegDataPart.java | 9 +++++ .../connection/P4RequestErrorHandler.java | 12 ++++++ .../impl/MessageP4RequestErrorHandler.java | 27 ++++++------- .../impl/util/DirectoryMappingUtil.java | 38 ------------------- .../p4/server/impl/util/RootSettingsUtil.java | 26 ++++++++++++- .../ConnectionTreeCellRenderer.java | 4 +- .../p4plugin/ui/connection/VcsRootNode.java | 20 ---------- .../ui/vcsroot/P4RootConfigPanel.java | 2 +- .../ui/vcsroot/P4VcsRootConfigurable.java | 14 ++++--- .../src/main/resources/META-INF/plugin.xml | 13 +++---- .../groboclown/p4plugin/P4Bundle.properties | 1 + 26 files changed, 212 insertions(+), 108 deletions(-) delete mode 100644 idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/util/DirectoryMappingUtil.java delete mode 100644 plugin-v3/src/main/java/net/groboclown/p4plugin/ui/connection/VcsRootNode.java diff --git a/CHANGES.md b/CHANGES.md index 61fca1b5..333f73c7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,20 @@ # IDEA Community VCS Integration for Perforce +## ::v0.10.15:: + +### Overview + +* Bug fixes + +### Details + +* Bug fixes + * Fixed part of a bug where the VCS root configuration shared options between root directories (#211). This is only a partial fix, as there's an underlying bug in the IDE that causes this problem. The resolved properties panel shows the path that's being set, so if the UI seems to show the wrong path, it's because of this underlying bug. + * Fixed some NPEs caused by certain error and logging conditions. + * Normalized the path view in the connections panel for Windows. + + ## ::v0.10.13:: and ::v0.10.14:: ### Overview diff --git a/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/ProjectConfigRegistry.java b/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/ProjectConfigRegistry.java index 9e8d3c97..9c0aab82 100644 --- a/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/ProjectConfigRegistry.java +++ b/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/ProjectConfigRegistry.java @@ -120,7 +120,7 @@ public ClientConfigRoot getClientFor(@Nullable FilePath file) { } } if (LOG.isDebugEnabled()) { - LOG.debug("Using client root " + closest.getClientRootDir() + " for " + file); + LOG.debug("Using client root " + (closest == null ? null : closest.getClientRootDir()) + " for " + file); } return closest; diff --git a/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/ConfigPart.java b/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/ConfigPart.java index bff49800..a48ca31a 100644 --- a/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/ConfigPart.java +++ b/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/ConfigPart.java @@ -202,4 +202,12 @@ public interface ConfigPart { * @return the raw port string value, before parsing as a name. */ String getRawPort(); + + /** + * Make a copy of this configuration part. + * + * @return a copy + */ + @NotNull + ConfigPart copy(); } diff --git a/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/ConfigPartAdapter.java b/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/ConfigPartAdapter.java index f1e53f38..d63bf112 100644 --- a/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/ConfigPartAdapter.java +++ b/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/ConfigPartAdapter.java @@ -196,6 +196,12 @@ public String getRawPort() { return null; } + @NotNull + @Override + public ConfigPart copy() { + return new ConfigPartAdapter(sourceName); + } + @Override public boolean requiresUserEnteredPassword() { return false; diff --git a/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/MultipleConfigPart.java b/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/MultipleConfigPart.java index 9bcef364..d2af9912 100644 --- a/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/MultipleConfigPart.java +++ b/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/part/MultipleConfigPart.java @@ -29,6 +29,7 @@ import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; public class MultipleConfigPart implements ConfigPart { @@ -134,6 +135,15 @@ public String getRawPort() { return null; } + @NotNull + @Override + public ConfigPart copy() { + MultipleConfigPart ret = new MultipleConfigPart(sourceName, + parts.stream().map(ConfigPart::copy).collect(Collectors.toList())); + ret.extraProblems.addAll(extraProblems); + return ret; + } + @Override public boolean hasServerNameSet() { for (ConfigPart part : parts) { @@ -414,6 +424,12 @@ public List getChildren() { return new ArrayList<>(parts); } + + @NotNull + public List cloneChildren() { + return parts.stream().map(ConfigPart::copy).collect(Collectors.toList()); + } + public void addAdditionalProblem(@NotNull ConfigProblem problem) { this.extraProblems.add(problem); } diff --git a/idea-p4server/api/src/test/java/net/groboclown/p4/server/api/MockConfigPart.java b/idea-p4server/api/src/test/java/net/groboclown/p4/server/api/MockConfigPart.java index 1c454863..91b9db7e 100644 --- a/idea-p4server/api/src/test/java/net/groboclown/p4/server/api/MockConfigPart.java +++ b/idea-p4server/api/src/test/java/net/groboclown/p4/server/api/MockConfigPart.java @@ -44,7 +44,8 @@ public class MockConfigPart private boolean requiresUserEnteredPassword; private Collection configProblems = new ArrayList<>(); - public MockConfigPart copy() { + @NotNull + public MockConfigPart copyMock() { MockConfigPart ret = new MockConfigPart() .withSourceName(sourceName) .withP4ServerName(serverName) @@ -308,6 +309,12 @@ public String getRawPort() { return serverName.getFullPort(); } + @NotNull + @Override + public ConfigPart copy() { + return copyMock(); + } + public MockConfigPart withConfigProblems(ConfigProblem... problems) { configProblems = Arrays.asList(problems); return this; diff --git a/idea-p4server/api/src/test/java/net/groboclown/p4/server/api/config/part/ConfigPartAdapterTest.java b/idea-p4server/api/src/test/java/net/groboclown/p4/server/api/config/part/ConfigPartAdapterTest.java index 03eff372..8ebd3953 100644 --- a/idea-p4server/api/src/test/java/net/groboclown/p4/server/api/config/part/ConfigPartAdapterTest.java +++ b/idea-p4server/api/src/test/java/net/groboclown/p4/server/api/config/part/ConfigPartAdapterTest.java @@ -14,9 +14,11 @@ package net.groboclown.p4.server.api.config.part; import net.groboclown.p4.server.api.config.ConfigProblem; +import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; import javax.annotation.Nonnull; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -224,10 +226,21 @@ private ProblemConfigPartAdapter(ConfigProblem... problems) { this.problems = Arrays.asList(problems); } + public ProblemConfigPartAdapter(List problems) { + super("Blah"); + this.problems = new ArrayList<>(problems); + } + @Nonnull @Override public Collection getConfigProblems() { return problems; } + + @NotNull + @Override + public ConfigPart copy() { + return new ProblemConfigPartAdapter(problems); + } } -} \ No newline at end of file +} diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/ProjectConfigRegistryImpl.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/ProjectConfigRegistryImpl.java index 244ba50b..8a84cb86 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/ProjectConfigRegistryImpl.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/ProjectConfigRegistryImpl.java @@ -37,7 +37,6 @@ import net.groboclown.p4.server.api.util.FilteredIterable; import net.groboclown.p4.server.impl.cache.ClientConfigRootImpl; import net.groboclown.p4.server.impl.cache.ServerStatusImpl; -import net.groboclown.p4.server.impl.util.DirectoryMappingUtil; import net.groboclown.p4.server.impl.util.RootSettingsUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -274,10 +273,9 @@ protected void updateVcsRoots() { continue; } if (settings instanceof P4VcsRootSettings) { - oldRoots.remove(DirectoryMappingUtil.getDirectory(getProject(), directoryMapping)); + oldRoots.remove(RootSettingsUtil.getDirectory(getProject(), directoryMapping)); updateRoot( - RootSettingsUtil.getFixedRootSettings(getProject(), directoryMapping, - DirectoryMappingUtil.getDirectory(getProject(), directoryMapping)), + RootSettingsUtil.getFixedRootSettings(getProject(), directoryMapping), directoryMapping); } else { LOG.warn("P4Vcs root mapping has non-vcs settings " + settings.getClass()); @@ -303,10 +301,8 @@ private void updateRoot(@NotNull P4VcsRootSettings settings, addClientConfig(clientConfig, root); return; } - if (LOG.isDebugEnabled()) { - LOG.debug(root + ": skipping invalid config " + parentPart); - } - } else if (LOG.isDebugEnabled()) { + } + if (LOG.isDebugEnabled()) { LOG.debug(root + ": skipping invalid config " + parentPart); } } catch (IllegalArgumentException e) { diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/PersistentRootConfigComponent.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/PersistentRootConfigComponent.java index cabbb54f..c1bdd841 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/PersistentRootConfigComponent.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/PersistentRootConfigComponent.java @@ -129,8 +129,9 @@ public Element getState() { Element ret = new Element("all-root-configs"); for (Map.Entry> entry : rootPartMap.entrySet()) { if (!isValidRoot(entry.getKey())) { - LOG.info("Skipped writing root " + entry.getKey() + - " because it does not appear to be a valid Perforce VCS root"); + LOG.info("Skipped persisting VCS root " + entry.getKey() + + " because it does not appear to be a valid Perforce root"); + continue; } VcsRootCacheStore store = new VcsRootCacheStore(entry.getKey()); store.setConfigParts(entry.getValue()); @@ -199,6 +200,8 @@ private boolean isValidRoot(final VirtualFile file) { return true; } } + // This can happen if the passed-in file is not yet registered in + // the VCS root path. return false; } } diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/ClientNameConfigPart.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/ClientNameConfigPart.java index a9cfb7ae..676fe7c0 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/ClientNameConfigPart.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/ClientNameConfigPart.java @@ -17,6 +17,7 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import net.groboclown.p4.server.api.config.ConfigProblem; +import net.groboclown.p4.server.api.config.part.ConfigPart; import net.groboclown.p4.server.api.config.part.ConfigPartAdapter; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -43,12 +44,19 @@ public ClientNameConfigPart(@NotNull String sourceName) { super(sourceName); } + // require this constructor + @SuppressWarnings("unused") public ClientNameConfigPart(@NotNull String sourceName, @NotNull VirtualFile root, @NotNull Map stateValues) { super(sourceName); this.clientName = stateValues.get("c"); } + private ClientNameConfigPart(@NotNull String sourceName, @Nullable String clientName) { + super(sourceName); + this.clientName = clientName; + } + @Override public boolean reload() { // Nothing to do @@ -69,6 +77,14 @@ public String getRawPort() { return null; } + @NotNull + @Override + public ConfigPart copy() { + ClientNameConfigPart ret = new ClientNameConfigPart(getSourceName(), clientName); + ret.additionalProblems.addAll(additionalProblems); + return ret; + } + @Override public boolean hasClientnameSet() { @@ -89,15 +105,6 @@ public void setClientname(@Nullable String clientName) { } } - - public void addAdditionalProblem(@NotNull ConfigProblem problem) { - additionalProblems.add(problem); - } - - public void clearAdditionalProblems() { - additionalProblems.clear(); - } - @Override public boolean equals(Object o) { if (o == null || ! getClass().equals(o.getClass())) { diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/EnvCompositePart.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/EnvCompositePart.java index babef0ad..3f30cb9b 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/EnvCompositePart.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/EnvCompositePart.java @@ -85,6 +85,12 @@ public String getRawPort() { return configParts.getRawPort(); } + @NotNull + @Override + public ConfigPart copy() { + return new EnvCompositePart(vcsRoot); + } + @Override public boolean hasServerNameSet() { return configParts.hasServerNameSet(); @@ -424,6 +430,12 @@ public boolean reload() { public Collection getConfigProblems() { return Collections.emptyList(); } + + @NotNull + @Override + public ConfigPart copy() { + return new EnvPassword(); + } } diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/FileConfigPart.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/FileConfigPart.java index 5de81c00..8fd0c710 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/FileConfigPart.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/FileConfigPart.java @@ -216,6 +216,12 @@ public String getRawPort() { return rawPort; } + @NotNull + @Override + public ConfigPart copy() { + return new FileConfigPart(vcsRoot, filePath); + } + @Nls @NotNull @Override diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/RequirePasswordDataPart.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/RequirePasswordDataPart.java index 625ac6e4..cd8e9ef6 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/RequirePasswordDataPart.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/RequirePasswordDataPart.java @@ -15,6 +15,7 @@ package net.groboclown.p4.server.impl.config.part; import com.intellij.openapi.vfs.VirtualFile; +import net.groboclown.p4.server.api.config.part.ConfigPart; import net.groboclown.p4.server.api.config.part.ConfigPartAdapter; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -30,6 +31,7 @@ public RequirePasswordDataPart() { } // for ConfigStateProvider + @SuppressWarnings("unused") public RequirePasswordDataPart(String sourceName, VirtualFile vcsRoot, Map values) { super(sourceName); } @@ -82,4 +84,10 @@ public int hashCode() { public Map getState() { return Collections.emptyMap(); } + + @NotNull + @Override + public ConfigPart copy() { + return new RequirePasswordDataPart(); + } } diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/ServerFingerprintDataPart.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/ServerFingerprintDataPart.java index edce64f5..39d5ed83 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/ServerFingerprintDataPart.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/ServerFingerprintDataPart.java @@ -17,6 +17,7 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import net.groboclown.p4.server.api.config.ConfigProblem; +import net.groboclown.p4.server.api.config.part.ConfigPart; import net.groboclown.p4.server.api.config.part.ConfigPartAdapter; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -35,6 +36,7 @@ public ServerFingerprintDataPart(@Nls @NotNull String sourceName) { } // for ConfigStateProvider + @SuppressWarnings("unused") public ServerFingerprintDataPart(@NotNull String sourceName, VirtualFile root, @NotNull Map values) { super(sourceName); @@ -96,4 +98,10 @@ public Map getState() { ret.put("f", fingerprint); return ret; } + + @NotNull + @Override + public ConfigPart copy() { + return new ServerFingerprintDataPart(fingerprint); + } } diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/SimpleDataPart.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/SimpleDataPart.java index 6139b48c..4cfc97e7 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/SimpleDataPart.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/SimpleDataPart.java @@ -79,6 +79,12 @@ public String getRawPort() { return trimmedProperty(PORT_KEY); } + @NotNull + @Override + public ConfigPart copy() { + return new SimpleDataPart(vcsRoot, sourceName, properties); + } + // Password must be carefully stored. @Override diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/WinRegDataPart.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/WinRegDataPart.java index 26717487..84f13d20 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/WinRegDataPart.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/config/part/WinRegDataPart.java @@ -67,6 +67,8 @@ class WinRegDataPart implements ConfigPart { private String charset; private String loginSso; + private final boolean isUserReg; + static boolean isAvailable() { return available && SystemInfo.isWindows && JnaWinRegistry.isAvailable(); @@ -77,6 +79,7 @@ static void setAvailable(boolean avail) { } WinRegDataPart(boolean userReg) { + this.isUserReg = userReg; if (userReg) { hive = JnaWinRegistry.HKEY_CURRENT_USER; keys = USER_KEYS; @@ -158,6 +161,12 @@ public String getRawPort() { return rawPort; } + @NotNull + @Override + public ConfigPart copy() { + return new WinRegDataPart(isUserReg); + } + @Nls @NotNull @Override diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/P4RequestErrorHandler.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/P4RequestErrorHandler.java index ca493972..41d5542b 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/P4RequestErrorHandler.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/P4RequestErrorHandler.java @@ -21,6 +21,7 @@ import net.groboclown.p4.server.api.P4CommandRunner; import net.groboclown.p4.server.api.P4ServerName; import net.groboclown.p4.server.api.config.ClientConfig; +import net.groboclown.p4.server.api.config.OptionalClientServerConfig; import net.groboclown.p4.server.api.config.ServerConfig; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -223,6 +224,17 @@ public P4ServerName getServerName() { return serverName; } + @Nullable + public OptionalClientServerConfig getOptionalClientServerConfig() { + if (clientConfig != null) { + return new OptionalClientServerConfig(clientConfig); + } + if (serverConfig != null) { + return new OptionalClientServerConfig(serverConfig, null); + } + return null; + } + @Override public String toString() { if (clientConfig != null) { diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/impl/MessageP4RequestErrorHandler.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/impl/MessageP4RequestErrorHandler.java index 7415e3da..2101ddec 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/impl/MessageP4RequestErrorHandler.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/connection/impl/MessageP4RequestErrorHandler.java @@ -322,7 +322,9 @@ protected P4CommandRunner.ServerResultException handleException( // and fall through. } else if (cause instanceof UnknownHostException) { ConnectionErrorMessage.send().unknownServer(new ServerErrorEvent.ServerNameErrorEvent<>( - info.getClientConfig(), e)); + info.getServerName(), + info.getOptionalClientServerConfig(), + e)); return createServerResultException(e, getMessage("error.UnknownServerException", e), P4CommandRunner.ErrorCategory.CONNECTION); @@ -553,13 +555,14 @@ private P4CommandRunner.ServerResultException handleAuthenticationFailureType( @NotNull Exception sourceException, @NotNull AuthenticationFailedException sourceAfe) { AuthenticationFailedException.ErrorType errorType = sourceAfe.getErrorType(); + OptionalClientServerConfig clientServerConfig = info.getOptionalClientServerConfig(); LOG.debug("Handling login failure " + errorType + " for " + info, new Exception()); switch (errorType) { case SESSION_EXPIRED: - if (info.hasServerConfig()) { + if (clientServerConfig != null) { // TODO could be handled before reaching here. LoginFailureMessage.send().sessionExpired(new ServerErrorEvent.ServerConfigErrorEvent<>( - info.getClientConfig(), sourceAfe)); + clientServerConfig, sourceAfe)); return createServerResultException(sourceException, getMessage("error.AuthenticationFailedException.SESSION_EXPIRED", sourceException), P4CommandRunner.ErrorCategory.ACCESS_DENIED); @@ -572,9 +575,9 @@ private P4CommandRunner.ServerResultException handleAuthenticationFailureType( P4CommandRunner.ErrorCategory.INTERNAL); } case PASSWORD_INVALID: - if (info.hasServerConfig()) { + if (clientServerConfig != null) { LoginFailureMessage.send().passwordInvalid(new ServerErrorEvent.ServerConfigErrorEvent<>( - info.getClientConfig(), sourceAfe)); + clientServerConfig, sourceAfe)); return createServerResultException(sourceException, getMessage("error.AuthenticationFailedException.PASSWORD_INVALID", sourceException), P4CommandRunner.ErrorCategory.ACCESS_DENIED); @@ -587,15 +590,13 @@ private P4CommandRunner.ServerResultException handleAuthenticationFailureType( P4CommandRunner.ErrorCategory.INTERNAL); } case NOT_LOGGED_IN: - if (info.hasServerConfig()) { + if (clientServerConfig != null) { // Server required a login, but it wasn't done, most probably because the password isn't known, // so no login attempt was made (see PASSWORD_UNNECESSARY below). We'll classify this as a // invalid password error, and it's up to the listener to figure out if a password should be // asked for. LoginFailureMessage.send().passwordInvalid( - new ServerErrorEvent.ServerConfigErrorEvent<>( - new OptionalClientServerConfig(info.getServerConfig(), info.getClientConfig()), - sourceAfe)); + new ServerErrorEvent.ServerConfigErrorEvent<>(clientServerConfig, sourceAfe)); return createServerResultException(sourceException, getMessage("error.AuthenticationFailedException.PASSWORD_INVALID", sourceException), P4CommandRunner.ErrorCategory.ACCESS_DENIED); @@ -609,9 +610,9 @@ private P4CommandRunner.ServerResultException handleAuthenticationFailureType( P4CommandRunner.ErrorCategory.INTERNAL); } case SSO_LOGIN: - if (info.hasServerConfig()) { + if (clientServerConfig != null) { LoginFailureMessage.send().singleSignOnFailed(new ServerErrorEvent.ServerConfigErrorEvent<>( - info.getClientConfig(), sourceAfe)); + clientServerConfig, sourceAfe)); return createServerResultException(sourceException, getMessage("error.AuthenticationFailedException.SSO_LOGIN", sourceException), P4CommandRunner.ErrorCategory.ACCESS_DENIED); @@ -625,12 +626,12 @@ private P4CommandRunner.ServerResultException handleAuthenticationFailureType( P4CommandRunner.ErrorCategory.INTERNAL); } case PASSWORD_UNNECESSARY: - if (info.hasServerConfig()) { + if (clientServerConfig != null) { // By having an explicit message for an unnecessary password, the // rest of the code could perform corrective action. // TODO could be handled before reaching here. LoginFailureMessage.send().passwordUnnecessary(new ServerErrorEvent.ServerConfigErrorEvent<>( - info.getClientConfig(), sourceAfe)); + clientServerConfig, sourceAfe)); return createServerResultException(sourceException, getMessage("error.AuthenticationFailedException.PASSWORD_UNNECESSARY", sourceException), P4CommandRunner.ErrorCategory.ACCESS_DENIED); diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/util/DirectoryMappingUtil.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/util/DirectoryMappingUtil.java deleted file mode 100644 index 6dbd9060..00000000 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/util/DirectoryMappingUtil.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * 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.p4.server.impl.util; - -import com.intellij.openapi.project.Project; -import com.intellij.openapi.vcs.VcsDirectoryMapping; -import com.intellij.openapi.vfs.VirtualFile; -import com.intellij.vcsUtil.VcsUtil; -import net.groboclown.p4.server.api.util.ProjectUtil; -import org.jetbrains.annotations.NotNull; - -public class DirectoryMappingUtil { - - @NotNull - public static VirtualFile getDirectory(@NotNull Project project, @NotNull VcsDirectoryMapping directoryMapping) { - String dir = directoryMapping.getDirectory(); - if (dir == null || dir.length() <= 0) { - return ProjectUtil.guessProjectBaseDir(project); - } - VirtualFile ret = VcsUtil.getVirtualFile(dir); - if (ret == null) { - return ProjectUtil.guessProjectBaseDir(project); - } - return ret; - } -} diff --git a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/util/RootSettingsUtil.java b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/util/RootSettingsUtil.java index 4ebe923f..2b8fbe12 100644 --- a/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/util/RootSettingsUtil.java +++ b/idea-p4server/impl/src/main/java/net/groboclown/p4/server/impl/util/RootSettingsUtil.java @@ -20,8 +20,10 @@ import com.intellij.openapi.vcs.VcsDirectoryMapping; import com.intellij.openapi.vcs.VcsRootSettings; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.vcsUtil.VcsUtil; import net.groboclown.p4.server.api.config.P4VcsRootSettings; import net.groboclown.p4.server.api.config.part.ConfigPart; +import net.groboclown.p4.server.api.util.ProjectUtil; import net.groboclown.p4.server.impl.config.P4VcsRootSettingsImpl; import org.jetbrains.annotations.NotNull; @@ -30,6 +32,28 @@ public class RootSettingsUtil { private static final Logger LOG = Logger.getInstance(RootSettingsUtil.class); + @NotNull + public static VirtualFile getDirectory(@NotNull Project project, @NotNull VcsDirectoryMapping directoryMapping) { + String dir = directoryMapping.getDirectory(); + if (dir == null || dir.length() <= 0) { + return ProjectUtil.guessProjectBaseDir(project); + } + VirtualFile ret = VcsUtil.getVirtualFile(dir); + if (ret == null) { + ret = ProjectUtil.guessProjectBaseDir(project); + LOG.info("VcsDirectoryMapping has directory " + dir + + " which could not be turned into a virtual file; using " + ret); + } + return ret; + } + + + @NotNull + public static P4VcsRootSettings getFixedRootSettings(@NotNull Project project, @NotNull VcsDirectoryMapping mapping) { + return getFixedRootSettings(project, mapping, getDirectory(project, mapping)); + } + + @NotNull public static P4VcsRootSettings getFixedRootSettings( @NotNull Project project, @@ -62,7 +86,7 @@ public static P4VcsRootSettings getFixedRootSettings( // This shouldn't happen, but it does. Instead, the mapping is supposed // to be created through the createEmptyVcsRootSettings() method. // That's reflected in the deprecation of setRootSettings. - LOG.warn("Encountered empty root settings in directory mapping."); + LOG.info("Encountered empty root settings in directory mapping."); if (LOG.isDebugEnabled()) { LOG.debug("Using root path " + rootDir); } diff --git a/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/connection/ConnectionTreeCellRenderer.java b/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/connection/ConnectionTreeCellRenderer.java index 2079607a..b6d4bcd9 100644 --- a/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/connection/ConnectionTreeCellRenderer.java +++ b/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/connection/ConnectionTreeCellRenderer.java @@ -29,6 +29,7 @@ import javax.swing.*; import javax.swing.tree.DefaultMutableTreeNode; +import java.io.File; public class ConnectionTreeCellRenderer extends ColoredTreeCellRenderer { @@ -97,8 +98,9 @@ private void renderClientConfigRoot(ClientConfigRoot value) { String rootPath = value.getProjectVcsRootDir().getPath(); if (rootPath.startsWith("file://")) { rootPath = rootPath.substring(7); - // TODO normalize the path for Windows? } + // Normalize path for Windows + rootPath = new File(rootPath).getAbsolutePath(); append(P4Bundle.message("connection.tree.root.path", rootPath), ROOT_PATH_STYLE); if (value.isOnline()) { diff --git a/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/connection/VcsRootNode.java b/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/connection/VcsRootNode.java deleted file mode 100644 index 0438d5f4..00000000 --- a/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/connection/VcsRootNode.java +++ /dev/null @@ -1,20 +0,0 @@ -/* - * 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.p4plugin.ui.connection; - -import javax.swing.tree.DefaultMutableTreeNode; - -public class VcsRootNode extends DefaultMutableTreeNode { -} diff --git a/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/vcsroot/P4RootConfigPanel.java b/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/vcsroot/P4RootConfigPanel.java index 0dd13c50..f7cf696e 100644 --- a/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/vcsroot/P4RootConfigPanel.java +++ b/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/vcsroot/P4RootConfigPanel.java @@ -164,7 +164,7 @@ private String getResolvedProperties(ConfigPart part) { P4Bundle.getString("configuration.resolve.password.set")); List keys = new ArrayList<>(props.keySet()); Collections.sort(keys); - StringBuilder sb = new StringBuilder(); + StringBuilder sb = new StringBuilder(P4Bundle.message("configuration.resolve.header", vcsRoot)); for (String key : keys) { sb.append(key).append('=').append(props.get(key)).append('\n'); } diff --git a/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/vcsroot/P4VcsRootConfigurable.java b/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/vcsroot/P4VcsRootConfigurable.java index e30e6a61..4e2dea0e 100644 --- a/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/vcsroot/P4VcsRootConfigurable.java +++ b/plugin-v3/src/main/java/net/groboclown/p4plugin/ui/vcsroot/P4VcsRootConfigurable.java @@ -31,7 +31,6 @@ import net.groboclown.p4.server.api.config.ServerConfig; import net.groboclown.p4.server.api.config.part.ConfigPart; import net.groboclown.p4.server.api.config.part.MultipleConfigPart; -import net.groboclown.p4.server.impl.util.DirectoryMappingUtil; import net.groboclown.p4plugin.P4Bundle; import net.groboclown.p4plugin.components.P4ServerComponent; import net.groboclown.p4plugin.ui.WrapperPanel; @@ -43,6 +42,7 @@ import javax.swing.*; import java.util.Collection; import java.util.List; +import java.util.stream.Collectors; public class P4VcsRootConfigurable implements UnnamedConfigurable { private static final Logger LOG = Logger.getInstance(P4VcsRootConfigurable.class); @@ -50,16 +50,17 @@ public class P4VcsRootConfigurable implements UnnamedConfigurable { private final Project project; private final VirtualFile vcsRoot; private final VcsDirectoryMapping mapping; + private final P4VcsRootSettings settings; private P4RootConfigPanel panel; private Controller controller; public P4VcsRootConfigurable(Project project, VcsDirectoryMapping mapping) { this.project = project; - this.vcsRoot = DirectoryMappingUtil.getDirectory(project, mapping); + this.settings = RootSettingsUtil.getFixedRootSettings(project, mapping); + this.vcsRoot = settings.getRootDir(); if (LOG.isDebugEnabled()) { LOG.debug("Creating configurable for vcs root " + vcsRoot); } - RootSettingsUtil.getFixedRootSettings(project, mapping, vcsRoot); this.mapping = mapping; } @@ -94,7 +95,7 @@ public void apply() if (isModified()) { LOG.info("Updating root configuration for " + vcsRoot); MultipleConfigPart parentPart = loadParentPartFromUI(); - RootSettingsUtil.getFixedRootSettings(project, mapping, vcsRoot).setConfigParts(parentPart.getChildren()); + settings.setConfigParts(parentPart.getChildren()); if (parentPart.hasError()) { problems = parentPart.getConfigProblems(); @@ -126,13 +127,16 @@ public void reset() { @Override public void disposeUIResources() { + if (LOG.isDebugEnabled()) { + LOG.debug("Disposing VCS root configuration GUI for root " + vcsRoot); + } panel = null; controller = null; } private List loadPartsFromSettings() { P4VcsRootSettings settings = getRootSettings(); - return settings.getConfigParts(); + return settings.getConfigParts().stream().map(ConfigPart::copy).collect(Collectors.toList()); } private ConfigPart loadParentPartFromSettings() { diff --git a/plugin-v3/src/main/resources/META-INF/plugin.xml b/plugin-v3/src/main/resources/META-INF/plugin.xml index 3293843e..467abefc 100644 --- a/plugin-v3/src/main/resources/META-INF/plugin.xml +++ b/plugin-v3/src/main/resources/META-INF/plugin.xml @@ -1,21 +1,20 @@ Perforce IDEA Community Integration PerforceIC - 0.10.14 + 0.10.15 VCS Integration +
  • 0.10.15
      +
    • The VCS root directory options now better sets values independently between roots.
      • +
    • Fixed some NPEs caused by certain error and logging conditions.
      • +
    • Normalized the path view in the connections panel for Windows.
      • +
  • 0.10.13 and 0.10.14
    • Quick patch to incorrect "add" to a read-only list.
    • -
  • 0.10.12
      -
    • Altered the server connection to attempt to use the client configuration, even for calls that don't strictly need it.
      • -
    • Fixed an issue where diff display could show a diff against a changelist instead of a revision number.
      • -
    • Fixed an issue where the root paths would not be mapped to the client configuration correctly.
      • -
    • Cleaned up some of the logging.
      • -
    • ]]>