diff --git a/src/main/java/de/tum/in/www1/artemis/repository/BuildPlanRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/BuildPlanRepository.java index 9d6113fb08aa..ff1f3c1a4cbd 100644 --- a/src/main/java/de/tum/in/www1/artemis/repository/BuildPlanRepository.java +++ b/src/main/java/de/tum/in/www1/artemis/repository/BuildPlanRepository.java @@ -23,6 +23,15 @@ public interface BuildPlanRepository extends JpaRepository { """) Optional findByProgrammingExercises_IdWithProgrammingExercises(@Param("exerciseId") long exerciseId); + @Query(""" + + SELECT buildPlan + FROM BuildPlan buildPlan + JOIN buildPlan.programmingExercises programmingExercises + WHERE programmingExercises.id = :exerciseId + """) + Optional findByProgrammingExercises_Id(long exerciseId); + default BuildPlan findByProgrammingExercises_IdWithProgrammingExercisesElseThrow(final long exerciseId) { return findByProgrammingExercises_IdWithProgrammingExercises(exerciseId) .orElseThrow(() -> new EntityNotFoundException("Could not find a build plan for exercise " + exerciseId)); diff --git a/src/main/java/de/tum/in/www1/artemis/service/export/ProgrammingExerciseExportService.java b/src/main/java/de/tum/in/www1/artemis/service/export/ProgrammingExerciseExportService.java index 9b418bb6bfbb..118b58d6e2fa 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/export/ProgrammingExerciseExportService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/export/ProgrammingExerciseExportService.java @@ -6,16 +6,12 @@ import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.List; -import java.util.Objects; -import java.util.Optional; +import java.util.*; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executors; import java.util.function.Predicate; @@ -31,10 +27,7 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; -import javax.xml.xpath.XPath; -import javax.xml.xpath.XPathConstants; -import javax.xml.xpath.XPathException; -import javax.xml.xpath.XPathFactory; +import javax.xml.xpath.*; import org.apache.commons.io.FileUtils; import org.eclipse.jgit.api.errors.GitAPIException; @@ -48,8 +41,6 @@ import org.xml.sax.InputSource; import org.xml.sax.SAXException; -import com.google.common.base.Charsets; - import de.tum.in.www1.artemis.domain.*; import de.tum.in.www1.artemis.domain.enumeration.ProgrammingLanguage; import de.tum.in.www1.artemis.domain.enumeration.RepositoryType; @@ -149,10 +140,10 @@ private Path exportProgrammingExerciseMaterialWithStudentReposOptional(Programmi }); // Export the build plan of a programming exercise, if one exists. Only relevant for Gitlab/Jenkins or Gitlab/GitlabCI setups. - var buildPlan = buildPlanRepository.findByProgrammingExercises_IdWithProgrammingExercises(exercise.getId()); + var buildPlan = buildPlanRepository.findByProgrammingExercises_Id(exercise.getId()); if (buildPlan.isPresent()) { Path buildPlanPath = exportDir.orElseThrow().resolve(BUILD_PLAN_FILE_NAME); - FileUtils.writeStringToFile(buildPlanPath.toFile(), buildPlan.orElseThrow().getBuildPlan(), Charsets.UTF_8); + FileUtils.writeStringToFile(buildPlanPath.toFile(), buildPlan.orElseThrow().getBuildPlan(), StandardCharsets.UTF_8); pathsToBeZipped.add(buildPlanPath); } diff --git a/src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseImportFromFileService.java b/src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseImportFromFileService.java index c7d5cb189fd9..876f5d6c6b02 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseImportFromFileService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseImportFromFileService.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; @@ -22,20 +23,18 @@ import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.base.Charsets; import de.tum.in.www1.artemis.domain.*; import de.tum.in.www1.artemis.domain.enumeration.RepositoryType; import de.tum.in.www1.artemis.repository.BuildPlanRepository; import de.tum.in.www1.artemis.service.*; import de.tum.in.www1.artemis.service.connectors.GitService; -import de.tum.in.www1.artemis.service.export.ProgrammingExerciseExportService; import de.tum.in.www1.artemis.web.rest.errors.BadRequestAlertException; @Service public class ProgrammingExerciseImportFromFileService { - private final Logger log = LoggerFactory.getLogger(ProgrammingExerciseExportService.class); + private final Logger log = LoggerFactory.getLogger(ProgrammingExerciseImportFromFileService.class); private final ProgrammingExerciseService programmingExerciseService; @@ -126,7 +125,7 @@ private void importBuildPlanIfExisting(ProgrammingExercise programmingExercise, Path buildPlanPath = importExerciseDir.resolve(BUILD_PLAN_FILE_NAME); if (Files.exists(buildPlanPath)) { try { - buildPlanRepository.setBuildPlanForExercise(FileUtils.readFileToString(buildPlanPath.toFile(), Charsets.UTF_8), programmingExercise); + buildPlanRepository.setBuildPlanForExercise(FileUtils.readFileToString(buildPlanPath.toFile(), StandardCharsets.UTF_8), programmingExercise); } catch (IOException e) { log.warn("Could not read build plan file. Continue importing the exercise but skipping the build plan.", e); diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java index c8f955d45bef..23069ddb8f56 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseBitbucketBambooIntegrationTest.java @@ -349,7 +349,7 @@ void exportInstructorRepositories_forbidden() throws Exception { @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void exportProgrammingExerciseInstructorMaterial() throws Exception { - programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial_shouldReturnFile(true); + programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial_shouldReturnFileWithoutBuildplan(true); // we have a working directory and one directory for each repository verify(fileService, times(4)).scheduleDirectoryPathForRecursiveDeletion(any(Path.class), eq(5L)); verify(fileService).schedulePathForDeletion(any(Path.class), eq(5L)); @@ -435,7 +435,7 @@ void testArchiveCourseWithProgrammingExercise() throws Exception { @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void testExportProgrammingExerciseInstructorMaterial_failToCreateZip() throws Exception { doThrow(IOException.class).when(zipFileService).createZipFile(any(Path.class), any()); - programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial(HttpStatus.INTERNAL_SERVER_ERROR, true, true, true); + programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial(HttpStatus.INTERNAL_SERVER_ERROR, true, true, true, false); } @Test @@ -443,21 +443,21 @@ void testExportProgrammingExerciseInstructorMaterial_failToCreateZip() throws Ex void testExportProgrammingExerciseInstructorMaterial_failToCreateTempDir() throws Exception { try (MockedStatic mockedFiles = mockStatic(Files.class)) { mockedFiles.when(() -> Files.createTempDirectory(any(Path.class), any(String.class))).thenThrow(IOException.class); - programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial(HttpStatus.INTERNAL_SERVER_ERROR, true, false, false); + programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial(HttpStatus.INTERNAL_SERVER_ERROR, true, false, false, false); } } @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void testExportProgrammingExerciseInstructorMaterial_embeddedFilesDontExist() throws Exception { - programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial_shouldReturnFile(false); + programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial_shouldReturnFileWithoutBuildplan(false); } @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void testExportProgrammingExerciseInstructorMaterial_failToExportRepository() throws Exception { doThrow(GitException.class).when(fileService).getTemporaryUniquePathWithoutPathCreation(any(Path.class), anyLong()); - programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial(HttpStatus.INTERNAL_SERVER_ERROR, false, true, true); + programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial(HttpStatus.INTERNAL_SERVER_ERROR, false, true, true, false); } @Test diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java index 85e61e914823..8cf0f9cf13e4 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseGitlabJenkinsIntegrationTest.java @@ -404,7 +404,7 @@ void exportInstructorRepositories_forbidden() throws Exception { @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void exportProgrammingExerciseInstructorMaterial() throws Exception { - programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial_shouldReturnFile(true); + programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial_shouldReturnFileWithBuildplan(); // we have a working directory and one directory for each repository verify(fileService, times(4)).scheduleDirectoryPathForRecursiveDeletion(any(Path.class), eq(5L)); verify(fileService).schedulePathForDeletion(any(Path.class), eq(5L)); @@ -427,14 +427,14 @@ void exportProgrammingExerciseInstructorMaterial_problemStatementShouldContainTe void testExportProgrammingExerciseInstructorMaterial_failToCreateTempDir() throws Exception { try (MockedStatic mockedFiles = mockStatic(Files.class)) { mockedFiles.when(() -> Files.createTempDirectory(any(Path.class), any(String.class))).thenThrow(IOException.class); - programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial(HttpStatus.INTERNAL_SERVER_ERROR, true, false, false); + programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial(HttpStatus.INTERNAL_SERVER_ERROR, true, false, false, false); } } @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void testExportProgrammingExerciseInstructorMaterial_embeddedFilesDontExist() throws Exception { - programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial_shouldReturnFile(false); + programmingExerciseTestService.exportProgrammingExerciseInstructorMaterial_shouldReturnFile(false, false); } @Test diff --git a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java index bedbe2c923ef..98753730ba90 100644 --- a/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java +++ b/src/test/java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java @@ -1332,7 +1332,7 @@ void exportInstructorRepositories_shouldReturnFile() throws Exception { } void exportInstructorAuxiliaryRepository_shouldReturnFile() throws Exception { - generateProgrammingExerciseForExport(false); + generateProgrammingExerciseForExport(); var auxRepo = addAuxiliaryRepositoryToProgrammingExercise(exercise); setupAuxRepoMock(auxRepo); setupRepositoryMocks(exercise); @@ -1348,7 +1348,7 @@ private void setupAuxRepoMock(AuxiliaryRepository auxiliaryRepository) throws Gi } void exportInstructorAuxiliaryRepository_forbidden() throws Exception { - generateProgrammingExerciseForExport(false); + generateProgrammingExerciseForExport(); var auxRepo = addAuxiliaryRepositoryToProgrammingExercise(exercise); var url = "/api/programming-exercises/" + exercise.getId() + "/export-instructor-auxiliary-repository/" + auxRepo.getId(); request.get(url, HttpStatus.FORBIDDEN, String.class); @@ -1365,7 +1365,7 @@ void exportInstructorRepositories_forbidden() throws Exception { } private String exportInstructorRepository(RepositoryType repositoryType, LocalRepository localRepository, HttpStatus expectedStatus) throws Exception { - generateProgrammingExerciseForExport(false); + generateProgrammingExerciseForExport(); setupMockRepo(localRepository, repositoryType, "some-file.java"); @@ -1374,7 +1374,7 @@ private String exportInstructorRepository(RepositoryType repositoryType, LocalRe } private String exportStudentRequestedRepository(HttpStatus expectedStatus, boolean includeTests) throws Exception { - generateProgrammingExerciseForExport(false); + generateProgrammingExerciseForExport(); setupMockRepo(exerciseRepo, RepositoryType.SOLUTION, "some-file.java"); if (includeTests) { @@ -1387,14 +1387,34 @@ private String exportStudentRequestedRepository(HttpStatus expectedStatus, boole // Test + /** + * Test that the export of the instructor material works as expected when no build plan exists (e.g. for bamboo setups at the moment) + *

+ * + * @param saveEmbeddedFiles whether embedded files should be saved or not, not saving them simulates that embedded files are no longer stored on the file system + * @throws Exception if the export fails + */ + void exportProgrammingExerciseInstructorMaterial_shouldReturnFileWithoutBuildplan(boolean saveEmbeddedFiles) throws Exception { + exportProgrammingExerciseInstructorMaterial_shouldReturnFile(saveEmbeddedFiles, false); + } + + /** + * Test that the export of the instructor material works as expected with a build plan included (relevant for Gitlab/Jenkins setups). + * + * @throws Exception if the export fails + */ + void exportProgrammingExerciseInstructorMaterial_shouldReturnFileWithBuildplan() throws Exception { + exportProgrammingExerciseInstructorMaterial_shouldReturnFile(false, true); + } + /** * Test that the export of the instructor material works as expected. * * @param saveEmbeddedFiles whether embedded files should be saved or not, not saving them simulates that embedded files are no longer stored on the file system * @throws Exception if the export fails */ - void exportProgrammingExerciseInstructorMaterial_shouldReturnFile(boolean saveEmbeddedFiles) throws Exception { - var zipFile = exportProgrammingExerciseInstructorMaterial(HttpStatus.OK, false, true, saveEmbeddedFiles); + void exportProgrammingExerciseInstructorMaterial_shouldReturnFile(boolean saveEmbeddedFiles, boolean shouldIncludeBuildplan) throws Exception { + var zipFile = exportProgrammingExerciseInstructorMaterial(HttpStatus.OK, false, true, saveEmbeddedFiles, shouldIncludeBuildplan); // Assure, that the zip folder is already created and not 'in creation' which would lead to a failure when extracting it in the next step await().until(zipFile::exists); assertThat(zipFile).isNotNull(); @@ -1418,15 +1438,20 @@ void exportProgrammingExerciseInstructorMaterial_shouldReturnFile(boolean saveEm assertThat(listOfIncludedFiles).anyMatch((filename) -> filename.toString().matches(".*-exercise.zip")) .anyMatch((filename) -> filename.toString().matches(".*-solution.zip")).anyMatch((filename) -> filename.toString().matches(".*-tests.zip")) .anyMatch((filename) -> filename.toString().matches(EXPORTED_EXERCISE_PROBLEM_STATEMENT_FILE_PREFIX + ".*.md")) - .anyMatch((filename) -> filename.toString().matches(EXPORTED_EXERCISE_DETAILS_FILE_PREFIX + ".*.json")) - .anyMatch((filename) -> BUILD_PLAN_FILE_NAME.equals(filename.toString())); + .anyMatch((filename) -> filename.toString().matches(EXPORTED_EXERCISE_DETAILS_FILE_PREFIX + ".*.json")); if (saveEmbeddedFiles) { assertThat(listOfIncludedFiles).anyMatch((filename) -> filename.toString().equals(embeddedFileName1)) .anyMatch((filename) -> filename.toString().equals(embeddedFileName2)); } - Path buildPlanPath = listOfIncludedFiles.stream().filter(file -> BUILD_PLAN_FILE_NAME.equals(file.getFileName().toString())).findFirst().orElseThrow(); - String buildPlanContent = Files.readString(extractedZipDir.resolve(buildPlanPath), StandardCharsets.UTF_8); - assertThat(buildPlanContent).isEqualTo("my build plan"); + if (shouldIncludeBuildplan) { + assertThat(listOfIncludedFiles).anyMatch((filename) -> BUILD_PLAN_FILE_NAME.equals(filename.toString())); + Path buildPlanPath = listOfIncludedFiles.stream().filter(file -> BUILD_PLAN_FILE_NAME.equals(file.getFileName().toString())).findFirst().orElseThrow(); + String buildPlanContent = Files.readString(extractedZipDir.resolve(buildPlanPath), StandardCharsets.UTF_8); + assertThat(buildPlanContent).isEqualTo("my build plan"); + } + else { + assertThat(listOfIncludedFiles).noneMatch((filename) -> BUILD_PLAN_FILE_NAME.equals(filename.toString())); + } } FileUtils.deleteDirectory(extractedZipDir.toFile()); @@ -1434,7 +1459,7 @@ void exportProgrammingExerciseInstructorMaterial_shouldReturnFile(boolean saveEm } void exportProgrammingExerciseInstructorMaterial_problemStatementNull_success() throws Exception { - var zipFile = exportProgrammingExerciseInstructorMaterial(HttpStatus.OK, true, true, false); + var zipFile = exportProgrammingExerciseInstructorMaterial(HttpStatus.OK, true, true, false, false); await().until(zipFile::exists); assertThat(zipFile).isNotNull(); Path extractedZipDir = zipFileTestUtilService.extractZipFileRecursively(zipFile.getAbsolutePath()); @@ -1483,25 +1508,27 @@ void exportProgrammingExerciseInstructorMaterial_forbidden() throws Exception { // change the group name to enforce a HttpStatus forbidden after having accessed the endpoint course.setInstructorGroupName("test"); courseRepository.save(course); - exportProgrammingExerciseInstructorMaterial(HttpStatus.FORBIDDEN, false, false, false); + exportProgrammingExerciseInstructorMaterial(HttpStatus.FORBIDDEN, false, false, false, false); } /** * export programming exercise instructor material * - * @param expectedStatus the expected http status, e.g. 200 OK - * @param problemStatementNull whether the problem statement should be null or not - * @param mockRepos whether the repos should be mocked or not, if we mock the files API we cannot mock them but also cannot use them - * @param saveEmbeddedFiles whether embedded files should be saved or not, not saving them simulates that embedded files are no longer stored on the file system + * @param expectedStatus the expected http status, e.g. 200 OK + * @param problemStatementNull whether the problem statement should be null or not + * @param mockRepos whether the repos should be mocked or not, if we mock the files API we cannot mock them but also cannot use them + * @param saveEmbeddedFiles whether embedded files should be saved or not, not saving them simulates that embedded files are no longer stored on the file system + * @param shouldIncludeBuildplan whether the build plan should be included in the export or not * @return the zip file * @throws Exception if the export fails */ - File exportProgrammingExerciseInstructorMaterial(HttpStatus expectedStatus, boolean problemStatementNull, boolean mockRepos, boolean saveEmbeddedFiles) throws Exception { + File exportProgrammingExerciseInstructorMaterial(HttpStatus expectedStatus, boolean problemStatementNull, boolean mockRepos, boolean saveEmbeddedFiles, + boolean shouldIncludeBuildplan) throws Exception { if (problemStatementNull) { generateProgrammingExerciseWithProblemStatementNullForExport(); } else { - generateProgrammingExerciseForExport(saveEmbeddedFiles); + generateProgrammingExerciseForExport(saveEmbeddedFiles, shouldIncludeBuildplan); } return exportProgrammingExerciseInstructorMaterial(expectedStatus, mockRepos); } @@ -1536,7 +1563,11 @@ private void generateProgrammingExerciseWithProblemStatementNullForExport() { exercise = programmingExerciseRepository.findWithTemplateAndSolutionParticipationById(exercise.getId()).orElseThrow(); } - private void generateProgrammingExerciseForExport(boolean saveEmbeddedFiles) throws IOException { + private void generateProgrammingExerciseForExport() throws IOException { + generateProgrammingExerciseForExport(false, false); + } + + private void generateProgrammingExerciseForExport(boolean saveEmbeddedFiles, boolean shouldIncludeBuildPlan) throws IOException { String embeddedFileName1 = "Markdown_2023-05-06T16-17-46-410_ad323711.jpg"; String embeddedFileName2 = "Markdown_2023-05-06T16-17-46-822_b921f475.jpg"; exercise.setProblemStatement(String.format(""" @@ -1551,7 +1582,9 @@ private void generateProgrammingExerciseForExport(boolean saveEmbeddedFiles) thr FilePathService.getMarkdownFilePath().resolve(embeddedFileName2).toFile()); } exercise = programmingExerciseRepository.save(exercise); - buildPlanRepository.setBuildPlanForExercise("my build plan", exercise); + if (shouldIncludeBuildPlan) { + buildPlanRepository.setBuildPlanForExercise("my build plan", exercise); + } exercise = programmingExerciseUtilService.addTemplateParticipationForProgrammingExercise(exercise); exercise = programmingExerciseUtilService.addSolutionParticipationForProgrammingExercise(exercise); exercise = programmingExerciseRepository.findWithTemplateAndSolutionParticipationById(exercise.getId()).orElseThrow();