Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Programming exercises: Changing the short name while importing breaks tests #10470

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from
Draft
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;

import org.apache.commons.io.FileUtils;
Expand Down Expand Up @@ -54,6 +53,8 @@ public class ProgrammingExerciseImportFromFileService {

private final StaticCodeAnalysisService staticCodeAnalysisService;

private final ProgrammingExerciseRepositoryService programmingExerciseRepositoryService;

private final RepositoryService repositoryService;

private final GitService gitService;
Expand All @@ -64,14 +65,13 @@ public class ProgrammingExerciseImportFromFileService {

private final BuildPlanRepository buildPlanRepository;

private static final List<String> SHORT_NAME_REPLACEMENT_EXCLUSIONS = List.of("gradle-wrapper.jar");

public ProgrammingExerciseImportFromFileService(ProgrammingExerciseService programmingExerciseService, ZipFileService zipFileService,
StaticCodeAnalysisService staticCodeAnalysisService, RepositoryService repositoryService, GitService gitService, FileService fileService, ProfileService profileService,
BuildPlanRepository buildPlanRepository) {
StaticCodeAnalysisService staticCodeAnalysisService, ProgrammingExerciseRepositoryService programmingExerciseRepositoryService, RepositoryService repositoryService,
GitService gitService, FileService fileService, ProfileService profileService, BuildPlanRepository buildPlanRepository) {
this.programmingExerciseService = programmingExerciseService;
this.zipFileService = zipFileService;
this.staticCodeAnalysisService = staticCodeAnalysisService;
this.programmingExerciseRepositoryService = programmingExerciseRepositoryService;
this.repositoryService = repositoryService;
this.gitService = gitService;
this.fileService = fileService;
Expand Down Expand Up @@ -104,7 +104,6 @@ public ProgrammingExercise importProgrammingExerciseFromFile(ProgrammingExercise
zipFile.transferTo(exerciseFilePath);
zipFileService.extractZipFileRecursively(exerciseFilePath);
checkRepositoriesExist(importExerciseDir);
var oldShortName = getProgrammingExerciseFromDetailsFile(importExerciseDir).getShortName();
programmingExerciseService.validateNewProgrammingExerciseSettings(originalProgrammingExercise, course);
// TODO: creating the whole exercise (from template) is a bad solution in this case, we do not want the template content, instead we want the file content of the zip
newProgrammingExercise = programmingExerciseService.createProgrammingExercise(originalProgrammingExercise, true);
Expand All @@ -113,7 +112,15 @@ public ProgrammingExercise importProgrammingExerciseFromFile(ProgrammingExercise
}
Path pathToDirectoryWithImportedContent = exerciseFilePath.toAbsolutePath().getParent().resolve(FilenameUtils.getBaseName(exerciseFilePath.toString()));
copyEmbeddedFiles(pathToDirectoryWithImportedContent);
importRepositoriesFromFile(newProgrammingExercise, importExerciseDir, oldShortName, user);
importRepositoriesFromFile(newProgrammingExercise, importExerciseDir, user);

try {
programmingExerciseRepositoryService.adjustProjectNames(getProgrammingExerciseFromDetailsFile(importExerciseDir).getTitle(), newProgrammingExercise);
}
catch (GitAPIException | IOException e) {
log.error("Error during adjustment of placeholders of ProgrammingExercise {}", newProgrammingExercise.getTitle(), e);
}

newProgrammingExercise.setCourse(course);
// It doesn't make sense to import a build plan on a local CI setup.
if (profileService.isGitlabCiOrJenkinsActive()) {
Expand Down Expand Up @@ -168,8 +175,14 @@ private void copyEmbeddedFiles(Path importExerciseDir) throws IOException {
}
}

private void importRepositoriesFromFile(ProgrammingExercise newExercise, Path basePath, String oldExerciseShortName, User user)
throws IOException, GitAPIException, URISyntaxException {
/**
* Imports the repositories from the extracted zip file.
*
* @param newExercise the new programming exercise to which the repositories should be imported
* @param basePath the path to the extracted zip file
* @param user the user performing the import
*/
private void importRepositoriesFromFile(ProgrammingExercise newExercise, Path basePath, User user) throws IOException, GitAPIException, URISyntaxException {
Repository templateRepo = gitService.getOrCheckoutRepository(new VcsRepositoryUri(newExercise.getTemplateRepositoryUri()), false);
Repository solutionRepo = gitService.getOrCheckoutRepository(new VcsRepositoryUri(newExercise.getSolutionRepositoryUri()), false);
Repository testRepo = gitService.getOrCheckoutRepository(new VcsRepositoryUri(newExercise.getTestRepositoryUri()), false);
Expand All @@ -179,8 +192,6 @@ private void importRepositoriesFromFile(ProgrammingExercise newExercise, Path ba
}

copyImportedExerciseContentToRepositories(templateRepo, solutionRepo, testRepo, auxiliaryRepositories, basePath);
replaceImportedExerciseShortName(Map.of(oldExerciseShortName, newExercise.getShortName()), List.of(solutionRepo, templateRepo, testRepo));
replaceImportedExerciseShortName(Map.of(oldExerciseShortName, newExercise.getShortName()), auxiliaryRepositories);

gitService.stageAllChanges(templateRepo);
gitService.stageAllChanges(solutionRepo);
Expand All @@ -198,12 +209,6 @@ private void importRepositoriesFromFile(ProgrammingExercise newExercise, Path ba

}

private void replaceImportedExerciseShortName(Map<String, String> replacements, List<Repository> repositories) throws IOException {
for (Repository repository : repositories) {
fileService.replaceVariablesInFileRecursive(repository.getLocalPath(), replacements, SHORT_NAME_REPLACEMENT_EXCLUSIONS);
}
}

private void copyImportedExerciseContentToRepositories(Repository templateRepo, Repository solutionRepo, Repository testRepo, List<Repository> auxiliaryRepositories,
Path basePath) throws IOException {
repositoryService.deleteAllContentInRepository(templateRepo);
Expand All @@ -229,7 +234,7 @@ private void copyImportedExerciseContentToRepositories(Repository templateRepo,
* @param repository the repository to which the content should be copied
* @param repoName the name of the repository
* @param basePath the path to the extracted zip file
**/
*/
private void copyExerciseContentToRepository(Repository repository, String repoName, Path basePath) throws IOException {
// @formatter:off
FileUtils.copyDirectory(
Expand All @@ -244,6 +249,13 @@ private void copyExerciseContentToRepository(Repository repository, String repoN
}
}

/**
* Reads the programming exercise details from the JSON file in the extracted zip path.
*
* @param extractedZipPath the path to the extracted zip file containing the exercise details
* @return the programming exercise object deserialized from the JSON file
* @throws IOException if there is an error reading the file
*/
private ProgrammingExercise getProgrammingExerciseFromDetailsFile(Path extractedZipPath) throws IOException {
var exerciseJsonPath = retrieveExerciseJsonPath(extractedZipPath);
ObjectMapper objectMapper = new ObjectMapper();
Expand Down Expand Up @@ -289,6 +301,13 @@ private Path retrieveRepositoryDirectoryPath(Path dirPath, String repoName) {
return result.getFirst();
}

/**
* Retrieves the path to the Exercise-Details.json file in the extracted zip path.
*
* @param dirPath the path to the extracted zip file containing the exercise details
* @return the path to the Exercise-Details.json file
* @throws IOException if there is an error reading the file
*/
private Path retrieveExerciseJsonPath(Path dirPath) throws IOException {
List<Path> result;
try (Stream<Path> stream = Files.walk(dirPath)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@
import static de.tum.cit.aet.artemis.core.config.Constants.TEST_REPO_NAME;

import java.io.IOException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

Expand All @@ -21,13 +19,9 @@
import com.fasterxml.jackson.core.JsonProcessingException;

import de.tum.cit.aet.artemis.assessment.domain.Visibility;
import de.tum.cit.aet.artemis.core.domain.User;
import de.tum.cit.aet.artemis.core.repository.UserRepository;
import de.tum.cit.aet.artemis.core.service.FileService;
import de.tum.cit.aet.artemis.programming.domain.AuxiliaryRepository;
import de.tum.cit.aet.artemis.programming.domain.ProgrammingExercise;
import de.tum.cit.aet.artemis.programming.domain.ProgrammingExerciseTestCase;
import de.tum.cit.aet.artemis.programming.domain.Repository;
import de.tum.cit.aet.artemis.programming.domain.RepositoryType;
import de.tum.cit.aet.artemis.programming.domain.VcsRepositoryUri;
import de.tum.cit.aet.artemis.programming.domain.build.BuildPlanType;
Expand All @@ -49,16 +43,12 @@ public class ProgrammingExerciseImportService {

private final Optional<ContinuousIntegrationTriggerService> continuousIntegrationTriggerService;

private final ProgrammingExerciseRepositoryService programmingExerciseRepositoryService;

private final ProgrammingExerciseService programmingExerciseService;

private final ProgrammingExerciseTaskService programmingExerciseTaskService;

private final GitService gitService;

private final FileService fileService;

private final UserRepository userRepository;

private final AuxiliaryRepositoryRepository auxiliaryRepositoryRepository;

private final UriService uriService;
Expand All @@ -70,18 +60,16 @@ public class ProgrammingExerciseImportService {
private final ProgrammingExerciseTestCaseRepository programmingExerciseTestCaseRepository;

public ProgrammingExerciseImportService(Optional<VersionControlService> versionControlService, Optional<ContinuousIntegrationService> continuousIntegrationService,
Optional<ContinuousIntegrationTriggerService> continuousIntegrationTriggerService, ProgrammingExerciseService programmingExerciseService,
ProgrammingExerciseTaskService programmingExerciseTaskService, GitService gitService, FileService fileService, UserRepository userRepository,
Optional<ContinuousIntegrationTriggerService> continuousIntegrationTriggerService, ProgrammingExerciseRepositoryService programmingExerciseRepositoryService,
ProgrammingExerciseService programmingExerciseService, ProgrammingExerciseTaskService programmingExerciseTaskService,
AuxiliaryRepositoryRepository auxiliaryRepositoryRepository, UriService uriService, TemplateUpgradePolicyService templateUpgradePolicyService,
ProgrammingExerciseImportBasicService programmingExerciseImportBasicService, ProgrammingExerciseTestCaseRepository programmingExerciseTestCaseRepository) {
this.versionControlService = versionControlService;
this.continuousIntegrationService = continuousIntegrationService;
this.continuousIntegrationTriggerService = continuousIntegrationTriggerService;
this.programmingExerciseRepositoryService = programmingExerciseRepositoryService;
this.programmingExerciseService = programmingExerciseService;
this.programmingExerciseTaskService = programmingExerciseTaskService;
this.gitService = gitService;
this.fileService = fileService;
this.userRepository = userRepository;
this.auxiliaryRepositoryRepository = auxiliaryRepositoryRepository;
this.uriService = uriService;
this.templateUpgradePolicyService = templateUpgradePolicyService;
Expand Down Expand Up @@ -134,7 +122,7 @@ public void importRepositories(final ProgrammingExercise templateExercise, final

try {
// Adjust placeholders that were replaced during creation of template exercise
adjustProjectNames(templateExercise, newExercise);
programmingExerciseRepositoryService.adjustProjectNames(templateExercise.getTitle(), newExercise);
}
catch (GitAPIException | IOException e) {
log.error("Error during adjustment of placeholders of ProgrammingExercise {}", newExercise.getTitle(), e);
Expand Down Expand Up @@ -220,58 +208,6 @@ private void cloneAndEnableAllBuildPlans(ProgrammingExercise templateExercise, P
continuousIntegration.enablePlan(targetExerciseProjectKey, solutionParticipation.getBuildPlanId());
}

/**
* Adjust project names in imported exercise for TEST, BASE and SOLUTION repositories.
* Replace values inserted in {@link ProgrammingExerciseRepositoryService#replacePlaceholders(ProgrammingExercise, Repository)}.
*
* @param templateExercise the exercise from which the values that should be replaced are extracted
* @param newExercise the exercise from which the values that should be inserted are extracted
* @throws GitAPIException If the checkout/push of one repository fails
* @throws IOException If the values in the files could not be replaced
*/
private void adjustProjectNames(ProgrammingExercise templateExercise, ProgrammingExercise newExercise) throws GitAPIException, IOException {
final var projectKey = newExercise.getProjectKey();

Map<String, String> replacements = new HashMap<>();

// Used in pom.xml
replacements.put("<artifactId>" + templateExercise.getTitle().replaceAll(" ", "-"), "<artifactId>" + newExercise.getTitle().replaceAll(" ", "-"));

// Used in settings.gradle
replacements.put("rootProject.name = '" + templateExercise.getTitle().replaceAll(" ", "-"), "rootProject.name = '" + newExercise.getTitle().replaceAll(" ", "-"));

// Used in readme.md (Gradle)
replacements.put("testImplementation(':" + templateExercise.getTitle().replaceAll(" ", "-"), "testImplementation(':" + newExercise.getTitle().replaceAll(" ", "-"));

// Used in .project
replacements.put("<name>" + templateExercise.getTitle(), "<name>" + newExercise.getTitle());

final var user = userRepository.getUser();

adjustProjectName(replacements, projectKey, newExercise.generateRepositoryName(RepositoryType.TEMPLATE), user);
adjustProjectName(replacements, projectKey, newExercise.generateRepositoryName(RepositoryType.TESTS), user);
adjustProjectName(replacements, projectKey, newExercise.generateRepositoryName(RepositoryType.SOLUTION), user);
}

/**
* Adjust project names in imported exercise for specific repository.
* Replace values inserted in {@link ProgrammingExerciseRepositoryService#replacePlaceholders(ProgrammingExercise, Repository)}.
*
* @param replacements the replacements that should be applied
* @param projectKey the project key of the new exercise
* @param repositoryName the name of the repository that should be adjusted
* @param user the user which performed the action (used as Git author)
* @throws GitAPIException If the checkout/push of one repository fails
*/
private void adjustProjectName(Map<String, String> replacements, String projectKey, String repositoryName, User user) throws GitAPIException, IOException {
final var repositoryUri = versionControlService.orElseThrow().getCloneRepositoryUri(projectKey, repositoryName);
Repository repository = gitService.getOrCheckoutRepository(repositoryUri, true);
fileService.replaceVariablesInFileRecursive(repository.getLocalPath().toAbsolutePath(), replacements, List.of("gradle-wrapper.jar"));
gitService.stageAllChanges(repository);
gitService.commitAndPush(repository, "Template adjusted by Artemis", true, user);
repository.setFiles(null); // Clear cache to avoid multiple commits when Artemis server is not restarted between attempts
}

/**
* Method to import a programming exercise, including all base build plans (template, solution) and repositories (template, solution, test).
* Referenced entities, s.a. the test cases or the hints will get cloned and assigned a new id.
Expand Down
Loading
Loading