Fix panel apply button, fix some of the password lookup issues.

groboclown · Mar 5, 2017 · 16034d6 · 16034d6
1 parent f607b9a
commit 16034d6
Show file tree

Hide file tree

Showing 18 changed files with 149 additions and 89 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -26,6 +26,10 @@
       configuration so it now reports a warning rather than throwing an
       error (#144).
     * Added validation check to ensure client name is not purely numeric.
+    * Added better logging in the case of connection checkout issues.
+    * Fixed configuration panel detection of modification.
+    * Fixed an issue where an invalid password would be considered needing to log in
+      again with the existing, known password.
     * Temporarily disabled the client name refresh button until that's all worked out.
 
 

diff --git a/plugin/META-INF/plugin.xml b/plugin/META-INF/plugin.xml
@@ -25,6 +25,8 @@
                 configuration so it now reports a warning rather than throwing an
                 error.</li>
               <li>Added validation check to ensure client name is not purely numeric.</li>
+              <li>Added better logging in the case of connection checkout issues.</li>
+              <li>Fixed configuration panel detection of modification.</li>
               <li>Temporarily disabled the client name refresh button until that's all worked out.</li>
             </ol>
           </li>

diff --git a/plugin/src/net/groboclown/idea/p4ic/config/part/SimpleDataPart.java b/plugin/src/net/groboclown/idea/p4ic/config/part/SimpleDataPart.java
@@ -152,7 +152,7 @@ public void setServerName(@Nullable String port) {
         setTrimmed(PORT_KEY, port);
     }
 
-    public void setServerName(@Nullable P4ServerName port) {
+    private void setServerName(@Nullable P4ServerName port) {
         if (port != null) {
             setTrimmed(PORT_KEY, port.getFullPort());
         } else {

diff --git a/plugin/src/net/groboclown/idea/p4ic/server/exceptions/ExceptionUtil.java b/plugin/src/net/groboclown/idea/p4ic/server/exceptions/ExceptionUtil.java
@@ -98,9 +98,7 @@ public static boolean isSessionExpiredProblem(@NotNull P4JavaException ex) {
             RequestException rex = (RequestException) ex;
             return (rex.hasMessageFragment(SESSION_EXPIRED_MESSAGE_1)
                     || rex.hasMessageFragment(SESSION_EXPIRED_MESSAGE_2)
-                    || rex.hasMessageFragment(SESSION_EXPIRED_MESSAGE_3)
-                    || (rex.getGenericCode() == MessageGenericCode.EV_CONFIG &&
-                    rex.getSubCode() == 21));
+                    || rex.hasMessageFragment(SESSION_EXPIRED_MESSAGE_3));
         }
         if (ex instanceof AuthenticationFailedException) {
             AuthenticationFailedException afex = (AuthenticationFailedException) ex;

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/P4MultipleConnectionWidget.java b/plugin/src/net/groboclown/idea/p4ic/ui/P4MultipleConnectionWidget.java
@@ -415,14 +415,12 @@ private class ConnectionStateListener implements
         @Override
         public void configUpdated(@NotNull Project project, @NotNull P4ProjectConfig newConfig,
                 @Nullable P4ProjectConfig previousConfiguration) {
-            // Just update everything.
-            reloadServerRefs();
+            update(true, true);
         }
 
         @Override
         public void configurationProblem(@NotNull Project project, @NotNull P4ProjectConfig config, @NotNull VcsConnectionProblem ex) {
-            // Just update everything.
-            reloadServerRefs();
+            update(true, true);
         }
 
         @Override

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/ClientNameConfigPartPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/ClientNameConfigPartPanel.java
@@ -22,9 +22,9 @@
 import net.groboclown.idea.p4ic.background.BackgroundAwtActionRunner;
 import net.groboclown.idea.p4ic.config.ClientConfig;
 import net.groboclown.idea.p4ic.config.ConfigProblem;
-import net.groboclown.idea.p4ic.config.ConfigPropertiesUtil;
 import net.groboclown.idea.p4ic.config.P4ProjectConfig;
 import net.groboclown.idea.p4ic.config.part.ClientNameDataPart;
+import net.groboclown.idea.p4ic.util.EqualUtil;
 import net.groboclown.idea.p4ic.v2.server.connection.ConnectionUIConfiguration;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
@@ -91,10 +91,11 @@ private void createUIComponents() {
 
     @Override
     public boolean isModified(@NotNull ClientNameDataPart originalPart) {
-        if (originalPart.getClientname() == null) {
-            return getSelectedClientName() == null;
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Checking if orig (" + originalPart.getClientname() + ") differs from `"
+                    + getSelectedClientName() + "`");
         }
-        return getSelectedClientName() != null && originalPart.getClientname().equals(getSelectedClientName());
+        return ! EqualUtil.isEqual(originalPart.getClientname(), getSelectedClientName());
     }
 
     @Nls

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/ConfigPartPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/ConfigPartPanel.java
@@ -23,8 +23,6 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.awt.*;
-
 public abstract class ConfigPartPanel<T extends ConfigPart>
         implements RequestConfigurationUpdateListener,
             ComponentListPanel.WithRootPanel {
@@ -33,11 +31,22 @@ public abstract class ConfigPartPanel<T extends ConfigPart>
 
     private final Project project;
     private final T part;
+    private final Class<T> type;
     private RequestConfigurationLoadListener requestConfigurationLoadListener;
 
+    @SuppressWarnings("unchecked")
     ConfigPartPanel(@NotNull Project project, @NotNull T part) {
         this.project = project;
         this.part = part;
+        this.type = (Class<T>) part.getClass();
+    }
+
+    @Nullable
+    T castAs(@NotNull ConfigPart part) {
+        if (type.isInstance(part)) {
+            return type.cast(part);
+        }
+        return null;
     }
 
     /**

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/ConfigStackPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/ConfigStackPanel.java
@@ -329,14 +329,20 @@ public boolean isModified(@NotNull P4ProjectConfigComponent configComponent) {
         final Iterator<ConfigPartPanel<?>> nows = partPanels.iterator();
         while (origs.hasNext() && nows.hasNext()) {
             ConfigPart orig = origs.next();
-            ConfigPart now = nows.next().getConfigPart();
-            if (!orig.equals(now)) {
+            ConfigPartPanel<?> now = nows.next();
+            if (isModified(now, orig)) {
                 return true;
             }
         }
         return false;
     }
 
+    private <T extends ConfigPart> boolean isModified(@NotNull ConfigPartPanel<T> panel, @NotNull ConfigPart part) {
+        T casted = panel.castAs(part);
+        return casted == null || panel.isModified(casted);
+    }
+
+
     @NotNull
     @Override
     public P4ProjectConfig updateConfigPartFromUI() {

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/EnvConfigPartPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/EnvConfigPartPanel.java
@@ -16,7 +16,6 @@
 
 import com.intellij.openapi.project.Project;
 import net.groboclown.idea.p4ic.P4Bundle;
-import net.groboclown.idea.p4ic.config.P4ProjectConfig;
 import net.groboclown.idea.p4ic.config.part.EnvCompositePart;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/FileConfigPartPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/FileConfigPartPanel.java
@@ -18,8 +18,8 @@
 import com.intellij.openapi.fileChooser.FileChooserDescriptor;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.ui.TextFieldWithBrowseButton;
+import com.intellij.openapi.util.io.FileUtil;
 import net.groboclown.idea.p4ic.P4Bundle;
-import net.groboclown.idea.p4ic.config.P4ProjectConfig;
 import net.groboclown.idea.p4ic.config.part.FileDataPart;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
@@ -73,9 +73,7 @@ FileDataPart copyPart() {
 
     @Override
     public boolean isModified(@NotNull FileDataPart originalPart) {
-        return (originalPart.getConfigFile() == null && getSelectedLocation() != null)
-                || (originalPart.getConfigFile() != null &&
-                !originalPart.getConfigFile().getParent().equals(getSelectedLocation()));
+        return ! FileUtil.filesEqual(originalPart.getConfigFile(), getSelectedFile());
     }
 
     @Nls
@@ -93,12 +91,16 @@ public JPanel getRootPanel() {
 
     @Override
     public void updateConfigPartFromUI() {
-        String newLocation = getSelectedLocation();
-        if (newLocation == null) {
-            getConfigPart().setConfigFile(null);
-        } else {
-            getConfigPart().setConfigFile(new File(newLocation));
+        getConfigPart().setConfigFile(getSelectedFile());
+    }
+
+    @Nullable
+    private File getSelectedFile() {
+        final String location = getSelectedLocation();
+        if (location == null) {
+            return null;
         }
+        return new File(location);
     }
 
     @Nullable

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/PropertyConfigPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/PropertyConfigPanel.java
@@ -14,7 +14,7 @@
 
 package net.groboclown.idea.p4ic.ui.config.props;
 
-import com.intellij.openapi.fileChooser.FileChooserDescriptor;
+import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.fileChooser.FileChooserDescriptorFactory;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.ui.TextFieldWithBrowseButton;
@@ -30,10 +30,14 @@
 
 import javax.swing.*;
 import java.io.File;
+import java.util.Collections;
 import java.util.ResourceBundle;
 
 public class PropertyConfigPanel
         extends ConfigPartPanel<SimpleDataPart> {
+    private static final Logger LOG = Logger.getInstance(PropertyConfigPanel.class);
+
+
     private JPanel rootPanel;
     private JLabel portFieldLabel;
     private JTextField portField;
@@ -111,14 +115,19 @@ public JPanel getRootPanel() {
 
     @Override
     public void updateConfigPartFromUI() {
-        getConfigPart().setDefaultCharset(charsetField.getText());
-        getConfigPart().setIgnoreFilename(ignoreFileNameField.getText());
-        getConfigPart().setClientHostname(hostnameField.getText());
-        getConfigPart().setTrustTicketFile(trustTicketFileField.getText());
-        getConfigPart().setAuthTicketFile(authTicketFileField.getText());
-        getConfigPart().setUsername(userField.getText());
-        getConfigPart().setServerName(portField.getText());
-        getConfigPart().setLoginSsoFile(loginSsoField.getText());
+        updateConfigPartFromUI(getConfigPart());
+    }
+
+    private void updateConfigPartFromUI(@NotNull SimpleDataPart part) {
+        part.setClientname(null);
+        part.setDefaultCharset(charsetField.getText());
+        part.setIgnoreFilename(ignoreFileNameField.getText());
+        part.setClientHostname(hostnameField.getText());
+        part.setTrustTicketFile(trustTicketFileField.getText());
+        part.setAuthTicketFile(authTicketFileField.getText());
+        part.setUsername(userField.getText());
+        part.setServerName(portField.getText());
+        part.setLoginSsoFile(loginSsoField.getText());
     }
 
     @NotNull
@@ -129,18 +138,15 @@ SimpleDataPart copyPart() {
 
     @Override
     public boolean isModified(@NotNull SimpleDataPart originalPart) {
-        return !isNotEqual(charsetField, originalPart.getDefaultCharset())
-            && !isNotEqual(ignoreFileNameField, originalPart.getIgnoreFileName())
-            && !isNotEqual(hostnameField, originalPart.getClientHostname())
-            && !isNotEqual(trustTicketFileField, originalPart.getTrustTicketFile())
-            && !isNotEqual(authTicketFileField, originalPart.getTrustTicketFile())
-            && !isNotEqual(userField, originalPart.getUsername())
-            && !isNotEqual(portField, originalPart.getRawServerName())
-            && !isNotEqual(loginSsoField, originalPart.getLoginSso());
+        // Accurate is-modified requires creating a new part, so that the real values
+        // can be compared.
+        SimpleDataPart newPart = new SimpleDataPart(getProject(), Collections.<String, String>emptyMap());
+        updateConfigPartFromUI(newPart);
+        return ! newPart.equals(originalPart);
     }
 
     private static boolean isNotEqual(@NotNull JTextField field, @Nullable String value) {
-        return EqualUtil.isEqual(field.getText(), value);
+        return ! EqualUtil.isEqual(field.getText(), value);
     }
 
     private static boolean isNotEqual(@NotNull TextFieldWithBrowseButton field, @Nullable File file) {
@@ -150,7 +156,7 @@ private static boolean isNotEqual(@NotNull TextFieldWithBrowseButton field, @Nul
         } else {
             f = new File(field.getText());
         }
-        return FileUtil.filesEqual(f, file);
+        return ! FileUtil.filesEqual(f, file);
     }
 
     @NotNull
@@ -164,7 +170,8 @@ private static String nullEmptyTrim(@Nullable String str) {
     @NotNull
     private static String nullEmptyFile(@NotNull Project project, @Nullable File f) {
         if (f == null) {
-            return project.getBaseDir().getPath();
+            // return project.getBaseDir().getPath();
+            return "";
         }
         return f.getAbsolutePath();
     }

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/RelativeConfigPartPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/RelativeConfigPartPanel.java
@@ -18,7 +18,6 @@
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.text.StringUtil;
 import net.groboclown.idea.p4ic.P4Bundle;
-import net.groboclown.idea.p4ic.config.P4ProjectConfig;
 import net.groboclown.idea.p4ic.config.part.RelativeConfigCompositePart;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
@@ -74,7 +73,7 @@ RelativeConfigCompositePart copyPart() {
 
     @Override
     public boolean isModified(@NotNull RelativeConfigCompositePart originalPart) {
-        return StringUtil.equals(originalPart.getName(), getConfigPart().getName());
+        return ! StringUtil.equals(originalPart.getName(), getConfigPart().getName());
     }
 
     {

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/RequirePasswordConfigPartPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/RequirePasswordConfigPartPanel.java
@@ -16,7 +16,6 @@
 
 import com.intellij.openapi.project.Project;
 import net.groboclown.idea.p4ic.P4Bundle;
-import net.groboclown.idea.p4ic.config.P4ProjectConfig;
 import net.groboclown.idea.p4ic.config.part.RequirePasswordDataPart;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;

diff --git a/plugin/src/net/groboclown/idea/p4ic/ui/config/props/ServerFingerprintConfigPartPanel.java b/plugin/src/net/groboclown/idea/p4ic/ui/config/props/ServerFingerprintConfigPartPanel.java
@@ -19,8 +19,8 @@
 import com.jgoodies.forms.layout.CellConstraints;
 import com.jgoodies.forms.layout.FormLayout;
 import net.groboclown.idea.p4ic.P4Bundle;
-import net.groboclown.idea.p4ic.config.P4ProjectConfig;
 import net.groboclown.idea.p4ic.config.part.ServerFingerprintDataPart;
+import net.groboclown.idea.p4ic.util.EqualUtil;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 
@@ -62,13 +62,13 @@ public JPanel getRootPanel() {
     @Override
     ServerFingerprintDataPart copyPart() {
         ServerFingerprintDataPart ret = new ServerFingerprintDataPart();
-        ret.setServerFingerprint(getConfigPart().getServerFingerprint());
+        ret.setServerFingerprint(fingerprintField.getText());
         return ret;
     }
 
     @Override
     public boolean isModified(@NotNull ServerFingerprintDataPart originalPart) {
-        return !originalPart.equals(getConfigPart());
+        return ! EqualUtil.isEqual(originalPart.getServerFingerprint(), fingerprintField.getText());
     }
 
     {

diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/authentication/PasswordManager.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/authentication/PasswordManager.java
@@ -147,7 +147,7 @@ public OneUseString getPassword(@Nullable final Project project, @NotNull final
                     // dispatch and in a read action, reference.
                     ret = setMemoryPassword(config, ret);
                     return ret;
-                } else if (! forceFetch) {
+                } else if (forceFetch) {
                     LOG.warn("Could not get password because the action is called from outside the dispatch thread and in a read action.");
                     if (LOG.isDebugEnabled()) {
                         LOG.debug("Read action for " + key, new Throwable());
@@ -184,6 +184,7 @@ public void run() {
                     throw new PasswordAccessedWrongException();
                 } else {
                     // The user did not force a password fetch.
+                    LOG.debug("Cannot access password at this time; using a blank password");
                     return new OneUseString((char[]) null);
                 }
             }
@@ -350,7 +351,9 @@ private OneUseString getMemoryPassword(@NotNull ServerConfig config) {
         if (pass == null) {
             return null;
         }
-        return new OneUseString(pass);
+        char[] copy = new char[pass.length];
+        System.arraycopy(pass, 0, copy, 0, pass.length);
+        return new OneUseString(copy);
     }
 
     private void clearMemoryPassword(@NotNull ServerConfig config) {

diff --git a/plugin/src/net/groboclown/idea/p4ic/v2/server/authentication/ServerAuthenticator.java b/plugin/src/net/groboclown/idea/p4ic/v2/server/authentication/ServerAuthenticator.java
@@ -417,19 +417,13 @@ private AuthenticationStatus authenticate(@NotNull IOptionsServer server, @NotNu
             @Override
             public Void exec(@NotNull IOptionsServer server)
                     throws P4JavaException {
-                boolean useAuthTicket = config.getAuthTicket() != null;
-                LoginOptions loginOptions = new LoginOptions(false, ! useAuthTicket);
-                // StringBuffer authFileContents = null;
-                // if (config.getAuthTicket() != null) {
-                //     authFileContents = loadAuthTicketContents(config.getAuthTicket());
-                // }
-                // server.login(knownPassword, authFileContents, loginOptions);
-                // if (authFileContents != null) {
-                //     saveAuthTicketContents(authFileContents, config.getAuthTicket());
-                // }
+                boolean useAuthTicket = config.getAuthTicket() != null && config.getAuthTicket().isFile();
+                LoginOptions loginOptions = new LoginOptions();
+                loginOptions.setDontWriteTicket(! useAuthTicket);
+
                 // If the password is blank, then there's no need for the
                 // user to log in; in fact, that wil raise an error by Perforce
-                if (knownPassword != null && knownPassword.length() > 0) {
+                if (knownPassword != null && ! knownPassword.isEmpty()) {
                     server.login(knownPassword, loginOptions);
                     LOG.debug("No issue logging in with known password");
                 } else {