Fix #220 around encoding as reported by Perforce. Also included a NPE…

… fix around a timing issue. A small fix for the pact tests.
groboclown · Aug 21, 2020 · 0dc2e18 · 0dc2e18
1 parent 6018c1b
commit 0dc2e18
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 13 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,6 +1,14 @@
 # IDEA Community VCS Integration for Perforce
 
 
+## ::v0.11.1::
+
+### Overview
+
+* Fixed a bug related to an NPE on a cached component.
+* Fixed a bug where the encoding from the Perforce server wasn't supported by Java (specifically, `utf8-bom`).  It now tries to strip the "-bom" off, then will resort to the default enocding of the local computer.  (Bug #220)
+
+
 ## ::v0.11.0::
 
 ### Overview

diff --git a/...ver/impl/src/main/java/net/groboclown/p4/server/impl/connection/ConnectCommandRunner.java b/...ver/impl/src/main/java/net/groboclown/p4/server/impl/connection/ConnectCommandRunner.java
@@ -144,7 +144,9 @@
 import net.groboclown.p4.simpleswarm.exceptions.UnauthorizedAccessException;
 import org.jetbrains.annotations.NotNull;
 
+import javax.annotation.Nonnull;
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -284,12 +286,7 @@ public P4CommandRunner.QueryAnswer<AnnotateFileResult> getFileAnnotation(
             ClientServerRef ref = config.getClientServerRef();
             IExtendedFileSpec headSpec = cmd.getFileDetails(client, specs);
             P4FileRevision headRevision = P4FileRevisionImpl.getHead(ref, headSpec);
-            String content = new String(cmd.loadContents(client, headSpec),
-                    headSpec.getHeadCharset() == null
-                        ? (config.getDefaultCharSet() == null
-                            ? Charset.defaultCharset().name()
-                            : config.getDefaultCharSet())
-                        : headSpec.getHeadCharset());
+            String content = loadContentAsString(config, client, headSpec);
             List<IFileAnnotation> annotations = cmd.getAnnotations(client, specs);
             List<IFileSpec> requiredHistory = FileAnnotationParser.getRequiredHistorySpecs(annotations);
             List<Pair<IFileSpec, IFileRevisionData>> history = cmd.getExactHistory((IOptionsServer) client.getServer(), requiredHistory);
@@ -300,6 +297,32 @@ public P4CommandRunner.QueryAnswer<AnnotateFileResult> getFileAnnotation(
         }));
     }
 
+    @Nonnull
+    private String loadContentAsString(@Nonnull ClientConfig config, IClient client, IExtendedFileSpec headSpec)
+            throws P4JavaException, IOException {
+        final byte[] contents = cmd.loadContents(client, headSpec);
+        String requestedCharset = headSpec.getHeadCharset() == null
+                ? (config.getDefaultCharSet() == null
+                ? Charset.defaultCharset().name()
+                : config.getDefaultCharSet())
+                : headSpec.getHeadCharset();
+        String charset = requestedCharset;
+        if (! Charset.isSupported(charset)) {
+            LOG.warn("Perforce requested charset for encoding '" + requestedCharset + "', but Java does not support it.");
+            if (charset.toLowerCase().endsWith("-bom")) {
+                charset = charset.substring(0, charset.length() - 4);
+            }
+            if (Charset.isSupported(charset)) {
+                LOG.warn("Using " + charset + " instead.");
+            } else {
+                charset = Charset.defaultCharset().name();
+                LOG.warn("Using default charset " + charset + " instead.");
+            }
+        }
+        // Could raise an UnsupportedEncodingException.
+        return new String(contents, charset);
+    }
+
     @NotNull
     @Override
     public P4CommandRunner.QueryAnswer<DescribeChangelistResult> describeChangelist(

diff --git a/plugin-v3/src/main/java/net/groboclown/p4plugin/components/CacheComponent.java b/plugin-v3/src/main/java/net/groboclown/p4plugin/components/CacheComponent.java
@@ -169,12 +169,15 @@ private Answer<Pair<IdeChangelistMap, IdeFileMap>> refreshServerOpenedCache(Coll
                             UserProjectPreferences.getMaxFileRetrieveCount(project))
             ).getPromise()
             .whenCompleted((changesResult) -> {
-                // TODO is this a duplicate call for the updateListener's CacheListener?
-                updateListener.setOpenedChanges(
-                        changesResult.getClientConfig().getClientServerRef(),
-                        changesResult.getPendingChangelists(), changesResult.getOpenedFiles());
+                if (updateListener != null) {
+                    // TODO is this a duplicate call for the updateListener's CacheListener?
+                    updateListener.setOpenedChanges(
+                            changesResult.getClientConfig().getClientServerRef(),
+                            changesResult.getPendingChangelists(), changesResult.getOpenedFiles());
+                }
+                // Else - timing issue with dispose and when this ran.
                 sink.resolve(null);
-                if (LOG.isDebugEnabled()) {
+                if (LOG.isDebugEnabled() && changelistMap != null && fileMap != null) {
                     LOG.debug(this + " opened cache refreshed; " + clientRoot.getClientRootDir()
                             + " contains " + changelistMap.getEstimateCount() + " pending changes, "
                             + fileMap.getEstimateSize() + " opened files.");

diff --git a/swarm/src/test/groovy/net/groboclown/p4/simpleswarm/ReviewPactSpec.groovy b/swarm/src/test/groovy/net/groboclown/p4/simpleswarm/ReviewPactSpec.groovy
@@ -32,15 +32,15 @@ class ReviewPactSpec extends Specification {
         def ticket = 'ticket1'
         SwarmConfig config = new SwarmConfig()
                 .withLogger(new MockLogger())
-                .withUri("http://localhost:65001")
+                .withUri("http://localhost:60830")
                 .withUsername(username)
                 .withTicket(ticket)
                 .withVersion(6.0)
         client = new ReviewActions(config)
         provider = new PactBuilder()
         provider.serviceConsumer 'net.groboclown.p4.swarm'
         provider.hasPactWith 'Swarm-v6'
-        provider.port 65001
+        provider.port 60830
     }