Improve ApplicationKeys and SiteConfigs collections #10029 (#10030)

modules/core/core-api/src/main/java/com/enonic/xp/app/ApplicationKeys.java

@@ -3,7 +3,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 
 import com.enonic.xp.annotation.PublicApi;
 import com.enonic.xp.support.AbstractImmutableEntityList;
@@ -12,38 +12,48 @@
 public final class ApplicationKeys
     extends AbstractImmutableEntityList<ApplicationKey>
 {
-    private ApplicationKeys( final ImmutableList<ApplicationKey> list )
+    private static final ApplicationKeys EMPTY = new ApplicationKeys( ImmutableSet.of() );
+
+    private ApplicationKeys( final ImmutableSet<ApplicationKey> list )
     {
-        super( list );
+        // ApplicationKeys is supposed to be set, but it was made as list initially. We deduplicate values and store them in list.
+        super( list.asList() );
     }
 
     public static ApplicationKeys from( final ApplicationKey... applicationKeys )
     {
-        return new ApplicationKeys( ImmutableList.copyOf( applicationKeys ) );
+        return fromInternal( ImmutableSet.copyOf( applicationKeys ) );
     }
 
     public static ApplicationKeys from( final Iterable<? extends ApplicationKey> applicationKeys )
     {
-        return new ApplicationKeys( ImmutableList.copyOf( applicationKeys ) );
+        return fromInternal( ImmutableSet.copyOf( applicationKeys ) );
     }
 
     public static ApplicationKeys from( final Collection<? extends ApplicationKey> applicationKeys )
     {
-        return new ApplicationKeys( ImmutableList.copyOf( applicationKeys ) );
+        return fromInternal( ImmutableSet.copyOf( applicationKeys ) );
     }
 
     public static ApplicationKeys from( final String... applicationKeys )
     {
-        return new ApplicationKeys( parseApplicationKeys( applicationKeys ) );
+        return fromInternal( Arrays.stream( applicationKeys ).map( ApplicationKey::from ).collect( ImmutableSet.toImmutableSet() ) );
     }
 
     public static ApplicationKeys empty()
     {
-        return new ApplicationKeys( ImmutableList.of() );
+        return EMPTY;
     }
 
-    private static ImmutableList<ApplicationKey> parseApplicationKeys( final String... applicationKeys )
+    private static ApplicationKeys fromInternal( final ImmutableSet<ApplicationKey> applicationKeys )
     {
-        return Arrays.stream( applicationKeys ).map( ApplicationKey::from ).collect( ImmutableList.toImmutableList() );
+        if ( applicationKeys.isEmpty() )
+        {
+            return EMPTY;
+        }
+        else
+        {
+            return new ApplicationKeys( applicationKeys );
+        }
     }
 }

modules/core/core-api/src/main/java/com/enonic/xp/site/Site.java

@@ -20,6 +20,7 @@ public String getDescription()
         return this.getData().getString( "description" );
     }
 
