From 7437706da0197a38ec1f2d032271cf8aae69a2af Mon Sep 17 00:00:00 2001 From: Tristan Verbeken Date: Fri, 17 May 2024 23:45:24 +0200 Subject: [PATCH 1/5] Added tryTemplate function --- .../SubmissionTemplateModel.java | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java index 13bdd40b..913181d1 100644 --- a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java +++ b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java @@ -52,7 +52,6 @@ public void parseSubmissionTemplate(String templateString) { mostSpaces = spaceAmount; } lines[i] = "\t".repeat(tabsPerSpaces.get(spaceAmount)) + line.replaceAll(" ", ""); - ; } // Create folder stack for keeping track of all the folders while exploring the insides @@ -175,4 +174,63 @@ public SubmissionResult checkSubmission(String file) throws IOException { return checkSubmission(new ZipFile(file)); } + // will throw error if there are errors in the template + public void tryTemplate(String template) throws Exception { + List lines = List.of(template.split("\n")); + // check if the template is valid, control if every line contains a file parsable string + // check if the file is in a valid folder location (indentation is correct) + // check if the first file has indentation 0 + List indentionAmounts = new ArrayList<>(); + indentionAmounts.add(0); + if(getIndentation(lines.get(0)) != 0){ + throw new Exception("First file should not have any spaces or tabs."); + } + boolean newFolder = false; + for(int line_index = 0; line_index < lines.size(); line_index++){ + String line = lines.get(line_index); + int indentation = getIndentation(line); + if(line.isEmpty()){ + throw new Exception("Empty file name in template, remove blank lines"); + } + if(newFolder && indentation > indentionAmounts.getLast()){ + // since the indentation is larger than the previous, we are dealing with the first file in a new folder + indentionAmounts.add(indentation); + }else{ + // we are dealing with a file in a folder, thus the indentation should be equal to one of the previous folders + for(int i = indentionAmounts.size() - 1; i >= 0; i--){ + if(indentionAmounts.get(i) == indentation){ + break; + } + if(i == 0){ + throw new Exception("File at line "+ line_index + " is not in a valid folder location (indentation is incorrect)"); + } + } + // check if file is correct, since location is correct + + // first check if file contains valid file names + if(line.substring(0,line.length() - 1).contains("/")){ + throw new Exception("File at line "+ line_index + " contains invalid characters"); + } + // check if file is a folder + if(line.charAt(line.length() - 1) == '/') { + newFolder = true; + } + + } + + + } + } + + private int getIndentation(String line){ + int length = line.length(); + // one space is equal to a tab + for(int i = 0; i < length; i++){ + if(line.charAt(i) != ' ' || line.charAt(i) != '\t'){ + return i - 1; + } + } + return length - 1; + } + } From 87f1b576713f8636f433813adf3dd45812150053 Mon Sep 17 00:00:00 2001 From: Tristan Verbeken Date: Mon, 20 May 2024 01:47:36 +0200 Subject: [PATCH 2/5] Added regex inversion, revamped template validation with proper error throwing. Fixed the tests. Added template validation for structure templates. --- .../pidgeon/controllers/TestController.java | 4 +- .../DockerSubmissionTestModel.java | 19 ++++-- .../SubmissionTemplateModel.java | 58 ++++++++++++++----- .../java/com/ugent/pidgeon/util/TestUtil.java | 19 +++++- .../controllers/TestControllerTest.java | 13 +++++ .../docker/DockerSubmissionTestTest.java | 12 ++-- .../pidgeon/model/FileStructureTest.java | 13 +++++ .../com/ugent/pidgeon/util/TestUtilTest.java | 21 +++++++ .../allowAll/template.txt | 2 +- .../denyAllFiles/template.txt | 2 +- 10 files changed, 136 insertions(+), 27 deletions(-) diff --git a/backend/app/src/main/java/com/ugent/pidgeon/controllers/TestController.java b/backend/app/src/main/java/com/ugent/pidgeon/controllers/TestController.java index e614fc5b..b91693d7 100644 --- a/backend/app/src/main/java/com/ugent/pidgeon/controllers/TestController.java +++ b/backend/app/src/main/java/com/ugent/pidgeon/controllers/TestController.java @@ -108,7 +108,7 @@ private ResponseEntity alterTests( structureTemplate = null; } - CheckResult> updateCheckResult = testUtil.checkForTestUpdate(projectId, user, dockerImage, dockerScript, dockerTemplate, httpMethod); + CheckResult> updateCheckResult = testUtil.checkForTestUpdate(projectId, user, dockerImage, dockerScript, dockerTemplate, structureTemplate, httpMethod); if (!updateCheckResult.getStatus().equals(HttpStatus.OK)) { @@ -220,7 +220,7 @@ public ResponseEntity getTests(@PathVariable("projectid") long projectId, Aut @DeleteMapping(ApiRoutes.PROJECT_BASE_PATH + "/{projectid}/tests") @Roles({UserRole.teacher, UserRole.student}) public ResponseEntity deleteTestById(@PathVariable("projectid") long projectId, Auth auth) { - CheckResult> updateCheckResult = testUtil.checkForTestUpdate(projectId, auth.getUserEntity(), null, null, null, HttpMethod.DELETE); + CheckResult> updateCheckResult = testUtil.checkForTestUpdate(projectId, auth.getUserEntity(), null, null, null, null, HttpMethod.DELETE); if (!updateCheckResult.getStatus().equals(HttpStatus.OK)) { return ResponseEntity.status(updateCheckResult.getStatus()).body(updateCheckResult.getMessage()); } diff --git a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/DockerSubmissionTestModel.java b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/DockerSubmissionTestModel.java index f336ff03..fdddad2c 100644 --- a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/DockerSubmissionTestModel.java +++ b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/DockerSubmissionTestModel.java @@ -334,14 +334,14 @@ public static boolean imageExists(String image) { } } - public static boolean isValidTemplate(String template) { + public static void tryTemplate(String template) { // lines with @ should be the first of a string // @ is always the first character // ">" options under the template should be "required, optional or description="..." boolean atLeastOne = false; // Template should not be empty String[] lines = template.split("\n"); if (lines[0].charAt(0) != '@') { - return false; + throw new IllegalArgumentException("Template should start with a '@'"); } boolean isConfigurationLine = false; for (String line : lines) { @@ -359,14 +359,25 @@ public static boolean isValidTemplate(String template) { // option lines if (!line.equalsIgnoreCase(">Required") && !line.equalsIgnoreCase(">Optional") && !isDescription) { - return false; + throw new IllegalArgumentException("Invalid option in template"); } } else { isConfigurationLine = false; } } } - return atLeastOne; + if(! atLeastOne){ + throw new IllegalArgumentException("Template should not be empty"); + } + } + + public static boolean isValidTemplate(String template){ + try{ + tryTemplate(template); + return true; + }catch (Exception e){ + return false; + } } } diff --git a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java index 913181d1..cf24c9e1 100644 --- a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java +++ b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java @@ -13,10 +13,39 @@ public class SubmissionTemplateModel { private static class FileEntry { public String name; Pattern pattern; + private String invert(String name){ + // invert . with \. for simpler filenames + List dotLocations = new ArrayList<>(); + List escapedDotLocations = new ArrayList<>(); + for (int i = 0; i < name.length(); i++) { + if(i > 0 && name.charAt(i - 1) == '\\' && name.charAt(i) == '.'){ + escapedDotLocations.add(i); + }else if (name.charAt(i) == '.') { + dotLocations.add(i); + } + } + StringBuilder sb = new StringBuilder(); + for(int i = 0; i < name.length(); i++) { + if(escapedDotLocations.contains(i + 1)){ + // skip the break + continue; + }else if(escapedDotLocations.contains(i)){ + sb.append("."); + }else if(dotLocations.contains(i)){ + sb.append("\\."); + }else{ + sb.append(name.charAt(i)); + } + } + return sb.toString(); + + } public FileEntry(String name) { - this.name = name; - pattern = Pattern.compile("^" + name + "$"); // hat for defining start of the string, $ defines the end + + this.name = invert(name); + + pattern = Pattern.compile("^" + this.name + "$"); // hat for defining start of the string, $ defines the end } public boolean matches(String fileName) { @@ -175,7 +204,7 @@ public SubmissionResult checkSubmission(String file) throws IOException { } // will throw error if there are errors in the template - public void tryTemplate(String template) throws Exception { + public static void tryTemplate(String template) throws IllegalArgumentException { List lines = List.of(template.split("\n")); // check if the template is valid, control if every line contains a file parsable string // check if the file is in a valid folder location (indentation is correct) @@ -183,18 +212,19 @@ public void tryTemplate(String template) throws Exception { List indentionAmounts = new ArrayList<>(); indentionAmounts.add(0); if(getIndentation(lines.get(0)) != 0){ - throw new Exception("First file should not have any spaces or tabs."); + throw new IllegalArgumentException("First file should not have any spaces or tabs."); } boolean newFolder = false; for(int line_index = 0; line_index < lines.size(); line_index++){ String line = lines.get(line_index); int indentation = getIndentation(line); if(line.isEmpty()){ - throw new Exception("Empty file name in template, remove blank lines"); + throw new IllegalArgumentException("Empty file name in template, remove blank lines"); } if(newFolder && indentation > indentionAmounts.getLast()){ // since the indentation is larger than the previous, we are dealing with the first file in a new folder indentionAmounts.add(indentation); + newFolder = false; }else{ // we are dealing with a file in a folder, thus the indentation should be equal to one of the previous folders for(int i = indentionAmounts.size() - 1; i >= 0; i--){ @@ -202,32 +232,34 @@ public void tryTemplate(String template) throws Exception { break; } if(i == 0){ - throw new Exception("File at line "+ line_index + " is not in a valid folder location (indentation is incorrect)"); + throw new IllegalArgumentException("File at line "+ line_index + " is not in a valid folder location (indentation is incorrect)"); } } // check if file is correct, since location is correct // first check if file contains valid file names if(line.substring(0,line.length() - 1).contains("/")){ - throw new Exception("File at line "+ line_index + " contains invalid characters"); + throw new IllegalArgumentException("File at line "+ line_index + " contains invalid characters"); } // check if file is a folder if(line.charAt(line.length() - 1) == '/') { newFolder = true; } - } - - + if(line.charAt(line.length() - 1) == '/'){ + // new folder start! + newFolder = true; + } } } - private int getIndentation(String line){ + + private static int getIndentation(String line){ int length = line.length(); // one space is equal to a tab for(int i = 0; i < length; i++){ - if(line.charAt(i) != ' ' || line.charAt(i) != '\t'){ - return i - 1; + if(line.charAt(i) != ' ' && line.charAt(i) != '\t'){ + return i; } } return length - 1; diff --git a/backend/app/src/main/java/com/ugent/pidgeon/util/TestUtil.java b/backend/app/src/main/java/com/ugent/pidgeon/util/TestUtil.java index 13c28bc0..098cf99f 100644 --- a/backend/app/src/main/java/com/ugent/pidgeon/util/TestUtil.java +++ b/backend/app/src/main/java/com/ugent/pidgeon/util/TestUtil.java @@ -50,6 +50,7 @@ public CheckResult> checkForTestUpdate( String dockerImage, String dockerScript, String dockerTemplate, + String structureTemplate, HttpMethod httpMethod ) { @@ -96,8 +97,22 @@ public CheckResult> checkForTestUpdate( return new CheckResult<>(HttpStatus.BAD_REQUEST, "No docker test script is configured for this test", null); } - if(dockerTemplate != null && !DockerSubmissionTestModel.isValidTemplate(dockerTemplate)) { - return new CheckResult<>(HttpStatus.BAD_REQUEST, "Invalid docker template", null); + try{ + // throws error if there are issues in the template + if(dockerTemplate != null) DockerSubmissionTestModel.tryTemplate(dockerTemplate); + if(structureTemplate != null) DockerSubmissionTestModel.tryTemplate(structureTemplate); + + }catch(IllegalArgumentException e){ + return new CheckResult<>(HttpStatus.BAD_REQUEST, e.getMessage(), null); + } + + if(dockerTemplate != null){ + try{ + DockerSubmissionTestModel.tryTemplate(dockerTemplate); + }catch (IllegalArgumentException e){ + return new CheckResult<>(HttpStatus.BAD_REQUEST, e.getMessage(), null); + } + } return new CheckResult<>(HttpStatus.OK, "", new Pair<>(testEntity, projectEntity)); diff --git a/backend/app/src/test/java/com/ugent/pidgeon/controllers/TestControllerTest.java b/backend/app/src/test/java/com/ugent/pidgeon/controllers/TestControllerTest.java index 686e9144..388a2675 100644 --- a/backend/app/src/test/java/com/ugent/pidgeon/controllers/TestControllerTest.java +++ b/backend/app/src/test/java/com/ugent/pidgeon/controllers/TestControllerTest.java @@ -169,6 +169,7 @@ public void testUpdateTest() throws Exception { eq(dockerImage), eq(dockerTestScript), eq(dockerTestTemplate), + eq(structureTemplate), eq(HttpMethod.POST) )).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(null, project))); @@ -219,6 +220,7 @@ public void testUpdateTest() throws Exception { eq(null), eq(null), eq(null), + eq(null), eq(HttpMethod.POST) )).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(null, project))); @@ -268,6 +270,7 @@ public void testUpdateTest() throws Exception { eq(dockerImage), eq(dockerTestScript), eq(dockerTestTemplate), + eq(structureTemplate), eq(HttpMethod.POST) )).thenReturn(new CheckResult<>(HttpStatus.I_AM_A_TEAPOT, "I'm a teapot", null)); @@ -326,6 +329,7 @@ public void testPutTest() throws Exception { eq(dockerImage), eq(dockerTestScript), eq(dockerTestTemplate), + eq(structureTemplate), eq(HttpMethod.PUT) )).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project))); @@ -380,6 +384,7 @@ public void testPutTest() throws Exception { eq(null), eq(null), eq(null), + eq(null), eq(HttpMethod.PUT) )).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project))); @@ -449,6 +454,7 @@ public void testPutTest() throws Exception { eq(dockerImage), eq(dockerTestScript), eq(dockerTestTemplate), + eq(structureTemplate), eq(HttpMethod.PUT) )).thenReturn(new CheckResult<>(HttpStatus.I_AM_A_TEAPOT, "I'm a teapot", null)); @@ -482,6 +488,7 @@ public void testGetPatch() throws Exception { eq(dockerImage), eq(null), eq(null), + eq(null), eq(HttpMethod.PATCH) )).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project))); @@ -523,6 +530,7 @@ public void testGetPatch() throws Exception { eq(null), eq(dockerTestScript), eq(null), + eq(null), eq(HttpMethod.PATCH) )).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project))); @@ -555,6 +563,7 @@ public void testGetPatch() throws Exception { eq(null), eq(null), eq(dockerTestTemplate), + eq(null), eq(HttpMethod.PATCH) )).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project))); @@ -587,6 +596,7 @@ public void testGetPatch() throws Exception { eq(null), eq(null), eq(null), + eq(null), eq(HttpMethod.PATCH) )).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project))); @@ -619,6 +629,7 @@ public void testGetPatch() throws Exception { eq(dockerImage), eq(null), eq(null), + eq(null), eq(HttpMethod.PATCH) )).thenReturn(new CheckResult<>(HttpStatus.I_AM_A_TEAPOT, "I'm a teapot", null)); @@ -679,6 +690,7 @@ public void testDeleteTest() throws Exception { eq(null), eq(null), eq(null), + eq(null), eq(HttpMethod.DELETE) )).thenReturn(new CheckResult<>(HttpStatus.OK, "", new Pair<>(test, project))); @@ -700,6 +712,7 @@ public void testDeleteTest() throws Exception { eq(null), eq(null), eq(null), + eq(null), eq(HttpMethod.DELETE) )).thenReturn(new CheckResult<>(HttpStatus.I_AM_A_TEAPOT, "I'm a teapot", null)); diff --git a/backend/app/src/test/java/com/ugent/pidgeon/docker/DockerSubmissionTestTest.java b/backend/app/src/test/java/com/ugent/pidgeon/docker/DockerSubmissionTestTest.java index 688b3157..83b08576 100644 --- a/backend/app/src/test/java/com/ugent/pidgeon/docker/DockerSubmissionTestTest.java +++ b/backend/app/src/test/java/com/ugent/pidgeon/docker/DockerSubmissionTestTest.java @@ -1,7 +1,9 @@ package com.ugent.pidgeon.docker; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.ugent.pidgeon.model.submissionTesting.DockerSubmissionTestModel; @@ -202,13 +204,15 @@ void dockerImageDoesNotExist(){ } @Test - void isValidTemplate(){ - assertFalse(DockerSubmissionTestModel.isValidTemplate("This is not a valid template")); - assertTrue(DockerSubmissionTestModel.isValidTemplate("@HelloWorld\n" + + void tryTemplate(){ + assertThrows(IllegalArgumentException.class,() -> DockerSubmissionTestModel.tryTemplate("This is not a valid template")); + + + assertDoesNotThrow(() -> DockerSubmissionTestModel.tryTemplate("@HelloWorld\n" + ">Description=\"Test for hello world!\"\n" + ">Required\n" + "HelloWorld!")); - assertTrue(DockerSubmissionTestModel.isValidTemplate("@helloworld\n" + assertDoesNotThrow(() -> DockerSubmissionTestModel.tryTemplate("@helloworld\n" + ">required\n" + ">description=\"Helloworldtest\"\n" + "Hello World\n" diff --git a/backend/app/src/test/java/com/ugent/pidgeon/model/FileStructureTest.java b/backend/app/src/test/java/com/ugent/pidgeon/model/FileStructureTest.java index 3d7a74c3..63889c90 100644 --- a/backend/app/src/test/java/com/ugent/pidgeon/model/FileStructureTest.java +++ b/backend/app/src/test/java/com/ugent/pidgeon/model/FileStructureTest.java @@ -7,7 +7,9 @@ import java.nio.file.Files; import java.nio.file.Path; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class FileStructureTest { @@ -43,6 +45,17 @@ void isEmpty(){ void denyFileExtension(){ assertFalse(runTest("noClassExtensions")); } + + @Test + void tryTemplateTest(){ + assertDoesNotThrow(() -> SubmissionTemplateModel.tryTemplate("template")); + assertDoesNotThrow(() -> SubmissionTemplateModel.tryTemplate("src/\n index.js\n seconfilehehe.file")); + assertThrows(IllegalArgumentException.class, () -> SubmissionTemplateModel.tryTemplate("src/\n index.js\n seconfilehehe.file\n thirdfile")); + //check trailing newline + assertDoesNotThrow(() -> SubmissionTemplateModel.tryTemplate("src/\n\tindex.js\n")); + + } + private boolean runTest(String testpath){ SubmissionTemplateModel model = new SubmissionTemplateModel(); if(testpath.lastIndexOf('/') != testpath.length() - 1){ diff --git a/backend/app/src/test/java/com/ugent/pidgeon/util/TestUtilTest.java b/backend/app/src/test/java/com/ugent/pidgeon/util/TestUtilTest.java index c3ae300b..a525f523 100644 --- a/backend/app/src/test/java/com/ugent/pidgeon/util/TestUtilTest.java +++ b/backend/app/src/test/java/com/ugent/pidgeon/util/TestUtilTest.java @@ -1,6 +1,7 @@ package com.ugent.pidgeon.util; import com.ugent.pidgeon.model.submissionTesting.DockerSubmissionTestModel; +import com.ugent.pidgeon.model.submissionTesting.SubmissionTemplateModel; import com.ugent.pidgeon.postgre.models.ProjectEntity; import com.ugent.pidgeon.postgre.models.TestEntity; import com.ugent.pidgeon.postgre.models.UserEntity; @@ -93,6 +94,7 @@ public void testCheckForTestUpdate() { String dockerImage = "dockerImage"; String dockerScript = "dockerScript"; String dockerTemplate = "@dockerTemplate\nExpectedOutput"; + String structureTemplate = "src/\n\tindex.js\n"; HttpMethod httpMethod = HttpMethod.POST; when(projectUtil.getProjectIfAdmin(projectEntity.getId(), userEntity)) @@ -103,6 +105,7 @@ public void testCheckForTestUpdate() { try (MockedStatic mockedTestModel = mockStatic(DockerSubmissionTestModel.class)) { mockedTestModel.when(() -> DockerSubmissionTestModel.imageExists(dockerImage)).thenReturn(true); mockedTestModel.when(() -> DockerSubmissionTestModel.isValidTemplate(any())).thenReturn(true); + projectEntity.setTestId(null); CheckResult> result = testUtil.checkForTestUpdate( projectEntity.getId(), @@ -110,6 +113,7 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.OK, result.getStatus()); @@ -124,6 +128,7 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, dockerTemplate, + structureTemplate, HttpMethod.POST ); assertEquals(HttpStatus.OK, result.getStatus()); @@ -138,6 +143,7 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.BAD_REQUEST, result.getStatus()); @@ -153,6 +159,7 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, null, + structureTemplate, httpMethod ); assertEquals(HttpStatus.OK, result.getStatus()); @@ -165,6 +172,7 @@ public void testCheckForTestUpdate() { dockerImage, null, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.BAD_REQUEST, result.getStatus()); @@ -177,6 +185,7 @@ public void testCheckForTestUpdate() { dockerImage, null, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.OK, result.getStatus()); @@ -189,6 +198,7 @@ public void testCheckForTestUpdate() { null, dockerScript, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.BAD_REQUEST, result.getStatus()); @@ -201,6 +211,7 @@ public void testCheckForTestUpdate() { null, dockerScript, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.OK, result.getStatus()); @@ -214,6 +225,7 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.OK, result.getStatus()); @@ -228,6 +240,7 @@ public void testCheckForTestUpdate() { null, null, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.BAD_REQUEST, result.getStatus()); @@ -239,6 +252,7 @@ public void testCheckForTestUpdate() { null, null, null, + structureTemplate, httpMethod ); assertEquals(HttpStatus.OK, result.getStatus()); @@ -251,6 +265,7 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.BAD_REQUEST, result.getStatus()); @@ -263,6 +278,7 @@ public void testCheckForTestUpdate() { dockerImage, null, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.BAD_REQUEST, result.getStatus()); @@ -274,6 +290,7 @@ public void testCheckForTestUpdate() { null, dockerScript, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.BAD_REQUEST, result.getStatus()); @@ -286,6 +303,7 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, dockerTemplate, + structureTemplate, HttpMethod.POST ); assertEquals(HttpStatus.CONFLICT, result.getStatus()); @@ -298,6 +316,7 @@ public void testCheckForTestUpdate() { null, null, null, + structureTemplate, httpMethod ); assertEquals(HttpStatus.OK, result.getStatus()); @@ -310,6 +329,7 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, dockerTemplate, + structureTemplate, HttpMethod.PATCH ); assertEquals(HttpStatus.NOT_FOUND, result.getStatus()); @@ -324,6 +344,7 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, dockerTemplate, + structureTemplate, httpMethod ); assertEquals(HttpStatus.I_AM_A_TEAPOT, result.getStatus()); diff --git a/backend/app/src/test/test-cases/FileStructureTestCases/allowAll/template.txt b/backend/app/src/test/test-cases/FileStructureTestCases/allowAll/template.txt index 9abb766c..3f5e9c7c 100644 --- a/backend/app/src/test/test-cases/FileStructureTestCases/allowAll/template.txt +++ b/backend/app/src/test/test-cases/FileStructureTestCases/allowAll/template.txt @@ -1 +1 @@ -.* \ No newline at end of file +\.* \ No newline at end of file diff --git a/backend/app/src/test/test-cases/FileStructureTestCases/denyAllFiles/template.txt b/backend/app/src/test/test-cases/FileStructureTestCases/denyAllFiles/template.txt index a75f68df..ff348fc9 100644 --- a/backend/app/src/test/test-cases/FileStructureTestCases/denyAllFiles/template.txt +++ b/backend/app/src/test/test-cases/FileStructureTestCases/denyAllFiles/template.txt @@ -1 +1 @@ --. \ No newline at end of file +-\. \ No newline at end of file From d21c93c829018135865abfee5ae4da70fd1fae30 Mon Sep 17 00:00:00 2001 From: Aqua-sc <108478185+Aqua-sc@users.noreply.github.com> Date: Mon, 20 May 2024 10:34:24 +0200 Subject: [PATCH 3/5] Fix tests --- .../java/com/ugent/pidgeon/util/TestUtil.java | 15 +++----- .../controllers/TestControllerTest.java | 2 +- .../com/ugent/pidgeon/util/TestUtilTest.java | 35 +++++++++++++++---- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/backend/app/src/main/java/com/ugent/pidgeon/util/TestUtil.java b/backend/app/src/main/java/com/ugent/pidgeon/util/TestUtil.java index 098cf99f..663ac04e 100644 --- a/backend/app/src/main/java/com/ugent/pidgeon/util/TestUtil.java +++ b/backend/app/src/main/java/com/ugent/pidgeon/util/TestUtil.java @@ -3,6 +3,7 @@ import com.ugent.pidgeon.controllers.ApiRoutes; import com.ugent.pidgeon.model.json.TestJson; import com.ugent.pidgeon.model.submissionTesting.DockerSubmissionTestModel; +import com.ugent.pidgeon.model.submissionTesting.SubmissionTemplateModel; import com.ugent.pidgeon.postgre.models.ProjectEntity; import com.ugent.pidgeon.postgre.models.TestEntity; import com.ugent.pidgeon.postgre.models.UserEntity; @@ -80,7 +81,6 @@ public CheckResult> checkForTestUpdate( return new CheckResult<>(HttpStatus.BAD_REQUEST, "A docker image is required if u add a script", null); } - // This returns false if the image isn't pullt yet! FIX PLS if(dockerImage != null && !DockerSubmissionTestModel.imageExists(dockerImage)) { return new CheckResult<>(HttpStatus.BAD_REQUEST, "A valid docker image is required in a docker test.", null); } @@ -97,23 +97,16 @@ public CheckResult> checkForTestUpdate( return new CheckResult<>(HttpStatus.BAD_REQUEST, "No docker test script is configured for this test", null); } - try{ + try { // throws error if there are issues in the template if(dockerTemplate != null) DockerSubmissionTestModel.tryTemplate(dockerTemplate); - if(structureTemplate != null) DockerSubmissionTestModel.tryTemplate(structureTemplate); + if(structureTemplate != null) SubmissionTemplateModel.tryTemplate(structureTemplate); - }catch(IllegalArgumentException e){ + } catch(IllegalArgumentException e){ return new CheckResult<>(HttpStatus.BAD_REQUEST, e.getMessage(), null); } - if(dockerTemplate != null){ - try{ - DockerSubmissionTestModel.tryTemplate(dockerTemplate); - }catch (IllegalArgumentException e){ - return new CheckResult<>(HttpStatus.BAD_REQUEST, e.getMessage(), null); - } - } return new CheckResult<>(HttpStatus.OK, "", new Pair<>(testEntity, projectEntity)); } diff --git a/backend/app/src/test/java/com/ugent/pidgeon/controllers/TestControllerTest.java b/backend/app/src/test/java/com/ugent/pidgeon/controllers/TestControllerTest.java index 388a2675..16664d59 100644 --- a/backend/app/src/test/java/com/ugent/pidgeon/controllers/TestControllerTest.java +++ b/backend/app/src/test/java/com/ugent/pidgeon/controllers/TestControllerTest.java @@ -596,7 +596,7 @@ public void testGetPatch() throws Exception { eq(null), eq(null), eq(null), - eq(null), + eq(structureTemplate), eq(HttpMethod.PATCH) )).thenReturn(new CheckResult<>(HttpStatus.OK, "",new Pair<>(test, project))); diff --git a/backend/app/src/test/java/com/ugent/pidgeon/util/TestUtilTest.java b/backend/app/src/test/java/com/ugent/pidgeon/util/TestUtilTest.java index a525f523..34359d70 100644 --- a/backend/app/src/test/java/com/ugent/pidgeon/util/TestUtilTest.java +++ b/backend/app/src/test/java/com/ugent/pidgeon/util/TestUtilTest.java @@ -102,9 +102,14 @@ public void testCheckForTestUpdate() { doReturn(testEntity).when(testUtil).getTestIfExists(projectEntity.getId()); - try (MockedStatic mockedTestModel = mockStatic(DockerSubmissionTestModel.class)) { + try (MockedStatic mockedTestModel = mockStatic(DockerSubmissionTestModel.class); + MockedStatic mockedTemplateModel = mockStatic(SubmissionTemplateModel.class) + ) { mockedTestModel.when(() -> DockerSubmissionTestModel.imageExists(dockerImage)).thenReturn(true); - mockedTestModel.when(() -> DockerSubmissionTestModel.isValidTemplate(any())).thenReturn(true); + mockedTestModel.when(() -> DockerSubmissionTestModel.tryTemplate(dockerTemplate)).then( + invocation -> null); + mockedTemplateModel.when(() -> SubmissionTemplateModel.tryTemplate(structureTemplate)).then( + invocation -> null); projectEntity.setTestId(null); CheckResult> result = testUtil.checkForTestUpdate( @@ -128,15 +133,32 @@ public void testCheckForTestUpdate() { dockerImage, dockerScript, dockerTemplate, - structureTemplate, + null, HttpMethod.POST ); assertEquals(HttpStatus.OK, result.getStatus()); doReturn(testEntity).when(testUtil).getTestIfExists(projectEntity.getId()); - /* Not a valid template */ - when(DockerSubmissionTestModel.isValidTemplate(any())).thenReturn(false); + /* Not a valid docker template */ + mockedTestModel.when(() -> DockerSubmissionTestModel.tryTemplate(dockerTemplate)) + .thenThrow(new IllegalArgumentException("Invalid template")); + result = testUtil.checkForTestUpdate( + projectEntity.getId(), + userEntity, + dockerImage, + dockerScript, + dockerTemplate, + structureTemplate, + httpMethod + ); + assertEquals(HttpStatus.BAD_REQUEST, result.getStatus()); + mockedTestModel.when(() -> DockerSubmissionTestModel.tryTemplate(dockerTemplate)).then( + invocation -> null); + + /* Invalid structure template */ + mockedTemplateModel.when(() -> SubmissionTemplateModel.tryTemplate(structureTemplate)) + .thenThrow(new IllegalArgumentException("Invalid template")); result = testUtil.checkForTestUpdate( projectEntity.getId(), userEntity, @@ -147,7 +169,8 @@ public void testCheckForTestUpdate() { httpMethod ); assertEquals(HttpStatus.BAD_REQUEST, result.getStatus()); - when(DockerSubmissionTestModel.isValidTemplate(any())).thenReturn(true); + mockedTemplateModel.when(() -> SubmissionTemplateModel.tryTemplate(structureTemplate)). + then(invocation -> null); /* Method is patch and no template provided */ From 1d2bcd2beff4c57acadab74b4b2a1ac41e9e7ad7 Mon Sep 17 00:00:00 2001 From: Aqua-sc <108478185+Aqua-sc@users.noreply.github.com> Date: Mon, 20 May 2024 10:59:17 +0200 Subject: [PATCH 4/5] small change to feedback --- .../model/submissionTesting/SubmissionTemplateModel.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java index cf24c9e1..6b26100e 100644 --- a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java +++ b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java @@ -239,7 +239,7 @@ public static void tryTemplate(String template) throws IllegalArgumentException // first check if file contains valid file names if(line.substring(0,line.length() - 1).contains("/")){ - throw new IllegalArgumentException("File at line "+ line_index + " contains invalid characters"); + throw new IllegalArgumentException("File/folder at line "+ (line_index+1) + " contains invalid characters"); } // check if file is a folder if(line.charAt(line.length() - 1) == '/') { From 215182aaf92ce83c532d6b32d5be010b25d8afd0 Mon Sep 17 00:00:00 2001 From: Aqua-sc <108478185+Aqua-sc@users.noreply.github.com> Date: Tue, 21 May 2024 12:10:16 +0200 Subject: [PATCH 5/5] get(...) ipv getLast() --- .../model/submissionTesting/SubmissionTemplateModel.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java index 6b26100e..1e83f944 100644 --- a/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java +++ b/backend/app/src/main/java/com/ugent/pidgeon/model/submissionTesting/SubmissionTemplateModel.java @@ -221,7 +221,7 @@ public static void tryTemplate(String template) throws IllegalArgumentException if(line.isEmpty()){ throw new IllegalArgumentException("Empty file name in template, remove blank lines"); } - if(newFolder && indentation > indentionAmounts.getLast()){ + if(newFolder && indentation > indentionAmounts.get(indentionAmounts.size() - 1)){ // since the indentation is larger than the previous, we are dealing with the first file in a new folder indentionAmounts.add(indentation); newFolder = false;