Shell lookups must return shells ordered in a deterministic way

- Modifies repository and service implementation to fetch AAS Ids ordered by the createdDate (and then the AAS Id as secondary criteria) - Fixes a bug in list all API - Makes fetch size configurable - Adds new configuration property to charts - Adds new tests to cover the changes better
eclipse-tractusx · Feb 26, 2024 · cdff066 · cdff066
1 parent ececdbf
commit cdff066
Show file tree

Hide file tree

Showing 11 changed files with 474 additions and 67 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## 0.4.0
+### Added
+- Granular access control
+
+## fixed
+
 ## 0.3.23
 ### Added
 

diff --git a/backend/src/main/java/org/eclipse/tractusx/semantics/RegistryProperties.java b/backend/src/main/java/org/eclipse/tractusx/semantics/RegistryProperties.java
@@ -53,6 +53,10 @@ public class RegistryProperties {
      * This flag turns on the granular access control logic if set to true.
      */
     private Boolean useGranularAccessControl;
+    /**
+     * Configures the number of records fetched in one batch when a page of shells is requested.
+     */
+    private Integer granularAccessControlFetchSize;
 
     /**
      * Properties for Identity Management system

diff --git a/...in/java/org/eclipse/tractusx/semantics/registry/repository/ShellIdentifierRepository.java b/...in/java/org/eclipse/tractusx/semantics/registry/repository/ShellIdentifierRepository.java
@@ -20,13 +20,15 @@
 
 package org.eclipse.tractusx.semantics.registry.repository;
 
+import java.time.Instant;
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
 
 import org.eclipse.tractusx.semantics.registry.model.Shell;
 import org.eclipse.tractusx.semantics.registry.model.ShellIdentifier;
 import org.eclipse.tractusx.semantics.registry.model.projection.ShellIdentifierMinimal;
+import org.springframework.data.domain.Pageable;
 import org.springframework.data.jpa.repository.JpaRepository;
 import org.springframework.data.jpa.repository.Modifying;
 import org.springframework.data.jpa.repository.Query;
@@ -41,19 +43,36 @@ public interface ShellIdentifierRepository extends JpaRepository<ShellIdentifier
    Set<ShellIdentifier> findByShellId( Shell shellId );
 
    @Query( value = """
-            SELECT NEW org.eclipse.tractusx.semantics.registry.model.projection.ShellIdentifierMinimal(sid.shellId.idExternal, sid.key, sid.value)
+           SELECT s.id
+           FROM ShellIdentifier sid
+              JOIN sid.shellId s
+           WHERE
+              CONCAT( sid.key, sid.value ) IN ( :keyValueCombinations )
+              AND (
+                 s.createdDate > :cutoffDate
+                 OR ( s.createdDate = :cutoffDate AND s.idExternal > :cursorValue )
+              )
+           GROUP BY s.id, s.createdDate, s.idExternal
+           HAVING COUNT(*) = :keyValueCombinationsSize
+           ORDER BY s.createdDate ASC, s.idExternal ASC
+         """ )
+   List<UUID> findAPageOfShellIdsBySpecificAssetIds(
+         List<String> keyValueCombinations, int keyValueCombinationsSize, Instant cutoffDate, String cursorValue, Pageable pageable );
+
+   @Query( value = """
+            SELECT NEW org.eclipse.tractusx.semantics.registry.model.projection.ShellIdentifierMinimal(s.idExternal, sid.key, sid.value)
             FROM ShellIdentifier sid
+               JOIN sid.shellId s
             WHERE
-                 sid.shellId.id IN (
-                    SELECT filtersid.shellId.id
-                    FROM ShellIdentifier filtersid
-                    WHERE
-                        CONCAT(filtersid.key, filtersid.value) IN (:keyValueCombinations)
-                    GROUP BY filtersid.shellId.id
-                    HAVING COUNT(*) = :keyValueCombinationsSize
-                )
+               s.id IN ( :shellIds )
+               AND (
+                  s.createdDate > :cutoffDate
+                  OR ( s.createdDate = :cutoffDate AND s.idExternal > :cursorValue )
+               )
+            ORDER BY s.createdDate ASC, s.idExternal ASC
          """ )
-   List<ShellIdentifierMinimal> findMinimalShellIdsBySpecificAssetIds( List<String> keyValueCombinations, int keyValueCombinationsSize );
+   List<ShellIdentifierMinimal> findMinimalShellIdsByShellIds(
+         List<UUID> shellIds, Instant cutoffDate, String cursorValue );
 
    /**
     * Returns external shell ids for the given keyValueCombinations.
@@ -74,7 +93,11 @@ HAVING COUNT(*) = :keyValueCombinationsSize
          FROM shell s
             JOIN shell_identifier si ON s.id = si.fk_shell_id
          WHERE
-            CONCAT(si.namespace, si.identifier) IN (:keyValueCombinations)
+            CONCAT( si.namespace, si.identifier ) IN ( :keyValueCombinations )
+            AND (
+               s.created_date > :cutoffDate
+               OR ( s.created_date = :cutoffDate AND s.id_external > :cursorValue )
+            )
             AND (
                :tenantId = :owningTenantId
                OR si.namespace = :globalAssetId
@@ -90,14 +113,19 @@ OR EXISTS (
                      AND sies.FK_SHELL_IDENTIFIER_EXTERNAL_SUBJECT_ID = si.id
                )
             )
-         GROUP BY s.id_external
+         GROUP BY s.id_external, s.created_date
          HAVING COUNT(*) = :keyValueCombinationsSize
+         ORDER BY s.created_date, s.id_external
+         LIMIT :pageSize
          """, nativeQuery = true )
    List<String> findExternalShellIdsByIdentifiersByExactMatch( @Param( "keyValueCombinations" ) List<String> keyValueCombinations,
          @Param( "keyValueCombinationsSize" ) int keyValueCombinationsSize,
          @Param( "tenantId" ) String tenantId,
          @Param( "publicWildcardPrefix" ) String publicWildcardPrefix,
          @Param( "publicWildcardAllowedTypes" ) List<String> publicWildcardAllowedTypes,
          @Param( "owningTenantId" ) String owningTenantId,
-         @Param( "globalAssetId" ) String globalAssetId );
+         @Param( "globalAssetId" ) String globalAssetId,
+         @Param( "cutoffDate" ) Instant cutoffDate,
+         @Param( "cursorValue" ) String cursorValue,
+         @Param( "pageSize" ) int pageSize);
 }
diff --git a/...end/src/main/java/org/eclipse/tractusx/semantics/registry/repository/ShellRepository.java b/...end/src/main/java/org/eclipse/tractusx/semantics/registry/repository/ShellRepository.java
@@ -20,6 +20,7 @@
 
 package org.eclipse.tractusx.semantics.registry.repository;
 
+import java.time.Instant;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -142,4 +143,7 @@ s.id IN (
             )
          """)
    List<Shell> findAllBySubmodelEndpointAddress( String endpointAddress );
+
+   @Query("SELECT s.createdDate FROM Shell s WHERE s.idExternal = :idExternal")
+   Optional<Instant> getCreatedDateByIdExternal( String idExternal );
 }
diff --git a/backend/src/main/java/org/eclipse/tractusx/semantics/registry/service/ShellService.java b/backend/src/main/java/org/eclipse/tractusx/semantics/registry/service/ShellService.java
@@ -22,6 +22,9 @@
 
 import static org.springframework.data.domain.PageRequest.ofSize;
 
+import java.time.Instant;
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
 import java.util.ArrayList;
 import java.util.Base64;
 import java.util.Collection;
@@ -56,6 +59,7 @@
 import org.eclipse.tractusx.semantics.registry.utils.ShellSpecification;
 import org.springframework.dao.DuplicateKeyException;
 import org.springframework.data.domain.Page;
+import org.springframework.data.domain.PageRequest;
 import org.springframework.data.jpa.domain.Specification;
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Service;
@@ -73,7 +77,11 @@ public class ShellService {
    public static final String DUPLICATE_SUBMODEL_ID_SHORT_EXCEPTION = "An AssetAdministration Submodel for the given IdShort does already exists.";
    private static final String SORT_FIELD_NAME_SHELL = "createdDate";
    private static final String SORT_FIELD_NAME_SUBMODEL = "id";
+   private static final String DEFAULT_EXTERNAL_ID = "00000000-0000-0000-0000-000000000000";
+   private static final Instant MINIMUM_SQL_DATETIME = OffsetDateTime
+         .of( 1800, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC ).toInstant();
    private static final int MAXIMUM_RECORDS = 1000;
+   private static final int DEFAULT_FETCH_SIZE = 500;
 
    private final ShellRepository shellRepository;
    private final ShellIdentifierRepository shellIdentifierRepository;
@@ -82,6 +90,7 @@ public class ShellService {
    private final String owningTenantId;
    private final String externalSubjectIdWildcardPrefix;
    private final List<String> externalSubjectIdWildcardAllowedTypes;
+   private final int granularAccessControlFetchSize;
 
    public ShellService( ShellRepository shellRepository,
          ShellIdentifierRepository shellIdentifierRepository,
@@ -95,6 +104,7 @@ public ShellService( ShellRepository shellRepository,
       this.owningTenantId = registryProperties.getIdm().getOwningTenantId();
       this.externalSubjectIdWildcardPrefix = registryProperties.getExternalSubjectIdWildcardPrefix();
       this.externalSubjectIdWildcardAllowedTypes = registryProperties.getExternalSubjectIdWildcardAllowedTypes();
+      this.granularAccessControlFetchSize = Optional.ofNullable( registryProperties.getGranularAccessControlFetchSize() ).orElse( DEFAULT_FETCH_SIZE );
    }
 
    @Transactional
@@ -199,16 +209,19 @@ public ShellCollectionDto findAllShells( Integer pageSize, String cursorVal, Str
       ShellCursor cursor = new ShellCursor( pageSize, cursorVal );
       var specification = shellAccessHandler.shellFilterSpecification( SORT_FIELD_NAME_SHELL, cursor, externalSubjectId );
       final var foundList = new ArrayList<Shell>();
-      boolean hasNext = true;
       //fetch 1 more item to make sure there is a visible item for the next page
-      int fetchSize = pageSize + 1;
-      while ( foundList.size() < fetchSize && hasNext ) {
-         Page<Shell> currentPage = shellRepository.findAll( specification, ofSize( fetchSize ) );
+      while ( foundList.size() < pageSize + 1 ) {
+         Page<Shell> currentPage = shellRepository.findAll( specification, ofSize( granularAccessControlFetchSize ) );
          List<Shell> shells = shellAccessHandler.filterListOfShellProperties( currentPage.stream().toList(), externalSubjectId );
          shells.stream()
-               .limit( (long) fetchSize - foundList.size() )
+               .limit( (long) pageSize + 1 - foundList.size() )
                .forEach( foundList::add );
-         hasNext = currentPage.hasNext();
+         if ( !currentPage.hasNext() ) {
+            break;
+         }
+         ShellCursor shellCursor = new ShellCursor( pageSize,
+               cursor.getEncodedCursorShell( lastItemOf( currentPage.getContent() ).getCreatedDate(), currentPage.hasNext() ) );
+         specification = shellAccessHandler.shellFilterSpecification( SORT_FIELD_NAME_SHELL, shellCursor, externalSubjectId );
       }
       String nextCursor = null;
 
@@ -261,35 +274,17 @@ public GetAllAssetAdministrationShellIdsByAssetLink200Response findExternalShell
          Integer pageSize, String cursor, String externalSubjectId ) {
 
       pageSize = getPageSize( pageSize );
+      final String cursorValue = getCursorDecoded( cursor ).orElse( DEFAULT_EXTERNAL_ID );
       try {
-         List<String> keyValueCombinations = shellIdentifiers.stream().map( shellIdentifier -> shellIdentifier.getKey() + shellIdentifier.getValue() ).toList();
-
-         //TODO: if we can define a sorting order, we should consider doing it on the database side
-         final List<String> assetIdList = new ArrayList<>();
-         String nextCursor;
+         final List<String> visibleAssetIds;
          if ( shellAccessHandler.supportsGranularAccessControl() ) {
-            List<ShellIdentifierMinimal> queryResults = shellIdentifierRepository
-                  .findMinimalShellIdsBySpecificAssetIds( keyValueCombinations, keyValueCombinations.size() );
-            Set<SpecificAssetId> userQuery = shellIdentifiers.stream()
-                  .map( id -> new SpecificAssetId( id.getKey(), id.getValue() ) )
-                  .collect( Collectors.toSet() );
-            List<String> allVisible = shellAccessHandler.filterToVisibleShellIdsForLookup( userQuery, queryResults, externalSubjectId );
-            allVisible.stream()
-                  .skip( getCursorDecoded( cursor, allVisible ) )
-                  .limit( pageSize )
-                  .forEach( assetIdList::add );
-            nextCursor = getCursorEncoded( allVisible, assetIdList );
+            visibleAssetIds = fetchAPageOfAasIdsUsingGranularAccessControl( shellIdentifiers, externalSubjectId, cursorValue, pageSize );
          } else {
-            List<String> queryResult = shellIdentifierRepository.findExternalShellIdsByIdentifiersByExactMatch( keyValueCombinations,
-                  keyValueCombinations.size(), externalSubjectId, externalSubjectIdWildcardPrefix, externalSubjectIdWildcardAllowedTypes, owningTenantId,
-                  ShellIdentifier.GLOBAL_ASSET_ID_KEY );
-            pageSize = getPageSize( pageSize );
-
-            int startIndex = getCursorDecoded( cursor, queryResult );
-            queryResult.stream().skip( startIndex ).limit( pageSize ).forEach( assetIdList::add );
-
-            nextCursor = getCursorEncoded( queryResult, assetIdList );
+            visibleAssetIds = fetchAPageOfAasIdsUsingLegacyAccessControl( shellIdentifiers, externalSubjectId, cursorValue, pageSize );
          }
+
+         final var assetIdList = visibleAssetIds.stream().limit( pageSize ).toList();
+         final String nextCursor = getCursorEncoded( visibleAssetIds, assetIdList );
          final var response = new GetAllAssetAdministrationShellIdsByAssetLink200Response();
          response.setResult( assetIdList );
          response.setPagingMetadata( new PagedResultPagingMetadata().cursor( nextCursor ) );
@@ -301,32 +296,50 @@ public GetAllAssetAdministrationShellIdsByAssetLink200Response findExternalShell
       }
    }
 
-   private String getCursorEncoded( List<String> queryResult, List<String> assetIdList ) {
-      if ( !queryResult.isEmpty() ) {
-         if ( !assetIdList.get( assetIdList.size() - 1 ).equals( queryResult.get( queryResult.size() - 1 ) ) ) {
-            String lastEle = assetIdList.get( assetIdList.size() - 1 );
-            return Base64.getEncoder().encodeToString( lastEle.getBytes() );
+   private List<String> fetchAPageOfAasIdsUsingLegacyAccessControl(
+         Set<ShellIdentifier> shellIdentifiers, String externalSubjectId, String cursorValue, int pageSize ) {
+      final var fetchSize = pageSize + 1;
+      final Instant cutoffDate = shellRepository.getCreatedDateByIdExternal( cursorValue )
+            .orElse( MINIMUM_SQL_DATETIME );
+      List<String> keyValueCombinations = toKeyValueCombinations( shellIdentifiers );
+      return shellIdentifierRepository.findExternalShellIdsByIdentifiersByExactMatch( keyValueCombinations,
+            keyValueCombinations.size(), externalSubjectId, externalSubjectIdWildcardPrefix, externalSubjectIdWildcardAllowedTypes, owningTenantId,
+            ShellIdentifier.GLOBAL_ASSET_ID_KEY, cutoffDate, cursorValue, fetchSize );
+   }
+
+   private List<String> fetchAPageOfAasIdsUsingGranularAccessControl(
+         Set<ShellIdentifier> shellIdentifiers, String externalSubjectId, String cursorValue, int pageSize )
+         throws DenyAccessException {
+      Set<SpecificAssetId> userQuery = shellIdentifiers.stream()
+            .map( id -> new SpecificAssetId( id.getKey(), id.getValue() ) )
+            .collect( Collectors.toSet() );
+      List<String> keyValueCombinations = toKeyValueCombinations( shellIdentifiers );
+      final var fetchSize = granularAccessControlFetchSize;
+
+      String currentCursorValue = cursorValue;
+      final List<String> visibleAssetIds = new ArrayList<>();
+      while ( visibleAssetIds.size() < pageSize + 1 ) {
+         final Instant currentCutoffDate = shellRepository.getCreatedDateByIdExternal( currentCursorValue )
+               .orElse( MINIMUM_SQL_DATETIME );
+         List<UUID> shellIds = shellIdentifierRepository.findAPageOfShellIdsBySpecificAssetIds(
+               keyValueCombinations, keyValueCombinations.size(), currentCutoffDate, currentCursorValue, PageRequest.ofSize( fetchSize ) );
+         if ( shellIds.isEmpty() ) {
+            break;
          }
-      }
-      return null;
-   }
+         List<ShellIdentifierMinimal> queryResults = shellIdentifierRepository
+               .findMinimalShellIdsByShellIds( shellIds, currentCutoffDate, currentCursorValue );
 
-   private String getCursorDecoded( String cursor ) {
-      return Optional.ofNullable( cursor )
-            .map( Base64.getDecoder()::decode )
-            .map( String::new )
-            .orElse( null );
-   }
-
-   private int getCursorDecoded( String cursor, List<String> queryResult ) {
-      return Optional.ofNullable( getCursorDecoded( cursor ) )
-            .map( decodedValue -> queryResult.indexOf( decodedValue ) + 1 )
-            .orElse( 0 );
+         shellAccessHandler.filterToVisibleShellIdsForLookup( userQuery, queryResults, externalSubjectId ).stream()
+               .limit( (long) fetchSize - visibleAssetIds.size() )
+               .forEach( visibleAssetIds::add );
+         currentCursorValue = lastItemOf( queryResults ).shellId();
+      }
+      return visibleAssetIds;
    }
 
    @Transactional( readOnly = true )
    public List<String> findExternalShellIdsByIdentifiersByAnyMatch( Set<ShellIdentifier> shellIdentifiers, String externalSubjectId ) {
-      List<String> keyValueCombinations = shellIdentifiers.stream().map( shellIdentifier -> shellIdentifier.getKey() + shellIdentifier.getValue() ).toList();
+      List<String> keyValueCombinations = toKeyValueCombinations( shellIdentifiers );
 
       return shellRepository.findExternalShellIdsByIdentifiersByAnyMatch(
             keyValueCombinations,
@@ -386,7 +399,7 @@ public Set<ShellIdentifier> save( String externalShellId, Set<ShellIdentifier> s
       return ImmutableSet.copyOf( shellIdentifierRepository.saveAll( identifiersToUpdate ) );
    }
 
-   private static void mapShellIdentifier( Stream<ShellIdentifier> identifiersToUpdate ) {
+   private void mapShellIdentifier( Stream<ShellIdentifier> identifiersToUpdate ) {
       identifiersToUpdate.filter( identifiers -> !identifiers.getKey().equalsIgnoreCase( "globalAssetId" ) ).forEach(
             identifier -> {
                if ( identifier.getSemanticId() != null ) {
@@ -494,7 +507,7 @@ public List<BatchResultDto> saveBatch( List<Shell> shells ) {
       } ).collect( Collectors.toList() );
    }
 
-   @Transactional(readOnly = true)
+   @Transactional( readOnly = true )
    public boolean hasAccessToShellWithVisibleSubmodelEndpoint( String endpointAddress, String externalSubjectId ) {
       List<Shell> shells = shellRepository.findAllBySubmodelEndpointAddress( endpointAddress );
       List<Shell> filtered = shellAccessHandler.filterListOfShellProperties( shells, externalSubjectId );
@@ -509,4 +522,27 @@ private Shell doFindShellByExternalIdWithoutFiltering( String externalShellId )
       return shellRepository.findByIdExternal( externalShellId )
             .orElseThrow( () -> new EntityNotFoundException( String.format( "Shell for identifier %s not found", externalShellId ) ) );
    }
+
+   private <T> T lastItemOf( List<T> list ) {
+      return list.get( list.size() - 1 );
+   }
+
+   private List<String> toKeyValueCombinations( Set<ShellIdentifier> shellIdentifiers ) {
+      return shellIdentifiers.stream()
+            .map( shellIdentifier -> shellIdentifier.getKey() + shellIdentifier.getValue() )
+            .toList();
+   }
+
+   private String getCursorEncoded( List<String> queryResult, List<String> assetIdList ) {
+      if ( !queryResult.isEmpty() && !lastItemOf( assetIdList ).equals( lastItemOf( queryResult ) ) ) {
+         return Base64.getEncoder().encodeToString( lastItemOf( assetIdList ).getBytes() );
+      }
+      return null;
+   }
+
+   private Optional<String> getCursorDecoded( String cursor ) {
+      return Optional.ofNullable( cursor )
+            .map( Base64.getDecoder()::decode )
+            .map( String::new );
+   }
 }
diff --git a/backend/src/main/resources/application.yml b/backend/src/main/resources/application.yml
@@ -66,6 +66,7 @@ registry:
   idm:
     public-client-id: catenax-portal
   use-granular-access-control: false
+  granular-access-control-fetch-size: 500
 
 springdoc:
   cache: