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

Integrated code lifecycle: Simplify repository handling for commit history #8546

Merged
merged 55 commits into from
May 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
a24337c
simplify participation and repository handling
May 5, 2024
4200eae
improve and simplify client code logic
May 5, 2024
2516169
use bare repository when local VC is active to avoid expensive clone …
May 5, 2024
2d44fc9
fix issues with template, test and solution repos
May 5, 2024
033460d
move code into GitService and RepositoryService
May 5, 2024
85ed36d
remove file cache in repository
May 6, 2024
6b6f1db
Force update branch/refs when setting up the repo (attempts to fix pr…
pzdr7 May 7, 2024
cf20c0b
Add explanatory comment
pzdr7 May 8, 2024
5c5d45c
Provide parameter for local-vcs-repo-path (fixes initialization of Gi…
pzdr7 May 8, 2024
868f27e
Add default value null for local-vcs-repo-path
pzdr7 May 8, 2024
83768bf
Fix typo
pzdr7 May 8, 2024
f969896
Fix URLs in RepositoryIntegrationTest
pzdr7 May 8, 2024
07a366a
(WIP) Define service method for retrieving the files at a commit
pzdr7 May 8, 2024
b9eeee2
Fix URLs in ProgrammingExerciseGitDiffReportIntegrationTest
pzdr7 May 8, 2024
ff7a830
(WIP) Replace forwards with service calls
pzdr7 May 8, 2024
5d5a26d
Remove unused profile service
pzdr7 May 8, 2024
0d02c0f
Add docs for service method
pzdr7 May 8, 2024
f1c02bb
Require participation instead of id (forces existence check); change …
pzdr7 May 8, 2024
c11a7d5
(experimental) Push participation to remote; mock getBareRepository()
pzdr7 May 8, 2024
e0d0fee
(experimental) Initialize remotes as bare in the tests where possible
pzdr7 May 8, 2024
a77bbe7
(experimental) Set non-bare where remote needs to be modified directly
pzdr7 May 8, 2024
7311428
Remove unused subscription
pzdr7 May 8, 2024
21567d1
Fix diff-report URL
pzdr7 May 8, 2024
b216ee5
Fix files-content-commit-details url
pzdr7 May 8, 2024
63ed8e4
Fix server style
pzdr7 May 8, 2024
42707f4
Merge branch 'develop' into chore/simplify-repository-handling-commit…
pzdr7 May 8, 2024
710c9cb
Merge branch 'develop' into 'chore/simplify-repository-handling-commi…
pzdr7 May 10, 2024
999e400
Merge branch 'develop' into chore/simplify-repository-handling-commit…
pzdr7 May 12, 2024
14dcef8
Merge branch 'develop' into chore/simplify-repository-handling-commit…
pzdr7 May 14, 2024
9019bfc
Fix architecture test
pzdr7 May 14, 2024
4a18de1
Remove unused profileService
pzdr7 May 14, 2024
eb4319d
Test error handling of CommitDetailsView
pzdr7 May 14, 2024
36733b3
Revise TODO regarding binary files
pzdr7 May 15, 2024
e4fd1c9
Merge branch 'develop' into chore/simplify-repository-handling-commit…
pzdr7 May 16, 2024
7e5feb9
Add TODOs regarding ModelAndView
pzdr7 May 16, 2024
9f42605
Merge branch 'develop' into chore/simplify-repository-handling-commit…
pzdr7 May 16, 2024
dcdf6d6
Remove unused participationRepository
pzdr7 May 16, 2024
4ff721c
Add additional error log
pzdr7 May 16, 2024
6d32934
Merge branch 'develop' into chore/simplify-repository-handling-commit…
pzdr7 May 17, 2024
c7a5ae4
Move cached repo retrieval out of try
pzdr7 May 17, 2024
dce0e6e
Replace Objects.equals with ==
pzdr7 May 17, 2024
c4c740b
Remove unnecessary null check
pzdr7 May 17, 2024
cc41e93
Remove unnecessary comment
pzdr7 May 17, 2024
88e64c5
Change parameter order
pzdr7 May 17, 2024
da7606b
Reverse order of access check vs. vcs uri retrieval
pzdr7 May 17, 2024
43be6f3
Change order of parameters
pzdr7 May 17, 2024
891c4f2
Remove unused parameter
pzdr7 May 17, 2024
941361f
Change parameter order
pzdr7 May 17, 2024
f3c6d1d
Simplify undefined checks
pzdr7 May 17, 2024
bd0d252
Remove LocalVC base dir retrieval from environment
pzdr7 May 17, 2024
63f4b51
Change permissions; make diff report available to TAs
pzdr7 May 17, 2024
5be6bb3
Move and rename method
pzdr7 May 17, 2024
94753e8
Rework logging
pzdr7 May 17, 2024
708f457
Merge branch 'develop' into chore/simplify-repository-handling-commit…
pzdr7 May 17, 2024
703f6b8
Merge branch 'develop' into chore/simplify-repository-handling-commit…
pzdr7 May 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions src/main/java/de/tum/in/www1/artemis/domain/Repository.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import java.io.IOException;
import java.nio.file.Path;
import java.util.Collection;
import java.util.Map;

import org.eclipse.jgit.lib.BaseRepositoryBuilder;

Expand All @@ -25,8 +24,6 @@ public class Repository extends org.eclipse.jgit.internal.storage.file.FileRepos

private final VcsRepositoryUri remoteRepositoryUri;

private Map<File, FileType> filesAndFolders;

private Collection<File> files;

public Repository(File gitDir, VcsRepositoryUri remoteRepositoryUri) throws IOException {
Expand Down Expand Up @@ -77,14 +74,6 @@ public Path getLocalPath() {
return localPath;
}

public Map<File, FileType> getContent() {
return filesAndFolders;
}

public void setContent(Map<File, FileType> filesAndFolders) {
this.filesAndFolders = filesAndFolders;
}

public Collection<File> getFiles() {
return files;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ default ProgrammingExercise findByIdWithSubmissionPolicyElseThrow(long programmi
* @throws EntityNotFoundException the programming exercise could not be found.
*/
@NotNull
// TODO: rename, this method does more than it promises
default ProgrammingExercise findByIdWithTemplateAndSolutionParticipationElseThrow(long programmingExerciseId) throws EntityNotFoundException {
Optional<ProgrammingExercise> programmingExercise = findWithTemplateAndSolutionParticipationTeamAssignmentConfigCategoriesById(programmingExerciseId);
return programmingExercise.orElseThrow(() -> new EntityNotFoundException("Programming Exercise", programmingExerciseId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,15 +273,8 @@ public void filterSensitiveInformationIfNecessary(final Participation participat
*/
public void filterSensitiveInformationIfNecessary(final Participation participation, final Collection<Result> results, Optional<User> user) {
results.forEach(Result::filterSensitiveInformation);
if (user.isPresent()) {
if (!authCheckService.isAtLeastTeachingAssistantForExercise(participation.getExercise(), user.get())) {
filterInformation(participation, results);
}
}
else {
if (!authCheckService.isAtLeastTeachingAssistantForExercise(participation.getExercise())) {
filterInformation(participation, results);
}
if (!authCheckService.isAtLeastTeachingAssistantForExercise(participation.getExercise(), user.orElse(null))) {
filterInformation(participation, results);
}
}

Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@
import de.tum.in.www1.artemis.repository.UserRepository;
import de.tum.in.www1.artemis.security.SecurityUtils;
import de.tum.in.www1.artemis.service.AuthorizationCheckService;
import de.tum.in.www1.artemis.service.RepositoryAccessService;
import de.tum.in.www1.artemis.service.connectors.ci.ContinuousIntegrationTriggerService;
import de.tum.in.www1.artemis.service.programming.AuxiliaryRepositoryService;
import de.tum.in.www1.artemis.service.programming.ProgrammingExerciseParticipationService;
import de.tum.in.www1.artemis.service.programming.ProgrammingMessagingService;
import de.tum.in.www1.artemis.service.programming.ProgrammingSubmissionService;
import de.tum.in.www1.artemis.service.programming.ProgrammingTriggerService;
import de.tum.in.www1.artemis.service.programming.RepositoryAccessService;
import de.tum.in.www1.artemis.service.util.TimeLogUtil;
import de.tum.in.www1.artemis.web.rest.errors.AccessForbiddenException;
import de.tum.in.www1.artemis.web.rest.errors.EntityNotFoundException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import de.tum.in.www1.artemis.domain.iris.message.IrisMessage;
import de.tum.in.www1.artemis.domain.iris.message.IrisTextMessageContent;
import de.tum.in.www1.artemis.domain.participation.ProgrammingExerciseParticipation;
import de.tum.in.www1.artemis.service.RepositoryService;
import de.tum.in.www1.artemis.service.connectors.GitService;
import de.tum.in.www1.artemis.service.connectors.pyris.dto.data.PyrisBuildLogEntryDTO;
import de.tum.in.www1.artemis.service.connectors.pyris.dto.data.PyrisFeedbackDTO;
Expand All @@ -33,6 +32,7 @@
import de.tum.in.www1.artemis.service.connectors.pyris.dto.data.PyrisResultDTO;
import de.tum.in.www1.artemis.service.connectors.pyris.dto.data.PyrisSubmissionDTO;
import de.tum.in.www1.artemis.service.connectors.pyris.dto.data.PyrisTextMessageContentDTO;
import de.tum.in.www1.artemis.service.programming.RepositoryService;

@Service
@Profile("iris")
Expand All @@ -57,16 +57,16 @@ public PyrisDTOService(GitService gitService, RepositoryService repositoryServic
* @return the converted PyrisProgrammingExerciseDTO
*/
public PyrisProgrammingExerciseDTO toPyrisDTO(ProgrammingExercise exercise) {
var templateRepositoryContents = getRepository(exercise.getTemplateParticipation()).map(repositoryService::getFilesWithContent).orElse(Map.of());
var solutionRepositoryContents = getRepository(exercise.getSolutionParticipation()).map(repositoryService::getFilesWithContent).orElse(Map.of());
var templateRepositoryContents = getRepository(exercise.getTemplateParticipation()).map(repositoryService::getFilesContentFromWorkingCopy).orElse(Map.of());
var solutionRepositoryContents = getRepository(exercise.getSolutionParticipation()).map(repositoryService::getFilesContentFromWorkingCopy).orElse(Map.of());
Optional<Repository> testRepo = Optional.empty();
try {
testRepo = Optional.ofNullable(gitService.getOrCheckoutRepository(exercise.getVcsTestRepositoryUri(), true));
}
catch (GitAPIException e) {
log.error("Could not fetch existing test repository", e);
}
var testsRepositoryContents = testRepo.map(repositoryService::getFilesWithContent).orElse(Map.of());
var testsRepositoryContents = testRepo.map(repositoryService::getFilesContentFromWorkingCopy).orElse(Map.of());

return new PyrisProgrammingExerciseDTO(exercise.getId(), exercise.getTitle(), exercise.getProgrammingLanguage(), templateRepositoryContents, solutionRepositoryContents,
testsRepositoryContents, exercise.getProblemStatement(), toInstant(exercise.getReleaseDate()), toInstant(exercise.getDueDate()));
Expand All @@ -82,7 +82,7 @@ public PyrisProgrammingExerciseDTO toPyrisDTO(ProgrammingExercise exercise) {
public PyrisSubmissionDTO toPyrisDTO(ProgrammingSubmission submission) {
var buildLogEntries = submission.getBuildLogEntries().stream().map(buildLogEntry -> new PyrisBuildLogEntryDTO(toInstant(buildLogEntry.getTime()), buildLogEntry.getLog()))
.toList();
var studentRepositoryContents = getRepository((ProgrammingExerciseParticipation) submission.getParticipation()).map(repositoryService::getFilesWithContent)
var studentRepositoryContents = getRepository((ProgrammingExerciseParticipation) submission.getParticipation()).map(repositoryService::getFilesContentFromWorkingCopy)
.orElse(Map.of());
return new PyrisSubmissionDTO(submission.getId(), toInstant(submission.getSubmissionDate()), studentRepositoryContents, submission.getParticipation().isPracticeMode(),
submission.isBuildFailed(), buildLogEntries, getLatestResult(submission));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
import de.tum.in.www1.artemis.repository.hestia.CoverageFileReportRepository;
import de.tum.in.www1.artemis.repository.hestia.CoverageReportRepository;
import de.tum.in.www1.artemis.repository.hestia.TestwiseCoverageReportEntryRepository;
import de.tum.in.www1.artemis.service.RepositoryService;
import de.tum.in.www1.artemis.service.connectors.GitService;
import de.tum.in.www1.artemis.service.connectors.ci.notification.dto.TestwiseCoverageReportDTO;
import de.tum.in.www1.artemis.service.programming.RepositoryService;
import de.tum.in.www1.artemis.web.rest.errors.InternalServerErrorException;

/**
Expand Down Expand Up @@ -225,7 +225,7 @@ private Map<String, Integer> getLineCountByFilePath(ProgrammingSubmission submis
var solutionRepo = gitService.getOrCheckoutRepository(solutionParticipation.getVcsRepositoryUri(), true);
gitService.resetToOriginHead(solutionRepo);
gitService.pullIgnoreConflicts(solutionRepo);
var solutionFiles = repositoryService.getFilesWithContent(solutionRepo);
var solutionFiles = repositoryService.getFilesContentFromWorkingCopy(solutionRepo);
var result = new HashMap<String, Integer>();
solutionFiles.forEach((filePath, value) -> {
// do not count lines for non-java/kotlin files
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import de.tum.in.www1.artemis.repository.ProgrammingExerciseTestCaseRepository;
import de.tum.in.www1.artemis.repository.SolutionProgrammingExerciseParticipationRepository;
import de.tum.in.www1.artemis.repository.hestia.ProgrammingExerciseSolutionEntryRepository;
import de.tum.in.www1.artemis.service.RepositoryService;
import de.tum.in.www1.artemis.service.connectors.GitService;
import de.tum.in.www1.artemis.service.hestia.ProgrammingExerciseGitDiffReportService;
import de.tum.in.www1.artemis.service.hestia.TestwiseCoverageService;
Expand All @@ -34,6 +33,7 @@
import de.tum.in.www1.artemis.service.hestia.behavioral.knowledgesource.FindCommonLines;
import de.tum.in.www1.artemis.service.hestia.behavioral.knowledgesource.GroupGitDiffAndCoverageEntriesByFilePathAndTestCase;
import de.tum.in.www1.artemis.service.hestia.behavioral.knowledgesource.InsertFileContents;
import de.tum.in.www1.artemis.service.programming.RepositoryService;

/**
* Service for handling Solution Entries of behavioral Test Cases.
Expand Down Expand Up @@ -192,7 +192,7 @@ private Map<String, String> readSolutionRepo(ProgrammingExercise programmingExer
gitService.resetToOriginHead(solutionRepo);
gitService.pullIgnoreConflicts(solutionRepo);

return repositoryService.getFilesWithContent(solutionRepo);
return repositoryService.getFilesContentFromWorkingCopy(solutionRepo);
}
catch (GitAPIException e) {
throw new BehavioralSolutionEntryGenerationException("Error while reading solution repository", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import de.tum.in.www1.artemis.domain.VcsRepositoryUri;
import de.tum.in.www1.artemis.domain.hestia.ProgrammingExerciseGitDiffEntry;
import de.tum.in.www1.artemis.domain.hestia.ProgrammingExerciseGitDiffReport;
import de.tum.in.www1.artemis.service.ProfileService;
import de.tum.in.www1.artemis.service.connectors.GitService;
import de.tum.in.www1.artemis.web.rest.GitDiffReportParserService;

Expand All @@ -33,10 +34,13 @@ public class CommitHistoryService {

private final GitService gitService;

private final ProfileService profileService;

private final GitDiffReportParserService gitDiffReportParserService;

public CommitHistoryService(GitService gitService, GitDiffReportParserService gitDiffReportParserService) {
public CommitHistoryService(GitService gitService, ProfileService profileService, GitDiffReportParserService gitDiffReportParserService) {
this.gitService = gitService;
this.profileService = profileService;
this.gitDiffReportParserService = gitDiffReportParserService;
}

Expand All @@ -51,7 +55,17 @@ public CommitHistoryService(GitService gitService, GitDiffReportParserService gi
* @throws IOException If an error occurs while accessing the file system
*/
public ProgrammingExerciseGitDiffReport generateReportForCommits(VcsRepositoryUri repositoryUri, String commitHash1, String commitHash2) throws GitAPIException, IOException {
Repository repository = gitService.getOrCheckoutRepository(repositoryUri, true);

Repository repository;
if (profileService.isLocalVcsActive()) {
log.debug("Using local VCS generateReportForCommits on repo {}", repositoryUri);
repository = gitService.getBareRepository(repositoryUri);
}
else {
log.debug("Checking out repo {} for generateReportForCommits", repositoryUri);
repository = gitService.getOrCheckoutRepository(repositoryUri, true);
}

RevCommit commitOld = repository.parseCommit(repository.resolve(commitHash1));
RevCommit commitNew = repository.parseCommit(repository.resolve(commitHash2));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import de.tum.in.www1.artemis.domain.enumeration.RepositoryType;
import de.tum.in.www1.artemis.repository.UserRepository;
import de.tum.in.www1.artemis.service.FileService;
import de.tum.in.www1.artemis.service.RepositoryService;
import de.tum.in.www1.artemis.service.ResourceLoaderService;
import de.tum.in.www1.artemis.service.connectors.GitService;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import de.tum.in.www1.artemis.service.FilePathService;
import de.tum.in.www1.artemis.service.FileService;
import de.tum.in.www1.artemis.service.ProfileService;
import de.tum.in.www1.artemis.service.RepositoryService;
import de.tum.in.www1.artemis.service.StaticCodeAnalysisService;
import de.tum.in.www1.artemis.service.ZipFileService;
import de.tum.in.www1.artemis.service.connectors.GitService;
Expand Down Expand Up @@ -214,7 +213,6 @@ private void copyExerciseContentToRepository(Repository repository, RepositoryTy
try (var files = Files.walk(repository.getLocalPath())) {
files.filter(file -> "gradlew".equals(file.getFileName().toString())).forEach(file -> file.toFile().setExecutable(true));
}
repository.setContent(null);
}

private ProgrammingExercise getProgrammingExerciseFromDetailsFile(Path extractedZipPath) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package de.tum.in.www1.artemis.service;
package de.tum.in.www1.artemis.service.programming;

import static de.tum.in.www1.artemis.config.Constants.PROFILE_CORE;

Expand All @@ -10,6 +10,8 @@
import de.tum.in.www1.artemis.domain.participation.Participation;
import de.tum.in.www1.artemis.domain.participation.ProgrammingExerciseParticipation;
import de.tum.in.www1.artemis.domain.participation.StudentParticipation;
import de.tum.in.www1.artemis.service.AuthorizationCheckService;
import de.tum.in.www1.artemis.service.ExerciseDateService;
import de.tum.in.www1.artemis.service.plagiarism.PlagiarismService;
import de.tum.in.www1.artemis.web.rest.errors.AccessForbiddenException;
import de.tum.in.www1.artemis.web.rest.repository.RepositoryActionType;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package de.tum.in.www1.artemis.service;
package de.tum.in.www1.artemis.service.programming;

import static de.tum.in.www1.artemis.config.Constants.PROFILE_CORE;

Expand Down
Loading
Loading