+    @Deprecated
     public PropertyTree getSiteConfig( final ApplicationKey applicationKey )
     {
         final SiteConfig siteConfig = this.getSiteConfigs().get( applicationKey );

modules/core/core-api/src/main/java/com/enonic/xp/site/SiteConfigs.java

@@ -1,82 +1,92 @@
 package com.enonic.xp.site;
 
 import java.util.Collection;
-import java.util.function.Function;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 
 import com.enonic.xp.annotation.PublicApi;
 import com.enonic.xp.app.ApplicationKey;
 import com.enonic.xp.support.AbstractImmutableEntityList;
 
 @PublicApi
-public class SiteConfigs
+public final class SiteConfigs
     extends AbstractImmutableEntityList<SiteConfig>
 {
-    private final ImmutableMap<ApplicationKey, SiteConfig> applicationsByName;
+    private static final SiteConfigs EMPTY = new SiteConfigs( ImmutableList.of() );
 
     private SiteConfigs( final ImmutableList<SiteConfig> list )
     {
         super( list );
-        this.applicationsByName =
-            list.stream().collect( ImmutableMap.toImmutableMap( SiteConfig::getApplicationKey, Function.identity() ) );
     }
 
     public SiteConfig get( final ApplicationKey applicationKey )
     {
-        return this.applicationsByName.get( applicationKey );
+        return list.stream().filter( sc -> applicationKey.equals( sc.getApplicationKey() ) ).findAny().orElse( null );
     }
 
+    @Deprecated
     public SiteConfig get( final String applicationKey )
     {
         return get( ApplicationKey.from( applicationKey ) );
     }
 
+    @Deprecated
     public ImmutableSet<ApplicationKey> getApplicationKeys()
     {
-        return applicationsByName.keySet();
+        return list.stream().map( SiteConfig::getApplicationKey ).collect( ImmutableSet.toImmutableSet() );
     }
 
     public static SiteConfigs empty()
     {
-        return new SiteConfigs( ImmutableList.of() );
+        return EMPTY;
     }
 
     public static SiteConfigs from( final SiteConfig... siteConfigs )
     {
-        return new SiteConfigs( ImmutableList.copyOf( siteConfigs ) );
+        return fromInternal( ImmutableList.copyOf( siteConfigs ) );
     }
 
     public static SiteConfigs from( final Iterable<? extends SiteConfig> siteConfigs )
     {
-        return new SiteConfigs( ImmutableList.copyOf( siteConfigs ) );
+        return fromInternal( ImmutableList.copyOf( siteConfigs ) );
     }
 
     public static SiteConfigs from( final Collection<? extends SiteConfig> siteConfigs )
     {
-        return new SiteConfigs( ImmutableList.copyOf( siteConfigs ) );
+        return fromInternal( ImmutableList.copyOf( siteConfigs ) );
+    }
+
+    private static SiteConfigs fromInternal( final ImmutableList<SiteConfig> siteConfigs )
+    {
+        if ( siteConfigs.isEmpty() )
+        {
+            return EMPTY;
+        }
+        else
+        {
+            return new SiteConfigs( siteConfigs );
+        }
     }
 
     public static Builder create()
     {
         return new Builder();
     }
 
-    public static class Builder
+    public static final class Builder
     {
         private final ImmutableList.Builder<SiteConfig> builder = ImmutableList.builder();
 
-        public Builder add( SiteConfig siteConfig )
+        public Builder add( final SiteConfig siteConfig )
         {
             builder.add( siteConfig );
             return this;
         }
 
         public SiteConfigs build()
         {
-            return new SiteConfigs( builder.build() );
+            return fromInternal( builder.build() );
         }
     }
 }

modules/core/core-api/src/test/java/com/enonic/xp/app/ApplicationKeysTest.java

@@ -1,16 +1,19 @@
 package com.enonic.xp.app;
 
 import java.util.ArrayList;
+import java.util.Collections;
 
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-public class ApplicationKeysTest
+class ApplicationKeysTest
 {
-    private static final ArrayList<ApplicationKey> list = new ArrayList();
+    private static final ArrayList<ApplicationKey> list = new ArrayList<>();
 
     @BeforeAll
     public static void initApplicationKeys()
@@ -43,7 +46,7 @@ public void fromArray()
     @Test
     public void fromIterable()
     {
-        ApplicationKeys applicationKeys = ApplicationKeys.from( (Iterable) ApplicationKeysTest.list );
+        ApplicationKeys applicationKeys = ApplicationKeys.from( (Iterable<ApplicationKey>) ApplicationKeysTest.list );
 
         assertEquals( 3, applicationKeys.getSize() );
         assertTrue( applicationKeys.contains( ApplicationKeysTest.list.get( 0 ) ) );
@@ -72,4 +75,19 @@ public void fromStringArray()
         assertTrue( applicationKeys.contains( ApplicationKeysTest.list.get( 1 ) ) );
         assertTrue( applicationKeys.contains( ApplicationKeysTest.list.get( 2 ) ) );
     }
+
+    @Test
+    void deduplicate()
+    {
+        ApplicationKeys applicationKeys =
+            ApplicationKeys.from( ApplicationKey.from( "aaa" ), ApplicationKey.from( "aaa" ), ApplicationKey.from( "bbb" ) );
+
+        assertThat( applicationKeys ).containsExactly( ApplicationKey.from( "aaa" ), ApplicationKey.from( "bbb" ) );
+    }
+
+    @Test
+    void empty_from_same()
+    {
+        assertSame( ApplicationKeys.empty(), ApplicationKeys.from( Collections.emptyList() ) );
+    }
 }

modules/core/core-api/src/test/java/com/enonic/xp/site/SiteTest.java

@@ -79,8 +79,8 @@ public void siteConfigs()
         SiteConfigs siteConfigs = site.getSiteConfigs();
         assertNotNull( siteConfigs );
         assertEquals( 1, siteConfigs.getSize() );
-        assertEquals( siteConfigs.get( 0 ).getConfig(), site.getSiteConfig( ApplicationKey.from( "myapplication" ) ) );
-        assertNotNull( siteConfigs.get( "myapplication" ) );
+        assertEquals( siteConfigs.get( 0 ).getConfig(), site.getSiteConfigs().get( ApplicationKey.from( "myapplication" ) ).getConfig() );
+        assertNotNull( siteConfigs.get( ApplicationKey.from( "myapplication" ) ) );
         assertTrue( SiteConfigs.empty().isEmpty() );
         assertEquals( 1, SiteConfigs.from( siteConfig ).getSize() );
     }

modules/core/core-project/src/main/java/com/enonic/xp/core/impl/project/ProjectServiceImpl.java

@@ -8,7 +8,6 @@
 import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.util.ArrayDeque;
-import java.util.Collection;
 import java.util.EnumSet;
 import java.util.Iterator;
 import java.util.List;
@@ -25,10 +24,10 @@
 import org.slf4j.LoggerFactory;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.io.ByteSource;
 
-import com.enonic.xp.app.ApplicationKey;
 import com.enonic.xp.app.ApplicationKeys;
 import com.enonic.xp.attachment.Attachment;
 import com.enonic.xp.attachment.AttachmentSerializer;
