Sorted autocomplete

gradle/changelog/autocomplete_sorted.yaml

@@ -0,0 +1,2 @@
+- type: fixed
+  description: Autocompletion has sorted suggestions ([#1918](https://github.com/scm-manager/scm-manager/pull/1918))

scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupDatabase.java

@@ -21,25 +21,20 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
-package sonia.scm.group.xml;
 
-//~--- non-JDK imports --------------------------------------------------------
+package sonia.scm.group.xml;
 
 import sonia.scm.group.Group;
 import sonia.scm.xml.XmlDatabase;
 
-//~--- JDK imports ------------------------------------------------------------
-
-import java.util.Collection;
-import java.util.LinkedHashMap;
-import java.util.Map;
-
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
+import java.util.Collection;
+import java.util.Map;
+import java.util.TreeMap;
 
 /**
  *
@@ -190,7 +185,7 @@ public void setLastModified(long lastModified)
   /** Field description */
   @XmlJavaTypeAdapter(XmlGroupMapAdapter.class)
   @XmlElement(name = "groups")
-  private Map<String, Group> groupMap = new LinkedHashMap<>();
+  private Map<String, Group> groupMap = new TreeMap<>();
 
   /** Field description */
   private Long lastModified;

scm-dao-xml/src/main/java/sonia/scm/group/xml/XmlGroupMapAdapter.java

@@ -21,19 +21,14 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
-package sonia.scm.group.xml;
 
-//~--- non-JDK imports --------------------------------------------------------
+package sonia.scm.group.xml;
 
 import sonia.scm.group.Group;
 
-//~--- JDK imports ------------------------------------------------------------
-
-import java.util.LinkedHashMap;
-import java.util.Map;
-
 import javax.xml.bind.annotation.adapters.XmlAdapter;
+import java.util.Map;
+import java.util.TreeMap;
 
 /**
  *
@@ -72,7 +67,7 @@ public XmlGroupList marshal(Map<String, Group> groupMap) throws Exception
   @Override
   public Map<String, Group> unmarshal(XmlGroupList groups) throws Exception
   {
-    Map<String, Group> groupMap = new LinkedHashMap<>();
+    Map<String, Group> groupMap = new TreeMap<>();
 
     for (Group group : groups)
     {

scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryDAO.java

@@ -26,7 +26,6 @@
 
 //~--- non-JDK imports --------------------------------------------------------
 
-import com.google.common.collect.ImmutableList;
 import com.google.inject.Singleton;
 import sonia.scm.io.FileSystem;
 import sonia.scm.repository.InternalRepositoryException;
@@ -42,23 +41,33 @@
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.Map;
+import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Supplier;
+
+import static java.util.Collections.unmodifiableList;
+import static java.util.stream.Collectors.toList;
 
 /**
  * @author Sebastian Sdorra
  */
 @Singleton
 public class XmlRepositoryDAO implements RepositoryDAO {
 
+  private static final RepositoryContainer EMPTY_REPOSITORY_CONTAINER = new RepositoryContainer(null);
 
   private final MetadataStore metadataStore = new MetadataStore();
 
   private final PathBasedRepositoryLocationResolver repositoryLocationResolver;
   private final FileSystem fileSystem;
   private final RepositoryExportingCheck repositoryExportingCheck;
 
-  private final Map<String, Repository> byId;
-  private final Map<NamespaceAndName, Repository> byNamespaceAndName;
+  private final Map<String, RepositoryContainer> byId;
+  private final Map<NamespaceAndName, RepositoryContainer> byNamespaceAndName;
+  private final ReadWriteLock byNamespaceLock = new ReentrantReadWriteLock();
 
   @Inject
   public XmlRepositoryDAO(PathBasedRepositoryLocationResolver repositoryLocationResolver, FileSystem fileSystem, RepositoryExportingCheck repositoryExportingCheck) {
@@ -67,17 +76,20 @@ public XmlRepositoryDAO(PathBasedRepositoryLocationResolver repositoryLocationRe
     this.repositoryExportingCheck = repositoryExportingCheck;
 
     this.byId = new ConcurrentHashMap<>();
-    this.byNamespaceAndName = new ConcurrentHashMap<>();
+    this.byNamespaceAndName = new TreeMap<>();
 
     init();
   }
 
   private void init() {
-    RepositoryLocationResolver.RepositoryLocationResolverInstance<Path> pathRepositoryLocationResolverInstance = repositoryLocationResolver.create(Path.class);
-    pathRepositoryLocationResolverInstance.forAllLocations((repositoryId, repositoryPath) -> {
-      Repository repository = metadataStore.read(repositoryPath);
-      byNamespaceAndName.put(repository.getNamespaceAndName(), repository);
-      byId.put(repositoryId, repository);
+    withWriteLockedByNamespaceMap(() -> {
+      RepositoryLocationResolver.RepositoryLocationResolverInstance<Path> pathRepositoryLocationResolverInstance = repositoryLocationResolver.create(Path.class);
+      pathRepositoryLocationResolverInstance.forAllLocations((repositoryId, repositoryPath) -> {
+        Repository repository = metadataStore.read(repositoryPath);
+        RepositoryContainer container = new RepositoryContainer(repository);
+        byNamespaceAndName.put(repository.getNamespaceAndName(), container);
+        byId.put(repositoryId, container);
+      });
     });
   }
 
@@ -106,8 +118,11 @@ public synchronized void add(Repository repository, Object location) {
       throw new InternalRepositoryException(repository, "failed to create filesystem", e);
     }
 
-    byId.put(repository.getId(), clone);
-    byNamespaceAndName.put(repository.getNamespaceAndName(), clone);
+    withWriteLockedByNamespaceMap(() -> {
+      RepositoryContainer container = new RepositoryContainer(clone);
+      byId.put(repository.getId(), container);
+      byNamespaceAndName.put(repository.getNamespaceAndName(), container);
+    });
   }
 
   @Override
@@ -117,7 +132,7 @@ public boolean contains(Repository repository) {
 
   @Override
   public boolean contains(NamespaceAndName namespaceAndName) {
-    return byNamespaceAndName.containsKey(namespaceAndName);
+    return withReadLockedByNamespaceMap(() -> byNamespaceAndName.containsKey(namespaceAndName));
   }
 
   @Override
@@ -127,17 +142,23 @@ public boolean contains(String id) {
 
   @Override
   public Repository get(NamespaceAndName namespaceAndName) {
-    return byNamespaceAndName.get(namespaceAndName);
+    return withReadLockedByNamespaceMap(() -> byNamespaceAndName.getOrDefault(namespaceAndName, EMPTY_REPOSITORY_CONTAINER).getRepository());
   }
 
   @Override
   public Repository get(String id) {
-    return byId.get(id);
+    return byId.getOrDefault(id, EMPTY_REPOSITORY_CONTAINER).getRepository();
   }
 
   @Override
   public Collection<Repository> getAll() {
-    return ImmutableList.copyOf(byNamespaceAndName.values());
+    return withReadLockedByNamespaceMap(() ->
+      unmodifiableList(
+        byNamespaceAndName
+          .values()
+          .stream()
+          .map(RepositoryContainer::getRepository)
+          .collect(toList())));
   }
 
   @Override
@@ -149,11 +170,20 @@ public void modify(Repository repository) {
 
     synchronized (this) {
       // remove old namespaceAndName from map, in case of rename
-      Repository prev = byId.put(clone.getId(), clone);
-      if (prev != null) {
-        byNamespaceAndName.remove(prev.getNamespaceAndName());
+      RepositoryContainer container = byId.get(clone.getId());
+      if (container != null) {
+        if (container.getRepository().getNamespaceAndName() != clone.getNamespaceAndName()) {
+          withWriteLockedByNamespaceMap(() -> {
+            byNamespaceAndName.remove(container.getRepository().getNamespaceAndName());
+            byNamespaceAndName.put(clone.getNamespaceAndName(), container);
+          });
+        }
+        container.setRepository(clone);
+      } else {
+        RepositoryContainer newContainer = new RepositoryContainer(clone);
+        withWriteLockedByNamespaceMap(() -> byNamespaceAndName.put(clone.getNamespaceAndName(), newContainer));
+        byId.put(clone.getId(), newContainer);
       }
-      byNamespaceAndName.put(clone.getNamespaceAndName(), clone);
     }
 
     Path repositoryPath = repositoryLocationResolver
@@ -164,7 +194,7 @@ public void modify(Repository repository) {
   }
 
   private boolean mustNotModifyRepository(Repository clone) {
-    return clone.isArchived() && byId.get(clone.getId()).isArchived()
+    return clone.isArchived() && byId.get(clone.getId()) != null && byId.get(clone.getId()).getRepository().isArchived()
       || repositoryExportingCheck.isExporting(clone);
   }
 
@@ -175,9 +205,9 @@ public void delete(Repository repository) {
     }
     Path path;
     synchronized (this) {
-      Repository prev = byId.remove(repository.getId());
+      RepositoryContainer prev = byId.remove(repository.getId());
       if (prev != null) {
-        byNamespaceAndName.remove(prev.getNamespaceAndName());
+        withWriteLockedByNamespaceMap(() -> byNamespaceAndName.remove(prev.getRepository().getNamespaceAndName()));
       }
       path = repositoryLocationResolver.remove(repository.getId());
     }
@@ -201,8 +231,44 @@ public Long getLastModified() {
 
   public void refresh() {
     repositoryLocationResolver.refresh();
-    byNamespaceAndName.clear();
+    withWriteLockedByNamespaceMap(byNamespaceAndName::clear);
     byId.clear();
     init();
   }
+
+  private void withWriteLockedByNamespaceMap(Runnable runnable) {
+    Lock lock = byNamespaceLock.writeLock();
+    lock.lock();
+    try {
+      runnable.run();
+    } finally {
+      lock.unlock();
+    }
+  }
+
+  private <T> T withReadLockedByNamespaceMap(Supplier<T> runnable) {
+    Lock lock = byNamespaceLock.readLock();
+    lock.lock();
+    try {
+      return runnable.get();
+    } finally {
+      lock.unlock();
+    }
+  }
+
+  private static class RepositoryContainer {
+    private Repository repository;
+
+    public RepositoryContainer(Repository repository) {
+      this.repository = repository;
+    }
+
+    public Repository getRepository() {
+      return repository;
+    }
+
+    public void setRepository(Repository repository) {
+      this.repository = repository;
+    }
+  }
 }

scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryRoleDatabase.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.repository.xml;
 
 import sonia.scm.repository.RepositoryRole;
@@ -33,8 +33,8 @@
 import javax.xml.bind.annotation.XmlRootElement;
 import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
 import java.util.Collection;
-import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.TreeMap;
 
 @XmlRootElement(name = "user-db")
 @XmlAccessorType(XmlAccessType.FIELD)
@@ -45,7 +45,7 @@ public class XmlRepositoryRoleDatabase implements XmlDatabase<RepositoryRole> {
 
   @XmlJavaTypeAdapter(XmlRepositoryRoleMapAdapter.class)
   @XmlElement(name = "roles")
-  private Map<String, RepositoryRole> roleMap = new LinkedHashMap<>();
+  private Map<String, RepositoryRole> roleMap = new TreeMap<>();
 
   public XmlRepositoryRoleDatabase() {
     long c = System.currentTimeMillis();

scm-dao-xml/src/main/java/sonia/scm/repository/xml/XmlRepositoryRoleMapAdapter.java

@@ -21,14 +21,14 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.repository.xml;
 
 import sonia.scm.repository.RepositoryRole;
 
 import javax.xml.bind.annotation.adapters.XmlAdapter;
-import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.TreeMap;
 
 public class XmlRepositoryRoleMapAdapter
         extends XmlAdapter<XmlRepositoryRoleList, Map<String, RepositoryRole>> {
@@ -40,7 +40,7 @@ public XmlRepositoryRoleList marshal(Map<String, RepositoryRole> roleMap) {
 
   @Override
   public Map<String, RepositoryRole> unmarshal(XmlRepositoryRoleList roles) {
-    Map<String, RepositoryRole> roleMap = new LinkedHashMap<>();
+    Map<String, RepositoryRole> roleMap = new TreeMap<>();
 
     for (RepositoryRole role : roles) {
       roleMap.put(role.getName(), role);

scm-dao-xml/src/main/java/sonia/scm/user/xml/XmlUserDatabase.java

@@ -21,25 +21,20 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-
-package sonia.scm.user.xml;
 
-//~--- non-JDK imports --------------------------------------------------------
+package sonia.scm.user.xml;
 
 import sonia.scm.user.User;
 import sonia.scm.xml.XmlDatabase;
 
-//~--- JDK imports ------------------------------------------------------------
-
-import java.util.Collection;
-import java.util.LinkedHashMap;
-import java.util.Map;
-
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
+import java.util.Collection;
+import java.util.Map;
+import java.util.TreeMap;
 
 /**
  *
@@ -193,5 +188,5 @@ public void setLastModified(long lastModified)
   /** Field description */
   @XmlJavaTypeAdapter(XmlUserMapAdapter.class)
   @XmlElement(name = "users")
-  private Map<String, User> userMap = new LinkedHashMap<>();
+  private Map<String, User> userMap = new TreeMap<>();
 }