feat: start to build out a new integration test framework (#1266)

* feat: start to build out a new integration test framework * feat: start to build out a new integration test framework * allow disabling rate limits + add tests for pages * moar (also fixes #1268) * disable tests for now
HangarMC · Sep 16, 2023 · fb570d5 · fb570d5
1 parent 36e43d5
commit fb570d5
Show file tree

Hide file tree

Showing 23 changed files with 651 additions and 151 deletions.
diff --git a/backend/pom.xml b/backend/pom.xml
@@ -392,6 +392,11 @@
             <version>${mockito-inline.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.platform</groupId>
+            <artifactId>junit-platform-suite</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

diff --git a/backend/src/main/java/io/papermc/hangar/HangarApplication.java b/backend/src/main/java/io/papermc/hangar/HangarApplication.java
@@ -1,6 +1,8 @@
 package io.papermc.hangar;
 
 import io.papermc.hangar.config.hangar.PagesConfig;
+import io.papermc.hangar.security.authentication.HangarPrincipal;
+import java.util.Optional;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.boot.context.properties.ConfigurationPropertiesScan;
@@ -17,8 +19,10 @@
 @ConfigurationPropertiesScan(value = "io.papermc.hangar.config.hangar", basePackageClasses = PagesConfig.class)
 public class HangarApplication {
 
+    public static boolean TEST_MODE = false;
+    public static Optional<HangarPrincipal> TEST_PRINCIPAL = Optional.empty();
+
     public static void main(final String[] args) {
         SpringApplication.run(HangarApplication.class, args);
     }
-
 }
diff --git a/backend/src/main/java/io/papermc/hangar/HangarComponent.java b/backend/src/main/java/io/papermc/hangar/HangarComponent.java
@@ -54,6 +54,9 @@ protected final HangarPrincipal getHangarPrincipal() {
     }
 
     private MemoizingSupplier<Optional<HangarPrincipal>> getHangarPrincipal0() {
+        if (HangarApplication.TEST_MODE) {
+            return MemoizingSupplier.of(() -> HangarApplication.TEST_PRINCIPAL);
+        }
         return MemoizingSupplier.of(() -> Optional.ofNullable(SecurityContextHolder.getContext().getAuthentication())
             .filter(HangarAuthenticationToken.class::isInstance)
             .map(HangarAuthenticationToken.class::cast)

diff --git a/backend/src/main/java/io/papermc/hangar/config/hangar/HangarConfig.java b/backend/src/main/java/io/papermc/hangar/config/hangar/HangarConfig.java
@@ -24,6 +24,7 @@ public class HangarConfig {
     private List<String> licenses = new ArrayList<>();
     private boolean allowIndexing = true;
     private boolean disableJGroups = false;
+    private boolean disableRateLimiting = false;
 
     @NestedConfigurationProperty
     public UpdateTasksConfig updateTasks;
@@ -188,4 +189,12 @@ public boolean isDisableJGroups() {
     public void setDisableJGroups(final boolean disableJGroups) {
         this.disableJGroups = disableJGroups;
     }
+
+    public boolean isDisableRateLimiting() {
+        return this.disableRateLimiting;
+    }
+
+    public void setDisableRateLimiting(final boolean disableRateLimiting) {
+        this.disableRateLimiting = disableRateLimiting;
+    }
 }
diff --git a/backend/src/main/java/io/papermc/hangar/controller/api/v1/PagesController.java b/backend/src/main/java/io/papermc/hangar/controller/api/v1/PagesController.java
@@ -38,9 +38,6 @@ public PagesController(final ProjectPageService projectPageService) {
     @ResponseStatus(HttpStatus.OK)
     public String getMainPage(final String slug) {
         final ExtendedProjectPage projectPage = this.projectPageService.getProjectPage(slug, "");
-        if (projectPage == null) {
-            throw new ResponseStatusException(HttpStatus.NOT_FOUND);
-        }
         return projectPage.getContents();
     }
 
@@ -51,9 +48,6 @@ public String getMainPage(final String slug) {
     @ResponseStatus(HttpStatus.OK)
     public String getPage(final String slug, final String path) {
         final ExtendedProjectPage projectPage = this.projectPageService.getProjectPage(slug, path);
-        if (projectPage == null) {
-            throw new ResponseStatusException(HttpStatus.NOT_FOUND);
-        }
         return projectPage.getContents();
     }
 
@@ -73,10 +67,6 @@ public void editMainPage(final String slug, final StringContent pageEditForm) {
     @ResponseStatus(HttpStatus.OK)
     public void editPage(final String slug, final PageEditForm pageEditForm) {
         final ExtendedProjectPage projectPage = this.projectPageService.getProjectPage(slug, pageEditForm.path());
-        if (projectPage == null) {
-            throw new HangarApiException(HttpStatus.NOT_FOUND, "Project page not found");
-        }
-
         this.projectPageService.saveProjectPage(projectPage.getProjectId(), projectPage.getId(), pageEditForm.content());
     }
 }
diff --git a/backend/src/main/java/io/papermc/hangar/controller/api/v1/PermissionsController.java b/backend/src/main/java/io/papermc/hangar/controller/api/v1/PermissionsController.java
@@ -64,6 +64,7 @@ private Pair<PermissionType, Permission> getPermissionsInScope(final String slug
             perms = this.getHangarPrincipal().getPossiblePermissions().intersect(perms);
             return new ImmutablePair<>(PermissionType.ORGANIZATION, perms);
         } else {
+            // unreachable
             throw new HangarApiException(HttpStatus.BAD_REQUEST, "Incorrect request parameters");
         }
     }

diff --git a/backend/src/main/java/io/papermc/hangar/model/internal/api/requests/StringContent.java b/backend/src/main/java/io/papermc/hangar/model/internal/api/requests/StringContent.java
@@ -10,6 +10,13 @@ public class StringContent {
 
     private @NotBlank(message = "general.error.fieldEmpty") @Schema(description = "A non-null, non-empty string") String content;
 
+    public StringContent() {
+    }
+
+    public StringContent(final String content) {
+        this.content = content;
+    }
+
     public String getContent() {
         return this.content;
     }

diff --git a/.../src/main/java/io/papermc/hangar/security/annotations/ratelimit/RateLimitInterceptor.java b/.../src/main/java/io/papermc/hangar/security/annotations/ratelimit/RateLimitInterceptor.java
@@ -1,6 +1,7 @@
 package io.papermc.hangar.security.annotations.ratelimit;
 
 import io.github.bucket4j.Bucket;
+import io.papermc.hangar.config.hangar.HangarConfig;
 import io.papermc.hangar.exceptions.HangarApiException;
 import io.papermc.hangar.service.internal.BucketService;
 import jakarta.servlet.http.HttpServletRequest;
@@ -19,10 +20,12 @@ public class RateLimitInterceptor implements HandlerInterceptor {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(RateLimitInterceptor.class);
     private final BucketService bucketService;
+    private final HangarConfig hangarConfig;
 
     @Autowired
-    public RateLimitInterceptor(final BucketService bucketService) {
+    public RateLimitInterceptor(final BucketService bucketService, final HangarConfig hangarConfig) {
         this.bucketService = bucketService;
+        this.hangarConfig = hangarConfig;
     }
 
     @Override
@@ -31,6 +34,10 @@ public boolean preHandle(final @NotNull HttpServletRequest request, final @NotNu
             return true;
         }
 
+        if (this.hangarConfig.isDisableRateLimiting()) {
+            return true;
+        }
+
         final Method method = handlerMethod.getMethod();
         final RateLimit limit = method.getAnnotation(RateLimit.class);
         if (limit != null) {

diff --git a/backend/src/main/java/io/papermc/hangar/service/api/ProjectsApiService.java b/backend/src/main/java/io/papermc/hangar/service/api/ProjectsApiService.java
@@ -21,8 +21,10 @@
 import java.util.function.Consumer;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.cache.annotation.Cacheable;
+import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
+import org.springframework.web.server.ResponseStatusException;
 
 @Service
 public class ProjectsApiService extends HangarComponent {
@@ -41,6 +43,9 @@ public ProjectsApiService(final ProjectsApiDAO projectsApiDAO, final UsersApiSer
     public Project getProject(final String slug) {
         final boolean seeHidden = this.getGlobalPermissions().has(Permission.SeeHidden);
         final Project project = this.projectsApiDAO.getProject(slug, seeHidden, this.getHangarUserId());
+        if (project == null) {
+            throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Project " + slug + " not found");
+        }
         project.setAvatarUrl(this.avatarService.getProjectAvatarUrl(project.getId(), project.getNamespace().getOwner()));
         return project;
     }

diff --git a/...d/src/main/java/io/papermc/hangar/service/internal/organizations/OrganizationFactory.java b/...d/src/main/java/io/papermc/hangar/service/internal/organizations/OrganizationFactory.java
@@ -48,7 +48,7 @@ public OrganizationFactory(final UserDAO userDAO, final OrganizationDAO organiza
     }
 
     @Transactional
-    public void createOrganization(final String name) {
+    public OrganizationTable createOrganization(final String name) {
         if (!this.config.org.enabled()) {
             throw new HangarApiException(HttpStatus.BAD_REQUEST, "organization.new.error.notEnabled");
         }
@@ -61,6 +61,7 @@ public void createOrganization(final String name) {
         final OrganizationTable organizationTable = this.organizationDAO.insert(new OrganizationTable(userTable.getId(), name, this.getHangarPrincipal().getId(), userTable.getId(), userTable.getUuid()));
         this.globalRoleService.addRole(GlobalRole.ORGANIZATION.create(null, userTable.getUuid(), userTable.getId(), false));
         this.organizationMemberService.addNewAcceptedByDefaultMember(OrganizationRole.ORGANIZATION_OWNER.create(organizationTable.getId(), userTable.getUuid(), this.getHangarPrincipal().getId(), true));
+        return organizationTable;
     }
 
     @Transactional

diff --git a/backend/src/main/java/io/papermc/hangar/util/RequestUtil.java b/backend/src/main/java/io/papermc/hangar/util/RequestUtil.java
@@ -1,5 +1,6 @@
 package io.papermc.hangar.util;
 
+import io.papermc.hangar.HangarApplication;
 import jakarta.servlet.http.HttpServletRequest;
 import org.springframework.http.HttpStatus;
 import org.springframework.web.server.ResponseStatusException;
@@ -15,6 +16,10 @@ private RequestUtil() {
     private static final String ATTR = "HangarIP";
 
     public static String getRemoteAddress(final HttpServletRequest request) {
+        if (HangarApplication.TEST_MODE) {
+            return "::1";
+        }
+
         final Object attribute = request.getAttribute(ATTR);
         if (attribute instanceof String ip) {
             return ip;

diff --git a/backend/src/main/resources/application.yml b/backend/src/main/resources/application.yml
@@ -61,6 +61,7 @@ hangar:
   ga-code: "UA-38006759-9"
   allow-indexing: true
   disable-jgroups: true
+  disable-ratelimiting: false
 
   licenses:
     - "Unspecified"

diff --git a/backend/src/test/java/io/papermc/hangar/controller/api/v1/ApiKeysControllerTest.java b/backend/src/test/java/io/papermc/hangar/controller/api/v1/ApiKeysControllerTest.java
@@ -0,0 +1,43 @@
+package io.papermc.hangar.controller.api.v1;
+
+import io.papermc.hangar.controller.api.v1.helper.ControllerTest;
+import io.papermc.hangar.controller.api.v1.helper.TestData;
+import io.papermc.hangar.model.common.NamedPermission;
+import io.papermc.hangar.model.internal.api.requests.CreateAPIKeyForm;
+import java.util.Set;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.Matchers.*;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+
+class ApiKeysControllerTest extends ControllerTest {
+
+    @Test
+    void testCreateGetDeleteKey() throws Exception {
+        // create
+        final String newKey = this.mockMvc.perform(post("/api/v1/keys")
+                .with(this.apiKey(TestData.KEY_ADMIN))
+                .header("Content-Type", "application/json")
+                .content(this.objectMapper.writeValueAsBytes(new CreateAPIKeyForm("cool_key", Set.of(NamedPermission.CREATE_PROJECT, NamedPermission.CREATE_ORGANIZATION)))))
+            .andExpect(status().is(201))
+            .andReturn().getResponse().getContentAsString();
+        final String identifier = newKey.split("\\.")[0];
+
+        // get to make sure create worked
+        this.mockMvc.perform(get("/api/v1/keys").with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200))
+            .andExpect(jsonPath("$[*].name").value(hasItem("cool_key")))
+            .andExpect(jsonPath("$[*].tokenIdentifier").value(hasItem(identifier)));
+
+        // delete
+        this.mockMvc.perform(delete("/api/v1/keys?name=cool_key").with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(204));
+
+        // get again to make sure delete worked
+        this.mockMvc.perform(get("/api/v1/keys").with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200))
+            .andExpect(jsonPath("$[*].name").value(not(hasItem("cool_key"))))
+            .andExpect(jsonPath("$[*].tokenIdentifier").value(not(hasItem(identifier))));
+    }
+}
diff --git a/backend/src/test/java/io/papermc/hangar/controller/api/v1/PagesControllerTest.java b/backend/src/test/java/io/papermc/hangar/controller/api/v1/PagesControllerTest.java
@@ -0,0 +1,130 @@
+package io.papermc.hangar.controller.api.v1;
+
+import io.papermc.hangar.controller.api.v1.helper.ControllerTest;
+import io.papermc.hangar.controller.api.v1.helper.TestData;
+import io.papermc.hangar.model.api.project.PageEditForm;
+import io.papermc.hangar.model.common.NamedPermission;
+import io.papermc.hangar.model.internal.api.requests.CreateAPIKeyForm;
+import io.papermc.hangar.model.internal.api.requests.StringContent;
+import java.util.Set;
+import org.junit.jupiter.api.Test;
+import org.springframework.http.MediaType;
+
+import static org.hamcrest.Matchers.*;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+
+class PagesControllerTest extends ControllerTest {
+
+    @Test
+    void testGetMainPage() throws Exception {
+        this.mockMvc.perform(get("/api/v1/pages/main/" + TestData.PROJECT.getSlug())
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200))
+            .andExpect(content().string(startsWith("# Test")));
+    }
+
+    @Test
+    void testEditMainPage() throws Exception {
+        // edit
+        this.mockMvc.perform(patch("/api/v1/pages/editmain/" + TestData.PROJECT.getSlug())
+                .content(this.objectMapper.writeValueAsBytes(new StringContent("# Test\nEdited")))
+                .contentType(MediaType.APPLICATION_JSON)
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200));
+
+        // validate
+        this.mockMvc.perform(get("/api/v1/pages/main/" + TestData.PROJECT.getSlug())
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200))
+            .andExpect(content().string(containsString("Edited")));
+    }
+
+    @Test
+    void testGetOtherPage() throws Exception {
+        this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=" + TestData.PAGE_PARENT.getSlug())
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200))
+            .andExpect(content().string(startsWith("# TestParentPage")));
+    }
+
+    @Test
+    void testSlashes() throws Exception {
+        this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=/" + TestData.PAGE_PARENT.getSlug() + "/")
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200))
+            .andExpect(content().string(startsWith("# TestParentPage")));
+    }
+
+    @Test
+    void testEditOtherPage() throws Exception {
+        // edit
+        this.mockMvc.perform(patch("/api/v1/pages/edit/" + TestData.PROJECT.getSlug())
+                .content(this.objectMapper.writeValueAsBytes(new PageEditForm(TestData.PAGE_PARENT.getSlug(), "# TestParentPage\nEdited")))
+                .contentType(MediaType.APPLICATION_JSON)
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200));
+
+        // validate
+        this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=" + TestData.PAGE_PARENT.getSlug())
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200))
+            .andExpect(content().string(containsString("Edited")));
+    }
+
+    @Test
+    void testGetChildPage() throws Exception {
+        this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=" + TestData.PAGE_CHILD.getSlug())
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200))
+            .andExpect(content().string(startsWith("# TestChildPage")));
+    }
+
+    @Test
+    void testEditChildPage() throws Exception {
+        // edit
+        this.mockMvc.perform(patch("/api/v1/pages/edit/" + TestData.PROJECT.getSlug())
+                .content(this.objectMapper.writeValueAsBytes(new PageEditForm(TestData.PAGE_CHILD.getSlug(), "# TestChildPage\nEdited")))
+                .contentType(MediaType.APPLICATION_JSON)
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200));
+
+        // validate
+        this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=" + TestData.PAGE_CHILD.getSlug())
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200))
+            .andExpect(content().string(containsString("Edited")));
+    }
+
+    @Test
+    void testGetInvalidProject() throws Exception {
+        this.mockMvc.perform(get("/api/v1/pages/main/Dum")
+            .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(404));
+    }
+
+    @Test
+    void testGetInvalidPage() throws Exception {
+        this.mockMvc.perform(get("/api/v1/pages/page/" + TestData.PROJECT.getSlug() + "?path=Dum")
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(404));
+    }
+
+    @Test
+    void testEditInvalidProject() throws Exception {
+        this.mockMvc.perform(patch("/api/v1/pages/editmain/Dum")
+                .content(this.objectMapper.writeValueAsBytes(new StringContent("# Dum")))
+                .contentType(MediaType.APPLICATION_JSON)
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(404));
+    }
+
+    @Test
+    void testEditInvalidPage() throws Exception {
+        this.mockMvc.perform(patch("/api/v1/pages/edit/" + TestData.PROJECT.getSlug())
+                .content(this.objectMapper.writeValueAsBytes(new PageEditForm(TestData.PAGE_PARENT.getSlug(), "# TestParentPage\nEdited")))
+                .contentType(MediaType.APPLICATION_JSON)
+                .with(this.apiKey(TestData.KEY_ADMIN)))
+            .andExpect(status().is(200));
+    }
+}