@@ -74,6 +73,7 @@
 import com.enonic.xp.security.RoleKeys;
 import com.enonic.xp.security.SecurityService;
 import com.enonic.xp.security.auth.AuthenticationInfo;
+import com.enonic.xp.site.SiteConfig;
 import com.enonic.xp.site.SiteConfigs;
 import com.enonic.xp.site.SiteConfigsDataSerializer;
 import com.enonic.xp.util.BinaryReference;
@@ -313,10 +313,11 @@ public ApplicationKeys getAvailableApplications( final ProjectName projectName )
             throw new ProjectNotFoundException( projectName );
         }
 
-        final Collection<ApplicationKey> projectApps = project.getSiteConfigs().getApplicationKeys();
-
-        return callWithListContext( () -> ApplicationKeys.from( Stream.concat( projectApps.stream(), doGetParents( project ).stream()
-            .flatMap( parent -> parent.getSiteConfigs().getApplicationKeys().stream() ) ).distinct().collect( Collectors.toList() ) ) );
+        return callWithListContext( () -> ApplicationKeys.from( Stream.concat( Stream.of( project ), doGetParents( project ).stream() )
+                                                                    .map( Project::getSiteConfigs )
+                                                                    .flatMap( SiteConfigs::stream )
+                                                                    .map( SiteConfig::getApplicationKey )
+                                                                    .collect( ImmutableSet.toImmutableSet() ) ) );
     }
 
     @Override
@@ -662,7 +663,7 @@ private PropertyTree createContentRootData( final CreateProjectParams params )
 
         final PropertySet contentRootData = data.addSet( "data" );
 
-        if ( !params.getSiteConfigs().getApplicationKeys().isEmpty() )
+        if ( !params.getSiteConfigs().isEmpty() )
         {
             SITE_CONFIGS_DATA_SERIALIZER.toProperties( params.getSiteConfigs(), contentRootData );
         }

modules/lib/lib-content/src/main/java/com/enonic/xp/lib/content/GetSiteConfigHandler.java

@@ -14,6 +14,7 @@
 import com.enonic.xp.security.RoleKeys;
 import com.enonic.xp.security.auth.AuthenticationInfo;
 import com.enonic.xp.site.Site;
+import com.enonic.xp.site.SiteConfig;
 
 public final class GetSiteConfigHandler
     extends BaseContextHandler
@@ -36,7 +37,8 @@ protected Object doExecute()
             siteSupplier = () -> contentService.getNearestSite( ContentId.from( key ) );
         }
         return Optional.ofNullable( callAsContentAdmin( siteSupplier::get ) )
-            .map( site -> site.getSiteConfig( ApplicationKey.from( applicationKey ) ) )
+            .flatMap( site -> Optional.ofNullable( site.getSiteConfigs().get( ApplicationKey.from( applicationKey ) ) ) )
+            .map( SiteConfig::getConfig )
             .map( PropertyTreeMapper::new )
             .orElse( null );
     }

modules/lib/lib-node/src/main/resources/lib/xp/node.ts

@@ -1066,7 +1066,7 @@ class RepoConnectionImpl
             includeChildren = true,
             parent,
             dataProcessor,
-            refresh
+            refresh,
         } = params ?? {};
 
         const handlerParams = __.newBean<DuplicateNodeHandlerParams>('com.enonic.xp.lib.node.DuplicateNodeHandlerParams');

modules/lib/lib-portal/src/main/java/com/enonic/xp/lib/portal/current/GetCurrentSiteConfigHandler.java

@@ -33,7 +33,11 @@ public PropertyTreeMapper execute()
 
             if ( site != null )
             {
-                appConfigPropertyTree = site.getSiteConfig( applicationKey );
+                final SiteConfig config = site.getSiteConfigs().get( applicationKey );
+                if ( config != null )
+                {
+                    appConfigPropertyTree = config.getConfig();
+                }
             }
             else
             {

modules/lib/lib-project/src/test/java/com/enonic/xp/lib/project/BaseProjectHandlerTest.java

@@ -2,6 +2,7 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import org.mockito.Mockito;
 
@@ -29,6 +30,7 @@
 import com.enonic.xp.security.acl.AccessControlEntry;
 import com.enonic.xp.security.acl.AccessControlList;
 import com.enonic.xp.security.acl.Permission;
+import com.enonic.xp.site.SiteConfig;
 import com.enonic.xp.testing.ScriptTestSupport;
 
 import static org.mockito.ArgumentMatchers.any;
@@ -104,7 +106,8 @@ protected void mockProject()
             {
                 throw new ProjectNotFoundException( (ProjectName) mock.getArguments()[0] );
             }
-            return ApplicationKeys.from( project.getSiteConfigs().getApplicationKeys() );
+            return ApplicationKeys.from(
+                project.getSiteConfigs().stream().map( SiteConfig::getApplicationKey ).collect( Collectors.toList() ) );
         } );
 
         when( this.projectService.getPermissions( any( ProjectName.class ) ) ).thenReturn( ProjectPermissions.create().build() );