Fix for #209

Change up the association of the vcs root with the directory mapping object. In some cases, the directory mapping object was incorrectly setup to point to the project root, rather than the vcs root. The dialog now carefully creates the vcs directory mapping object to have the expected data.
groboclown · Feb 25, 2020 · 7194563 · 7194563
1 parent d42861a
commit 7194563
Show file tree

Hide file tree

Showing 8 changed files with 122 additions and 55 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,5 +1,17 @@
 # IDEA Community VCS Integration for Perforce
 
+## ::v0.10.11::
+
+### Overview
+
+* Bug fixes
+
+### Details
+
+* Bug fixes
+    * Fixed how the VCS Root Directory maps to the configuration (#209).  It should now allow for multiple workspace root directories to have their own, independent configurations.
+
+
 ## ::v0.10.10::
 
 ### Overview

diff --git a/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/P4VcsRootSettings.java b/idea-p4server/api/src/main/java/net/groboclown/p4/server/api/config/P4VcsRootSettings.java
@@ -15,13 +15,21 @@
 package net.groboclown.p4.server.api.config;
 
 import com.intellij.openapi.vcs.VcsRootSettings;
+import com.intellij.openapi.vfs.VirtualFile;
 import net.groboclown.p4.server.api.config.part.ConfigPart;
+import org.jetbrains.annotations.NotNull;
 
 import java.util.List;
 
 public interface P4VcsRootSettings extends VcsRootSettings {
 
+    @NotNull
     List<ConfigPart> getConfigParts();
 
-    void setConfigParts(List<ConfigPart> parts);
+    void setConfigParts(@NotNull List<ConfigPart> parts);
+
+    boolean usesDefaultConfigParts();
+
+    @NotNull
+    VirtualFile getRootDir();
 }
diff --git a/...server/impl/src/main/java/net/groboclown/p4/server/impl/config/P4VcsRootSettingsImpl.java b/...server/impl/src/main/java/net/groboclown/p4/server/impl/config/P4VcsRootSettingsImpl.java
@@ -44,22 +44,40 @@ public P4VcsRootSettingsImpl(@NotNull Project project, @NotNull VirtualFile root
         this.rootDir = rootDir;
     }
 
+    @NotNull
+    @Override
+    public VirtualFile getRootDir() {
+        return rootDir;
+    }
+
+    @Override
+    public boolean usesDefaultConfigParts() {
+        final PersistentRootConfigComponent config = PersistentRootConfigComponent.getInstance(project);
+        if (config == null) {
+            return true;
+        }
+        List<ConfigPart> ret = config.getConfigPartsForRoot(rootDir);
+        return ret == null || ret.isEmpty();
+    }
 
     @Override
+    @NotNull
     public List<ConfigPart> getConfigParts() {
         final PersistentRootConfigComponent config = PersistentRootConfigComponent.getInstance(project);
         if (config == null) {
+            LOG.debug("No PersistentRootConfigComponent loaded; using default configuration parts.");
             return getDefaultConfigParts();
         }
         List<ConfigPart> ret = config.getConfigPartsForRoot(rootDir);
         if (ret == null || ret.isEmpty()) {
+            LOG.debug("No persisted configuration parts for root directory " + rootDir);
             return getDefaultConfigParts();
         }
         return ret;
     }
 
     @Override
-    public void setConfigParts(List<ConfigPart> parts) {
+    public void setConfigParts(@NotNull List<ConfigPart> parts) {
         for (ConfigPart part : parts) {
             if (part == null) {
                 LOG.warn("Added null part" + parts);

diff --git a/...mpl/src/main/java/net/groboclown/p4/server/impl/config/PersistentRootConfigComponent.java b/...mpl/src/main/java/net/groboclown/p4/server/impl/config/PersistentRootConfigComponent.java
@@ -65,28 +65,45 @@ public PersistentRootConfigComponent(@NotNull Project project) {
         this.project = project;
     }
 
+    @Nullable
+    public static PersistentRootConfigComponent getInstance(@NotNull Project project) {
+        return project.getComponent(COMPONENT_CLASS);
+    }
+
+    boolean hasConfigPartsForRoot(@NotNull VirtualFile root) {
+        return getConfigPartsForRoot(root) != null;
+    }
+
+    void setConfigPartsForRoot(@NotNull VirtualFile root, @NotNull List<ConfigPart> parts) {
+        List<ConfigPart> copy = Collections.unmodifiableList(new ArrayList<>(parts));
+        synchronized (sync) {
+            rootPartMap.put(root, copy);
+            if (LOG.isDebugEnabled()) {
+                // NOTE: not synchronized access for debug read.
+                LOG.debug("Stored root " + root + "; current roots stored = " + rootPartMap.keySet());
+            }
+        }
+    }
+
     @Nullable
     List<ConfigPart> getConfigPartsForRoot(@NotNull VirtualFile root) {
         List<ConfigPart> res;
         synchronized (sync) {
             res = rootPartMap.get(root);
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Retrieved config parts for root " + root + " = " + res +
+                        "; current roots stored = " + rootPartMap.keySet());
+            }
         }
         return res;
     }
 
-
-    @Nullable
-    public static PersistentRootConfigComponent getInstance(@NotNull Project project) {
-        return project.getComponent(COMPONENT_CLASS);
-    }
-
     @NotNull
     @Override
     public String getComponentName() {
         return COMPONENT_CLASS.getName();
     }
 
-
     @Override
     public void projectOpened() {
         // do nothing
@@ -106,17 +123,6 @@ public void initComponent() {
     public void disposeComponent() {
     }
 
-    boolean hasConfigPartsForRoot(@NotNull VirtualFile root) {
-        return getConfigPartsForRoot(root) != null;
-    }
-
-    void setConfigPartsForRoot(@NotNull VirtualFile root, @NotNull List<ConfigPart> parts) {
-        List<ConfigPart> copy = Collections.unmodifiableList(new ArrayList<>(parts));
-        synchronized (sync) {
-            rootPartMap.put(root, copy);
-        }
-    }
-
     @Nullable
     @Override
     public Element getState() {

diff --git a/plugin-v3/src/main/java/net/groboclown/p4plugin/extension/P4Vcs.java b/plugin-v3/src/main/java/net/groboclown/p4plugin/extension/P4Vcs.java
@@ -46,7 +46,6 @@
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.util.ThreeState;
 import com.intellij.util.messages.MessageBusConnection;
-import com.intellij.vcsUtil.VcsUtil;
 import net.groboclown.p4.server.api.P4VcsKey;
 import net.groboclown.p4.server.api.util.ProjectUtil;
 import net.groboclown.p4.server.api.values.P4CommittedChangelist;
@@ -159,7 +158,7 @@ public class P4Vcs extends AbstractVcs<P4CommittedChangelist> {
 
     private P4EditFileProvider editProvider;
 
-    private P4ProjectConfigurable myConfigurable;
+    private final P4ProjectConfigurable myConfigurable;
 
     private final P4ChangelistListener changelistListener;
 
@@ -230,7 +229,6 @@ public String getDisplayName() {
      */
     @Override
     public UnnamedConfigurable getRootConfigurable(VcsDirectoryMapping mapping) {
-        RootSettingsUtil.getFixedRootSettings(myProject, mapping);
         return new P4VcsRootConfigurable(getProject(), mapping);
     }
 

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
@@ -30,7 +30,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.config.P4VcsRootSettingsImpl;
 import net.groboclown.p4.server.impl.util.DirectoryMappingUtil;
 import net.groboclown.p4plugin.P4Bundle;
 import net.groboclown.p4plugin.components.P4ServerComponent;
@@ -56,12 +55,25 @@ public class P4VcsRootConfigurable implements UnnamedConfigurable {
     public P4VcsRootConfigurable(Project project, VcsDirectoryMapping mapping) {
         this.project = project;
         this.vcsRoot = DirectoryMappingUtil.getDirectory(project, mapping);
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Creating configurable for vcs root " + vcsRoot);
+        }
+        RootSettingsUtil.getFixedRootSettings(project, mapping, vcsRoot);
         this.mapping = mapping;
     }
 
     @Nullable
     @Override
     public JComponent createComponent() {
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Creating component for root " + vcsRoot + "; mapping " + mapping);
+            VcsRootSettings settings = mapping.getRootSettings();
+            LOG.debug("VCS Settings: " + (settings == null ? null : settings.getClass().getName()));
+            if (settings instanceof P4VcsRootSettings) {
+                P4VcsRootSettings p4settings = (P4VcsRootSettings) settings;
+                LOG.debug("P4 Settings for " + mapping.getDirectory() + ": " + p4settings.getConfigParts());
+            }
+        }
         controller = new Controller(project);
         panel = new P4RootConfigPanel(vcsRoot, controller);
         controller.configPartContainer = panel;
@@ -80,10 +92,8 @@ public void apply()
         Collection<ConfigProblem> problems = null;
         if (isModified()) {
             LOG.info("Updating root configuration for " + vcsRoot);
-            // ClientConfig oldConfig = loadConfigFromSettings();
-            P4VcsRootSettings settings = new P4VcsRootSettingsImpl(project, vcsRoot);
             MultipleConfigPart parentPart = loadParentPartFromUI();
-            RootSettingsUtil.getFixedRootSettings(project, mapping).setConfigParts(parentPart.getChildren());
+            RootSettingsUtil.getFixedRootSettings(project, mapping, vcsRoot).setConfigParts(parentPart.getChildren());
 
             if (parentPart.hasError()) {
                 problems = parentPart.getConfigProblems();
@@ -158,7 +168,7 @@ public ClientConfig loadConfigFromSettings() {
 
     @NotNull
     private P4VcsRootSettings getRootSettings() {
-        return RootSettingsUtil.getFixedRootSettings(project, mapping);
+        return RootSettingsUtil.getFixedRootSettings(project, mapping, vcsRoot);
     }
 
     @Nls(capitalization = Nls.Capitalization.Sentence)

diff --git a/plugin-v3/src/main/java/net/groboclown/p4plugin/util/RootSettingsUtil.java b/plugin-v3/src/main/java/net/groboclown/p4plugin/util/RootSettingsUtil.java
@@ -16,39 +16,62 @@
 
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.project.Project;
+import com.intellij.openapi.util.io.FileUtil;
 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.util.ProjectUtil;
+import net.groboclown.p4.server.api.config.part.ConfigPart;
 import net.groboclown.p4.server.impl.config.P4VcsRootSettingsImpl;
 import org.jetbrains.annotations.NotNull;
 
+import java.util.List;
+
 public class RootSettingsUtil {
     private static final Logger LOG = Logger.getInstance(RootSettingsUtil.class);
 
     @NotNull
     public static P4VcsRootSettings getFixedRootSettings(
             @NotNull Project project,
-            @NotNull VcsDirectoryMapping mapping) {
-        VcsRootSettings settings = mapping.getRootSettings();
-        if (settings != null && ! (settings instanceof P4VcsRootSettings)) {
-            LOG.warn("Encountered wrong root settings type in directory mapping: " + settings.getClass());
-            settings = null;
+            @NotNull VcsDirectoryMapping mapping,
+            @NotNull VirtualFile rootDir) {
+        // See #209 - This mapping, if done wrong, will cause the plugin
+        // to not associate the roots to the configuration correctly.
+
+        VcsRootSettings rawSettings = mapping.getRootSettings();
+        P4VcsRootSettings retSettings = null;
+        if (rawSettings instanceof P4VcsRootSettings) {
+            retSettings = (P4VcsRootSettings) rawSettings;
+        } else {
+            LOG.warn("Encountered wrong root settings type in directory mapping: " +
+                    (rawSettings == null ? null : rawSettings.getClass()));
         }
-        if (settings == null) {
+        List<ConfigPart> parts = null;
+        if (retSettings != null) {
+            if (! FileUtil.pathsEqual(rootDir.getPath(), retSettings.getRootDir().getPath())) {
+                LOG.info("Mapping has directory " + mapping.getDirectory() +
+                        " which does not match P4 VCS settings dir " +
+                        retSettings.getRootDir().getPath() + "; root dir " + rootDir);
+                if (! retSettings.usesDefaultConfigParts()) {
+                    parts = retSettings.getConfigParts();
+                }
+                retSettings = null;
+            }
+        }
+        if (retSettings == null) {
             // 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.");
-            VirtualFile rootDir = VcsUtil.getVirtualFile(mapping.getDirectory());
-            if (rootDir == null) {
-                rootDir = ProjectUtil.guessProjectBaseDir(project);
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Using root path " + rootDir);
+            }
+            retSettings = new P4VcsRootSettingsImpl(project, rootDir);
+            if (parts != null && retSettings.usesDefaultConfigParts()) {
+                retSettings.setConfigParts(parts);
             }
-            settings = new P4VcsRootSettingsImpl(project, rootDir);
-            mapping.setRootSettings(settings);
+            mapping.setRootSettings(retSettings);
         }
-        return (P4VcsRootSettings) settings;
+        return retSettings;
     }
 }
diff --git a/plugin-v3/src/main/resources/META-INF/plugin.xml b/plugin-v3/src/main/resources/META-INF/plugin.xml
@@ -1,19 +1,19 @@
 <idea-plugin>
     <name>Perforce IDEA Community Integration</name>
     <id>PerforceIC</id>
-    <version>0.10.10</version>
+    <version>0.10.11</version>
     <!-- see http://www.jetbrains.org/intellij/sdk/docs/basics/getting_started/build_number_ranges.html -->
     <idea-version since-build="171"/>
     <category>VCS Integration</category>
     <change-notes><![CDATA[
     <ul>
+      <li><em>0.10.11</em><ul>
+        <li>Fixes to associating configuration with the VCS Root Directory.</li>
+      </ul></li>
       <li><em>0.10.10</em><ul>
         <li>Fixed the issue where refreshing the VCS changes after submitting a change causes a
         critical failure due to IDE changelists vs Perforce cache being out-of-sync.</li>
       </ul></li>
-      <li><em>0.10.9</em><ul>
-        <li>Fixed API usage to support future IDE versions.</li>
-      </ul></li>
     </ul>
     ]]></change-notes>
     <description><![CDATA[
@@ -118,7 +118,7 @@
         <group id="P4FileActions">
             <reference ref="CheckinFiles"/>
             <action id="P4.Edit" class="net.groboclown.p4plugin.actions.P4EditAction"
-                    text="Add or edit" icon="AllIcons.Actions.Edit"
+                    text="Add or Edit" icon="AllIcons.Actions.Edit"
                     use-shortcut-of="ChangesView.AddUnversioned"/>
             <reference ref="ChangesView.Revert"/>
             <reference ref="UpdateFiles" />
@@ -190,13 +190,11 @@
 
         <!-- The per-project, top-level server connection handler. -->
         <component>
-            <interface-class>net.groboclown.p4plugin.components.P4ServerComponent</interface-class>
             <implementation-class>net.groboclown.p4plugin.components.P4ServerComponent</implementation-class>
         </component>
 
         <!-- The project-level cache of server-side objects. -->
         <component>
-            <interface-class>net.groboclown.p4plugin.components.CacheComponent</interface-class>
             <implementation-class>net.groboclown.p4plugin.components.CacheComponent</implementation-class>
         </component>
 
@@ -207,7 +205,6 @@
 
         <component>
             <implementation-class>net.groboclown.p4plugin.ui.VcsDockedComponent</implementation-class>
-            <interface-class>net.groboclown.p4plugin.ui.VcsDockedComponent</interface-class>
         </component>
 
         <component>
@@ -216,22 +213,18 @@
         </component>
 
         <component>
-            <interface-class>net.groboclown.p4plugin.components.UserErrorComponent</interface-class>
             <implementation-class>net.groboclown.p4plugin.components.UserErrorComponent</implementation-class>
         </component>
 
         <component>
-            <interface-class>net.groboclown.p4.server.impl.config.PersistentRootConfigComponent</interface-class>
             <implementation-class>net.groboclown.p4.server.impl.config.PersistentRootConfigComponent</implementation-class>
         </component>
 
         <component>
-            <interface-class>net.groboclown.p4plugin.components.CacheViewRefreshComponent</interface-class>
             <implementation-class>net.groboclown.p4plugin.components.CacheViewRefreshComponent</implementation-class>
         </component>
 
         <component>
-            <interface-class>net.groboclown.p4plugin.components.SwarmConnectionComponent</interface-class>
             <implementation-class>net.groboclown.p4plugin.components.SwarmConnectionComponent</implementation-class>
         </component>
 
@@ -254,7 +247,6 @@
 
         <!-- Manages password issues -->
         <component>
-            <interface-class>net.groboclown.p4plugin.components.InvalidPasswordMonitorComponent</interface-class>
             <implementation-class>net.groboclown.p4plugin.components.InvalidPasswordMonitorComponent</implementation-class>
         </component>
     </application-components>