Merge pull request #64 from indiana-university/repo_detection_strateg…

…y_changes LMSA-9645 - Making sure that null package config is handled.
indiana-university · Feb 10, 2025 · 3f8c8c6 · 3f8c8c6
2 parents a99b417 + d28d952
commit 3f8c8c6
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 16 deletions.
diff --git a/...guration/src/main/java/edu/iu/uits/lms/common/swagger/LmsRepositoryDetectionStrategy.java b/...guration/src/main/java/edu/iu/uits/lms/common/swagger/LmsRepositoryDetectionStrategy.java
@@ -37,32 +37,35 @@
 import lombok.Data;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.data.repository.core.RepositoryMetadata;
-import org.springframework.data.rest.core.annotation.RepositoryRestResource;
-import org.springframework.data.rest.core.annotation.RestResource;
 import org.springframework.data.rest.core.mapping.RepositoryDetectionStrategy;
 
 import java.util.List;
 
+/**
+ * Create a RepositoryDetectionStrategy that has to match the configured package list, in addition to either a
+ * class annotated with RepositoryRestResource or RestResource
+ */
 @Data
 @AllArgsConstructor
 @Slf4j
 public class LmsRepositoryDetectionStrategy implements RepositoryDetectionStrategy {
 
-    private List<String> packagesToInclude;
+    private List<String> initPackagesToInclude;
 
     @Override
     public boolean isExported(RepositoryMetadata metadata) {
+        // If no packages were configured to include, use an empty list
+        List<String> packagesToInclude = initPackagesToInclude == null ? List.of() : initPackagesToInclude;
+
         // Check if the repository's package is in the allowed list
         String repositoryPackage = metadata.getRepositoryInterface().getPackageName();
-        boolean isPackageAllowed = packagesToInclude.stream()
-                .anyMatch(repositoryPackage::startsWith);
+        boolean isPackageAllowed = packagesToInclude.stream().anyMatch(repositoryPackage::startsWith);
 
-        // Check for specific annotations
-        boolean hasRepositoryRestResource = metadata.getRepositoryInterface().isAnnotationPresent(RepositoryRestResource.class);
-        boolean hasRestResource = metadata.getRepositoryInterface().isAnnotationPresent(RestResource.class);
+        // Check for allowed annotations
+        boolean hasAnnotation = RepositoryDetectionStrategies.ANNOTATED.isExported(metadata);
 
-        log.debug("Packages to check: {}", packagesToInclude);
-        log.debug("Checking {}: pkg: {}, repoRest: {}, rest: {}", repositoryPackage, isPackageAllowed, hasRepositoryRestResource, hasRestResource);
-        return isPackageAllowed && (hasRepositoryRestResource || hasRestResource);
+        log.debug("Packages to check for class {}: {}", metadata.getRepositoryInterface(), packagesToInclude);
+        log.debug("Checking {}: pkg: {}, anyAnnotation: {}", repositoryPackage, isPackageAllowed, hasAnnotation);
+        return isPackageAllowed && hasAnnotation;
     }
 }
diff --git a/...uration/src/test/java/edu/iu/uits/lms/common/swagger/RepositoryDetectionStrategyTest.java b/...uration/src/test/java/edu/iu/uits/lms/common/swagger/RepositoryDetectionStrategyTest.java
@@ -59,20 +59,36 @@ public void setUp() {
 
     @Test
     public void testGoodPackageHasRepositoryRestResource() {
-        doReturn(TestRepositoryRestResource.class).when(metadata).getRepositoryInterface();
+        doReturn(TestRepositoryRestResourceExported.class).when(metadata).getRepositoryInterface();
 
         boolean result = detectionStrategy.isExported(metadata);
         assertTrue(result, "Repository should be exported when package is allowed and has @RepositoryRestResource");
     }
 
+    @Test
+    public void testGoodPackageHasRepositoryRestResourceButNotExported() {
+        doReturn(TestRepositoryRestResourceNotExported.class).when(metadata).getRepositoryInterface();
+
+        boolean result = detectionStrategy.isExported(metadata);
+        assertFalse(result, "Repository should not be exported when package is allowed and has @RepositoryRestResource, but exported = false");
+    }
+
     @Test
     public void testGoodPackageHasRestResource() {
-        doReturn(TestRestRepository.class).when(metadata).getRepositoryInterface();
+        doReturn(TestRestRepositoryExported.class).when(metadata).getRepositoryInterface();
 
         boolean result = detectionStrategy.isExported(metadata);
         assertTrue(result, "Repository should be exported when package is allowed and has @RestResource");
     }
 
+    @Test
+    public void testGoodPackageHasRestResourceButNotExported() {
+        doReturn(TestRestRepositoryNotExported.class).when(metadata).getRepositoryInterface();
+
+        boolean result = detectionStrategy.isExported(metadata);
+        assertFalse(result, "Repository should not be exported when package is allowed and has @RestResource, but exported = false");
+    }
+
     @Test
     public void testGoodPackageHasBothAnnotations() {
         doReturn(TestBothAnnotationsRepository.class).when(metadata).getRepositoryInterface();
@@ -94,7 +110,7 @@ public void testBadPackage() {
         //Override the strategy to use a different package
         detectionStrategy = new LmsRepositoryDetectionStrategy(List.of("com.other.repository"));
 
-        doReturn(TestRestRepository.class).when(metadata).getRepositoryInterface();
+        doReturn(TestRestRepositoryExported.class).when(metadata).getRepositoryInterface();
         boolean result = detectionStrategy.isExported(metadata);
         assertFalse(result, "Repository should not be exported when package is not allowed");
     }
@@ -107,12 +123,38 @@ public void testGoodPackageNoAnnotations() {
         assertFalse(result, "Repository should not be exported when no annotations are present");
     }
 
+    @Test
+    public void testNoConfiguredPackage() {
+        //Override the strategy to use a different package
+        detectionStrategy = new LmsRepositoryDetectionStrategy(List.of());
+
+        doReturn(TestRestRepositoryExported.class).when(metadata).getRepositoryInterface();
+        boolean result = detectionStrategy.isExported(metadata);
+        assertFalse(result, "Repository should not be exported when package is not allowed");
+    }
+
+    @Test
+    public void testNullConfiguredPackage() {
+        //Override the strategy to use a different package
+        detectionStrategy = new LmsRepositoryDetectionStrategy(null);
+
+        doReturn(TestRestRepositoryExported.class).when(metadata).getRepositoryInterface();
+        boolean result = detectionStrategy.isExported(metadata);
+        assertFalse(result, "Repository should not be exported when package is not allowed");
+    }
+
     // Mock repository classes for testing
     @RestResource
-    interface TestRestRepository {}
+    interface TestRestRepositoryExported {}
 
     @RepositoryRestResource
-    interface TestRepositoryRestResource {}
+    interface TestRepositoryRestResourceExported {}
+
+    @RestResource(exported = false)
+    interface TestRestRepositoryNotExported {}
+
+    @RepositoryRestResource(exported = false)
+    interface TestRepositoryRestResourceNotExported {}
 
     interface TestNoAnnotationRepository {}