Skip to content

Commit 16034d6

Browse files
groboclowngroboclown
committed
Fix panel apply button, fix some of the password lookup issues.
1 parent f607b9a commit 16034d6

18 files changed

+149
-89
lines changed

‎CHANGES.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@
2626
configuration so it now reports a warning rather than throwing an
2727
error (#144).
2828
* Added validation check to ensure client name is not purely numeric.
29+
* Added better logging in the case of connection checkout issues.
30+
* Fixed configuration panel detection of modification.
31+
* Fixed an issue where an invalid password would be considered needing to log in
32+
again with the existing, known password.
2933
* Temporarily disabled the client name refresh button until that's all worked out.
3034

3135

‎plugin/META-INF/plugin.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
configuration so it now reports a warning rather than throwing an
2626
error.</li>
2727
<li>Added validation check to ensure client name is not purely numeric.</li>
28+
<li>Added better logging in the case of connection checkout issues.</li>
29+
<li>Fixed configuration panel detection of modification.</li>
2830
<li>Temporarily disabled the client name refresh button until that's all worked out.</li>
2931
</ol>
3032
</li>

‎plugin/src/net/groboclown/idea/p4ic/config/part/SimpleDataPart.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public void setServerName(@Nullable String port) {
152152
setTrimmed(PORT_KEY, port);
153153
}
154154

155-
public void setServerName(@Nullable P4ServerName port) {
155+
private void setServerName(@Nullable P4ServerName port) {
156156
if (port != null) {
157157
setTrimmed(PORT_KEY, port.getFullPort());
158158
} else {

‎plugin/src/net/groboclown/idea/p4ic/server/exceptions/ExceptionUtil.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,7 @@ public static boolean isSessionExpiredProblem(@NotNull P4JavaException ex) {
9898
RequestException rex = (RequestException) ex;
9999
return (rex.hasMessageFragment(SESSION_EXPIRED_MESSAGE_1)
100100
|| rex.hasMessageFragment(SESSION_EXPIRED_MESSAGE_2)
101-
|| rex.hasMessageFragment(SESSION_EXPIRED_MESSAGE_3)
102-
|| (rex.getGenericCode() == MessageGenericCode.EV_CONFIG &&
103-
rex.getSubCode() == 21));
101+
|| rex.hasMessageFragment(SESSION_EXPIRED_MESSAGE_3));
104102
}
105103
if (ex instanceof AuthenticationFailedException) {
106104
AuthenticationFailedException afex = (AuthenticationFailedException) ex;

‎plugin/src/net/groboclown/idea/p4ic/ui/P4MultipleConnectionWidget.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -415,14 +415,12 @@ private class ConnectionStateListener implements
415415
@Override
416416
public void configUpdated(@NotNull Project project, @NotNull P4ProjectConfig newConfig,
417417
@Nullable P4ProjectConfig previousConfiguration) {
418-
// Just update everything.
419-
reloadServerRefs();
418+
update(true, true);
420419
}
421420

422421
@Override
423422
public void configurationProblem(@NotNull Project project, @NotNull P4ProjectConfig config, @NotNull VcsConnectionProblem ex) {
424-
// Just update everything.
425-
reloadServerRefs();
423+
update(true, true);
426424
}
427425

428426
@Override

‎plugin/src/net/groboclown/idea/p4ic/ui/config/props/ClientNameConfigPartPanel.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@
2222
import net.groboclown.idea.p4ic.background.BackgroundAwtActionRunner;
2323
import net.groboclown.idea.p4ic.config.ClientConfig;
2424
import net.groboclown.idea.p4ic.config.ConfigProblem;
25-
import net.groboclown.idea.p4ic.config.ConfigPropertiesUtil;
2625
import net.groboclown.idea.p4ic.config.P4ProjectConfig;
2726
import net.groboclown.idea.p4ic.config.part.ClientNameDataPart;
27+
import net.groboclown.idea.p4ic.util.EqualUtil;
2828
import net.groboclown.idea.p4ic.v2.server.connection.ConnectionUIConfiguration;
2929
import org.jetbrains.annotations.Nls;
3030
import org.jetbrains.annotations.NotNull;
@@ -91,10 +91,11 @@ private void createUIComponents() {
9191

9292
@Override
9393
public boolean isModified(@NotNull ClientNameDataPart originalPart) {
94-
if (originalPart.getClientname() == null) {
95-
return getSelectedClientName() == null;
94+
if (LOG.isDebugEnabled()) {
95+
LOG.debug("Checking if orig (" + originalPart.getClientname() + ") differs from `"
96+
+ getSelectedClientName() + "`");
9697
}
97-
return getSelectedClientName() != null && originalPart.getClientname().equals(getSelectedClientName());
98+
return ! EqualUtil.isEqual(originalPart.getClientname(), getSelectedClientName());
9899
}
99100

100101
@Nls

‎plugin/src/net/groboclown/idea/p4ic/ui/config/props/ConfigPartPanel.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
import org.jetbrains.annotations.NotNull;
2424
import org.jetbrains.annotations.Nullable;
2525

26-
import java.awt.*;
27-
2826
public abstract class ConfigPartPanel<T extends ConfigPart>
2927
implements RequestConfigurationUpdateListener,
3028
ComponentListPanel.WithRootPanel {
@@ -33,11 +31,22 @@ public abstract class ConfigPartPanel<T extends ConfigPart>
3331

3432
private final Project project;
3533
private final T part;
34+
private final Class<T> type;
3635
private RequestConfigurationLoadListener requestConfigurationLoadListener;
3736

37+
@SuppressWarnings("unchecked")
3838
ConfigPartPanel(@NotNull Project project, @NotNull T part) {
3939
this.project = project;
4040
this.part = part;
41+
this.type = (Class<T>) part.getClass();
42+
}
43+
44+
@Nullable
45+
T castAs(@NotNull ConfigPart part) {
46+
if (type.isInstance(part)) {
47+
return type.cast(part);
48+
}
49+
return null;
4150
}
4251

4352
/**

‎plugin/src/net/groboclown/idea/p4ic/ui/config/props/ConfigStackPanel.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,14 +329,20 @@ public boolean isModified(@NotNull P4ProjectConfigComponent configComponent) {
329329
final Iterator<ConfigPartPanel<?>> nows = partPanels.iterator();
330330
while (origs.hasNext() && nows.hasNext()) {
331331
ConfigPart orig = origs.next();
332-
ConfigPart now = nows.next().getConfigPart();
333-
if (!orig.equals(now)) {
332+
ConfigPartPanel<?> now = nows.next();
333+
if (isModified(now, orig)) {
334334
return true;
335335
}
336336
}
337337
return false;
338338
}
339339

340+
private <T extends ConfigPart> boolean isModified(@NotNull ConfigPartPanel<T> panel, @NotNull ConfigPart part) {
341+
T casted = panel.castAs(part);
342+
return casted == null || panel.isModified(casted);
343+
}
344+
345+
340346
@NotNull
341347
@Override
342348
public P4ProjectConfig updateConfigPartFromUI() {

‎plugin/src/net/groboclown/idea/p4ic/ui/config/props/EnvConfigPartPanel.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import com.intellij.openapi.project.Project;
1818
import net.groboclown.idea.p4ic.P4Bundle;
19-
import net.groboclown.idea.p4ic.config.P4ProjectConfig;
2019
import net.groboclown.idea.p4ic.config.part.EnvCompositePart;
2120
import org.jetbrains.annotations.Nls;
2221
import org.jetbrains.annotations.NotNull;

‎plugin/src/net/groboclown/idea/p4ic/ui/config/props/FileConfigPartPanel.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818
import com.intellij.openapi.fileChooser.FileChooserDescriptor;
1919
import com.intellij.openapi.project.Project;
2020
import com.intellij.openapi.ui.TextFieldWithBrowseButton;
21+
import com.intellij.openapi.util.io.FileUtil;
2122
import net.groboclown.idea.p4ic.P4Bundle;
22-
import net.groboclown.idea.p4ic.config.P4ProjectConfig;
2323
import net.groboclown.idea.p4ic.config.part.FileDataPart;
2424
import org.jetbrains.annotations.Nls;
2525
import org.jetbrains.annotations.NotNull;
@@ -73,9 +73,7 @@ FileDataPart copyPart() {
7373

7474
@Override
7575
public boolean isModified(@NotNull FileDataPart originalPart) {
76-
return (originalPart.getConfigFile() == null && getSelectedLocation() != null)
77-
|| (originalPart.getConfigFile() != null &&
78-
!originalPart.getConfigFile().getParent().equals(getSelectedLocation()));
76+
return ! FileUtil.filesEqual(originalPart.getConfigFile(), getSelectedFile());
7977
}
8078

8179
@Nls
@@ -93,12 +91,16 @@ public JPanel getRootPanel() {
9391

9492
@Override
9593
public void updateConfigPartFromUI() {
96-
String newLocation = getSelectedLocation();
97-
if (newLocation == null) {
98-
getConfigPart().setConfigFile(null);
99-
} else {
100-
getConfigPart().setConfigFile(new File(newLocation));
94+
getConfigPart().setConfigFile(getSelectedFile());
95+
}
96+
97+
@Nullable
98+
private File getSelectedFile() {
99+
final String location = getSelectedLocation();
100+
if (location == null) {
101+
return null;
101102
}
103+
return new File(location);
102104
}
103105

104106
@Nullable

‎plugin/src/net/groboclown/idea/p4ic/ui/config/props/PropertyConfigPanel.java

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
package net.groboclown.idea.p4ic.ui.config.props;
1616

17-
import com.intellij.openapi.fileChooser.FileChooserDescriptor;
17+
import com.intellij.openapi.diagnostic.Logger;
1818
import com.intellij.openapi.fileChooser.FileChooserDescriptorFactory;
1919
import com.intellij.openapi.project.Project;
2020
import com.intellij.openapi.ui.TextFieldWithBrowseButton;
@@ -30,10 +30,14 @@
3030

3131
import javax.swing.*;
3232
import java.io.File;
33+
import java.util.Collections;
3334
import java.util.ResourceBundle;
3435

3536
public class PropertyConfigPanel
3637
extends ConfigPartPanel<SimpleDataPart> {
38+
private static final Logger LOG = Logger.getInstance(PropertyConfigPanel.class);
39+
40+
3741
private JPanel rootPanel;
3842
private JLabel portFieldLabel;
3943
private JTextField portField;
@@ -111,14 +115,19 @@ public JPanel getRootPanel() {
111115

112116
@Override
113117
public void updateConfigPartFromUI() {
114-
getConfigPart().setDefaultCharset(charsetField.getText());
115-
getConfigPart().setIgnoreFilename(ignoreFileNameField.getText());
116-
getConfigPart().setClientHostname(hostnameField.getText());
117-
getConfigPart().setTrustTicketFile(trustTicketFileField.getText());
118-
getConfigPart().setAuthTicketFile(authTicketFileField.getText());
119-
getConfigPart().setUsername(userField.getText());
120-
getConfigPart().setServerName(portField.getText());
121-
getConfigPart().setLoginSsoFile(loginSsoField.getText());
118+
updateConfigPartFromUI(getConfigPart());
119+
}
120+
121+
private void updateConfigPartFromUI(@NotNull SimpleDataPart part) {
122+
part.setClientname(null);
123+
part.setDefaultCharset(charsetField.getText());
124+
part.setIgnoreFilename(ignoreFileNameField.getText());
125+
part.setClientHostname(hostnameField.getText());
126+
part.setTrustTicketFile(trustTicketFileField.getText());
127+
part.setAuthTicketFile(authTicketFileField.getText());
128+
part.setUsername(userField.getText());
129+
part.setServerName(portField.getText());
130+
part.setLoginSsoFile(loginSsoField.getText());
122131
}
123132

124133
@NotNull
@@ -129,18 +138,15 @@ SimpleDataPart copyPart() {
129138

130139
@Override
131140
public boolean isModified(@NotNull SimpleDataPart originalPart) {
132-
return !isNotEqual(charsetField, originalPart.getDefaultCharset())
133-
&& !isNotEqual(ignoreFileNameField, originalPart.getIgnoreFileName())
134-
&& !isNotEqual(hostnameField, originalPart.getClientHostname())
135-
&& !isNotEqual(trustTicketFileField, originalPart.getTrustTicketFile())
136-
&& !isNotEqual(authTicketFileField, originalPart.getTrustTicketFile())
137-
&& !isNotEqual(userField, originalPart.getUsername())
138-
&& !isNotEqual(portField, originalPart.getRawServerName())
139-
&& !isNotEqual(loginSsoField, originalPart.getLoginSso());
141+
// Accurate is-modified requires creating a new part, so that the real values
142+
// can be compared.
143+
SimpleDataPart newPart = new SimpleDataPart(getProject(), Collections.<String, String>emptyMap());
144+
updateConfigPartFromUI(newPart);
145+
return ! newPart.equals(originalPart);
140146
}
141147

142148
private static boolean isNotEqual(@NotNull JTextField field, @Nullable String value) {
143-
return EqualUtil.isEqual(field.getText(), value);
149+
return ! EqualUtil.isEqual(field.getText(), value);
144150
}
145151

146152
private static boolean isNotEqual(@NotNull TextFieldWithBrowseButton field, @Nullable File file) {
@@ -150,7 +156,7 @@ private static boolean isNotEqual(@NotNull TextFieldWithBrowseButton field, @Nul
150156
} else {
151157
f = new File(field.getText());
152158
}
153-
return FileUtil.filesEqual(f, file);
159+
return ! FileUtil.filesEqual(f, file);
154160
}
155161

156162
@NotNull
@@ -164,7 +170,8 @@ private static String nullEmptyTrim(@Nullable String str) {
164170
@NotNull
165171
private static String nullEmptyFile(@NotNull Project project, @Nullable File f) {
166172
if (f == null) {
167-
return project.getBaseDir().getPath();
173+
// return project.getBaseDir().getPath();
174+
return "";
168175
}
169176
return f.getAbsolutePath();
170177
}

‎plugin/src/net/groboclown/idea/p4ic/ui/config/props/RelativeConfigPartPanel.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import com.intellij.openapi.project.Project;
1919
import com.intellij.openapi.util.text.StringUtil;
2020
import net.groboclown.idea.p4ic.P4Bundle;
21-
import net.groboclown.idea.p4ic.config.P4ProjectConfig;
2221
import net.groboclown.idea.p4ic.config.part.RelativeConfigCompositePart;
2322
import org.jetbrains.annotations.Nls;
2423
import org.jetbrains.annotations.NotNull;
@@ -74,7 +73,7 @@ RelativeConfigCompositePart copyPart() {
7473

7574
@Override
7675
public boolean isModified(@NotNull RelativeConfigCompositePart originalPart) {
77-
return StringUtil.equals(originalPart.getName(), getConfigPart().getName());
76+
return ! StringUtil.equals(originalPart.getName(), getConfigPart().getName());
7877
}
7978

8079
{

‎plugin/src/net/groboclown/idea/p4ic/ui/config/props/RequirePasswordConfigPartPanel.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import com.intellij.openapi.project.Project;
1818
import net.groboclown.idea.p4ic.P4Bundle;
19-
import net.groboclown.idea.p4ic.config.P4ProjectConfig;
2019
import net.groboclown.idea.p4ic.config.part.RequirePasswordDataPart;
2120
import org.jetbrains.annotations.Nls;
2221
import org.jetbrains.annotations.NotNull;

‎plugin/src/net/groboclown/idea/p4ic/ui/config/props/ServerFingerprintConfigPartPanel.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
import com.jgoodies.forms.layout.CellConstraints;
2020
import com.jgoodies.forms.layout.FormLayout;
2121
import net.groboclown.idea.p4ic.P4Bundle;
22-
import net.groboclown.idea.p4ic.config.P4ProjectConfig;
2322
import net.groboclown.idea.p4ic.config.part.ServerFingerprintDataPart;
23+
import net.groboclown.idea.p4ic.util.EqualUtil;
2424
import org.jetbrains.annotations.Nls;
2525
import org.jetbrains.annotations.NotNull;
2626

@@ -62,13 +62,13 @@ public JPanel getRootPanel() {
6262
@Override
6363
ServerFingerprintDataPart copyPart() {
6464
ServerFingerprintDataPart ret = new ServerFingerprintDataPart();
65-
ret.setServerFingerprint(getConfigPart().getServerFingerprint());
65+
ret.setServerFingerprint(fingerprintField.getText());
6666
return ret;
6767
}
6868

6969
@Override
7070
public boolean isModified(@NotNull ServerFingerprintDataPart originalPart) {
71-
return !originalPart.equals(getConfigPart());
71+
return ! EqualUtil.isEqual(originalPart.getServerFingerprint(), fingerprintField.getText());
7272
}
7373

7474
{

‎plugin/src/net/groboclown/idea/p4ic/v2/server/authentication/PasswordManager.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public OneUseString getPassword(@Nullable final Project project, @NotNull final
147147
// dispatch and in a read action, reference.
148148
ret = setMemoryPassword(config, ret);
149149
return ret;
150-
} else if (! forceFetch) {
150+
} else if (forceFetch) {
151151
LOG.warn("Could not get password because the action is called from outside the dispatch thread and in a read action.");
152152
if (LOG.isDebugEnabled()) {
153153
LOG.debug("Read action for " + key, new Throwable());
@@ -184,6 +184,7 @@ public void run() {
184184
throw new PasswordAccessedWrongException();
185185
} else {
186186
// The user did not force a password fetch.
187+
LOG.debug("Cannot access password at this time; using a blank password");
187188
return new OneUseString((char[]) null);
188189
}
189190
}
@@ -350,7 +351,9 @@ private OneUseString getMemoryPassword(@NotNull ServerConfig config) {
350351
if (pass == null) {
351352
return null;
352353
}
353-
return new OneUseString(pass);
354+
char[] copy = new char[pass.length];
355+
System.arraycopy(pass, 0, copy, 0, pass.length);
356+
return new OneUseString(copy);
354357
}
355358

356359
private void clearMemoryPassword(@NotNull ServerConfig config) {

‎plugin/src/net/groboclown/idea/p4ic/v2/server/authentication/ServerAuthenticator.java

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -417,19 +417,13 @@ private AuthenticationStatus authenticate(@NotNull IOptionsServer server, @NotNu
417417
@Override
418418
public Void exec(@NotNull IOptionsServer server)
419419
throws P4JavaException {
420-
boolean useAuthTicket = config.getAuthTicket() != null;
421-
LoginOptions loginOptions = new LoginOptions(false, ! useAuthTicket);
422-
// StringBuffer authFileContents = null;
423-
// if (config.getAuthTicket() != null) {
424-
// authFileContents = loadAuthTicketContents(config.getAuthTicket());
425-
// }
426-
// server.login(knownPassword, authFileContents, loginOptions);
427-
// if (authFileContents != null) {
428-
// saveAuthTicketContents(authFileContents, config.getAuthTicket());
429-
// }
420+
boolean useAuthTicket = config.getAuthTicket() != null && config.getAuthTicket().isFile();
421+
LoginOptions loginOptions = new LoginOptions();
422+
loginOptions.setDontWriteTicket(! useAuthTicket);
423+
430424
// If the password is blank, then there's no need for the
431425
// user to log in; in fact, that wil raise an error by Perforce
432-
if (knownPassword != null && knownPassword.length() > 0) {
426+
if (knownPassword != null && ! knownPassword.isEmpty()) {
433427
server.login(knownPassword, loginOptions);
434428
LOG.debug("No issue logging in with known password");
435429
} else {

0 commit comments

Comments
 (0)