Skip to content
This repository was archived by the owner on Feb 22, 2024. It is now read-only.

Commit 91d3556

Browse files
author
Jan-Willem Gmelig Meyling
committed
Added some more logging
1 parent 023dc0f commit 91d3556

7 files changed

Lines changed: 64 additions & 29 deletions

File tree

‎src/main/java/nl/tudelft/ewi/devhub/server/backend/warnings/IgnoredFileWarningGenerator.java‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.google.inject.Inject;
55
import com.google.inject.servlet.RequestScoped;
66
import lombok.SneakyThrows;
7+
import lombok.extern.slf4j.Slf4j;
78
import nl.tudelft.ewi.devhub.server.database.entities.Commit;
89
import nl.tudelft.ewi.devhub.server.database.entities.warnings.IgnoredFileWarning;
910
import nl.tudelft.ewi.devhub.server.web.models.GitPush;
@@ -18,14 +19,17 @@
1819
/**
1920
* @author Liam Clark
2021
*/
22+
@Slf4j
2123
@RequestScoped
2224
public class IgnoredFileWarningGenerator extends AbstractCommitWarningGenerator<IgnoredFileWarning, GitPush>
2325
implements CommitPushWarningGenerator<IgnoredFileWarning> {
2426

25-
private static final String[] extensions = {".iml",".class", ".bin", ".pdf", ".doc", ".docx", ".jar"};
27+
private static final String[] extensions = {".iml",".class", ".bin", ".doc", ".docx", ".jar"};
2628
private static final String[] folders = { ".idea/", ".metadata/", ".settings/",
2729
".project/", ".classpath/", "target/", "bin/", ".metadata/"};
30+
2831
private Repository repository;
32+
private Commit commit;
2933

3034
@Inject
3135
public IgnoredFileWarningGenerator(GitServerClient gitServerClient) {
@@ -35,32 +39,36 @@ public IgnoredFileWarningGenerator(GitServerClient gitServerClient) {
3539
@Override
3640
@SneakyThrows
3741
public Set<IgnoredFileWarning> generateWarnings(Commit commit, GitPush attachment) {
38-
repository = getRepository(commit);
42+
log.debug("Start generating warnings for {} in {}", commit, this);
43+
this.repository = getRepository(commit);
44+
this.commit = commit;
3945
Set<IgnoredFileWarning> warnings = Sets.newHashSet();
4046
walkCommitStructure("",commit.getCommitId(),warnings);
47+
log.debug("Finished generating warnings for {} in {}", commit, this);
4148
return warnings;
4249
}
4350

4451
@SneakyThrows
45-
public void walkCommitStructure(String path, String commit, Set<IgnoredFileWarning> warnings){
52+
public void walkCommitStructure(String path, String commitId, Set<IgnoredFileWarning> warnings){
4653

4754
if(folderViolation(path)){
4855
IgnoredFileWarning warning = new IgnoredFileWarning();
4956
warning.setFileName(path);
5057
warnings.add(warning);
5158
}
5259

53-
Map<String,EntryType> directory = repository.listDirectoryEntries(commit,path);
60+
Map<String,EntryType> directory = repository.listDirectoryEntries(commitId, path);
5461
directory.entrySet().stream()
5562
.filter(entry -> entry.getValue().equals(EntryType.FOLDER))
56-
.forEach(entry-> walkCommitStructure(path+entry.getKey(),commit,warnings));
63+
.forEach(entry-> walkCommitStructure(path+entry.getKey(),commitId, warnings));
5764

5865
directory.entrySet().stream()
5966
.filter(entry -> !entry.getValue().equals(EntryType.FOLDER))
6067
.map(Map.Entry::getKey)
6168
.filter(IgnoredFileWarningGenerator::fileViolation)
6269
.map(file -> {
6370
IgnoredFileWarning warning = new IgnoredFileWarning();
71+
warning.setCommit(commit);
6472
warning.setFileName(file);
6573
return warning;
6674
})

‎src/main/java/nl/tudelft/ewi/devhub/server/backend/warnings/LargeCommitWarningGenerator.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.google.common.collect.Sets;
55
import com.google.inject.Inject;
66
import lombok.SneakyThrows;
7+
import lombok.extern.slf4j.Slf4j;
78
import nl.tudelft.ewi.devhub.server.database.entities.Commit;
89
import nl.tudelft.ewi.devhub.server.database.entities.Course;
910
import nl.tudelft.ewi.devhub.server.database.entities.warnings.LargeCommitWarning;
@@ -18,6 +19,7 @@
1819
/**
1920
* @author Liam Clark
2021
*/
22+
@Slf4j
2123
public class LargeCommitWarningGenerator extends AbstractCommitWarningGenerator<LargeCommitWarning, GitPush>
2224
implements CommitPushWarningGenerator<LargeCommitWarning> {
2325

@@ -34,14 +36,17 @@ public LargeCommitWarningGenerator(GitServerClient gitServerClient) {
3436
@Override
3537
@SneakyThrows
3638
public Set<LargeCommitWarning> generateWarnings(Commit commit, GitPush attachment) {
39+
log.debug("Start generating warnings for {} in {}", commit, this);
3740
List<DiffFile> diffs = getGitCommit(commit).diff().getDiffs();
3841

3942
if(tooManyFiles(diffs, commit) || tooManyLineChanges(diffs, commit)) {
4043
LargeCommitWarning warning = new LargeCommitWarning();
4144
warning.setCommit(commit);
45+
log.debug("Finished generating warnings for {} in {}", commit, this);
4246
return Sets.newHashSet(warning);
4347
}
4448

49+
log.debug("Finished generating warnings for {} in {}", commit, this);
4550
return Collections.emptySet();
4651
}
4752

‎src/main/java/nl/tudelft/ewi/devhub/server/backend/warnings/LargeFileWarningGenerator.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.google.inject.Inject;
55
import com.google.inject.servlet.RequestScoped;
66
import lombok.SneakyThrows;
7+
import lombok.extern.slf4j.Slf4j;
78
import nl.tudelft.ewi.devhub.server.database.entities.Commit;
89
import nl.tudelft.ewi.devhub.server.database.entities.Course;
910
import nl.tudelft.ewi.devhub.server.database.entities.warnings.LargeFileWarning;
@@ -20,6 +21,7 @@
2021
/**
2122
* @author Jan-Willem Gmelig Meyling
2223
*/
24+
@Slf4j
2325
@RequestScoped
2426
public class LargeFileWarningGenerator extends AbstractCommitWarningGenerator<LargeFileWarning, GitPush>
2527
implements CommitPushWarningGenerator<LargeFileWarning> {
@@ -39,17 +41,21 @@ public LargeFileWarningGenerator(GitServerClient gitServerClient) {
3941
@Override
4042
@SneakyThrows
4143
public Set<LargeFileWarning> generateWarnings(Commit commit, GitPush attachment) {
44+
log.debug("Started generating warnings for {} in {}", commit, this);
4245
final List<DiffFile> diffs = getGitCommit(commit).diff().getDiffs();
4346
this.repository = getRepository(commit);
4447
this.commit = commit;
4548
this.maxFileSize = getIntegerProperty(commit.getRepository().getCourse(), MAX_FILE_SIZE_PROPERTY, MAX_FILE_SIZE);
4649

47-
return diffs.stream()
50+
Set<LargeFileWarning> warnings = diffs.stream()
4851
.filter(file -> !file.isDeleted())
4952
.filter(this::filterTextFiles)
5053
.filter(this::filterLargeFiles)
5154
.map(this::mapToWarning)
5255
.collect(Collectors.toSet());
56+
57+
log.debug("Finished generating warnings for {} in {}", commit, this);
58+
return warnings;
5359
}
5460

5561
@SneakyThrows

‎src/main/java/nl/tudelft/ewi/devhub/server/backend/warnings/PullRequestWarningGenerator.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public PullRequestWarningGenerator(GitServerClient gitServerClient) {
3131

3232
@Override
3333
public Set<GitUsageWarning> generateWarnings(final Commit commitEntity, final GitPush attachment) {
34+
log.debug("Started generating warnings for {} in {}", commitEntity, this);
3435
final Set<GitUsageWarning> warnings = Sets.newHashSet();
3536

3637
try {
@@ -44,6 +45,7 @@ public Set<GitUsageWarning> generateWarnings(final Commit commitEntity, final Gi
4445
log.warn(e.getMessage(), e);
4546
}
4647

48+
log.debug("Finished generating warnings for {} in {}", commitEntity, this);
4749
return warnings;
4850
}
4951

‎src/main/java/nl/tudelft/ewi/devhub/server/database/controllers/Warnings.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package nl.tudelft.ewi.devhub.server.database.controllers;
22

3+
import com.google.common.base.Preconditions;
34
import com.google.common.collect.ImmutableList;
45
import com.google.inject.Inject;
56
import com.google.inject.persist.Transactional;
@@ -137,6 +138,9 @@ public List<CommitWarning> getAllCommitWarningsFor(final Group group, final Set<
137138
*/
138139
@Transactional
139140
public <V extends CommitWarning> Set<V> persist(final Group group, final Set<V> warnings) {
141+
Preconditions.checkNotNull(group);
142+
Preconditions.checkNotNull(warnings);
143+
140144
final Set<Commit> commits = getCommitsForWarnings(warnings);
141145
final List<CommitWarning> existingWarnings = getAllCommitWarningsFor(group, commits);
142146
return warnings.stream()
@@ -151,8 +155,10 @@ public <V extends CommitWarning> Set<V> persist(final Group group, final Set<V>
151155
* @return Set of commits
152156
*/
153157
protected static Set<Commit> getCommitsForWarnings(Collection<? extends CommitWarning> warnings) {
158+
assert warnings != null;
154159
return warnings.stream()
155160
.map(CommitWarning::getCommit)
161+
.map(Preconditions::checkNotNull)
156162
.collect(toSet());
157163
}
158164

‎src/main/java/nl/tudelft/ewi/devhub/server/web/resources/HooksResource.java‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package nl.tudelft.ewi.devhub.server.web.resources;
22

33
import com.google.common.base.Joiner;
4+
import com.google.common.base.Preconditions;
45
import com.google.common.collect.Lists;
56
import com.google.inject.persist.Transactional;
67
import com.google.inject.servlet.RequestScoped;
@@ -128,7 +129,7 @@ public void onGitPush(@Context HttpServletRequest request, @Valid GitPush push)
128129
.flatMap(branchModel -> findOpenPullRequests(group, branchModel))
129130
.forEach(pullRequest -> updatePullRequest(repository, pullRequest));
130131

131-
notBuildCommits.forEach(commit -> triggerWarnings(commit, push));
132+
notBuildCommits.forEach(commit -> triggerWarnings(group, commit, push));
132133
}
133134

134135
@POST
@@ -140,29 +141,37 @@ public void triggerWarnings(@QueryParam("repository") @NotEmpty String repositor
140141

141142
Group group = groups.findByRepoName(repository);
142143
Commit commit = commits.ensureExists(group, commitId);
144+
145+
Preconditions.checkNotNull(group);
146+
Preconditions.checkNotNull(commit);
147+
143148
GitPush gitPush = new GitPush();
144149
gitPush.setRepository(repository);
145-
triggerWarnings(commit, gitPush);
150+
triggerWarnings(group, commit, gitPush);
146151
}
147152

148-
protected void triggerWarnings(Commit commit, GitPush gitPush) {
149-
try {
150-
Group group = commit.getRepository();
153+
protected void triggerWarnings(final Group group, final Commit commit, final GitPush gitPush) {
154+
assert group != null;
155+
assert commit != null;
156+
assert gitPush != null;
151157

152-
Set<? extends CommitWarning> pushWarnings = pushWarningGenerators.stream()
153-
.flatMap(generator -> {
158+
Set<? extends CommitWarning> pushWarnings = pushWarningGenerators.stream()
159+
.flatMap(generator -> {
160+
try {
154161
Set<CommitWarning> commitWarningList = generator.generateWarnings(commit, gitPush);
155162
return commitWarningList.stream();
156-
})
157-
.collect(Collectors.toSet());
163+
}
164+
catch (Exception e) {
165+
log.warn("Failed to generate warnings with {} for {} ", generator, commit);
166+
log.warn(e.getMessage(), e);
167+
return Stream.empty();
168+
}
169+
})
170+
.collect(Collectors.toSet());
158171

159-
Set<? extends CommitWarning> persistedWarnings = warnings.persist(group, pushWarnings);
160-
log.info("Persisted {} of {} push warnings for {}", persistedWarnings.size(),
161-
pushWarnings.size(), group);
162-
}
163-
catch (Exception e) {
164-
log.warn("Failed to generate warnings for {}", e, commit);
165-
}
172+
Set<? extends CommitWarning> persistedWarnings = warnings.persist(group, pushWarnings);
173+
log.info("Persisted {} of {} push warnings for {}", persistedWarnings.size(),
174+
pushWarnings.size(), group);
166175
}
167176

168177
@SneakyThrows

‎src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/IgnoredFileWarningTest.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
package nl.tudelft.ewi.devhub.server.backend.warnings;
22

3-
import static org.junit.Assert.assertEquals;
4-
import static org.mockito.Matchers.anyString;
5-
import static org.mockito.Mockito.when;
63
import nl.tudelft.ewi.devhub.server.database.entities.Group;
7-
import nl.tudelft.ewi.devhub.server.database.entities.warnings.GitUsageWarning;
84
import nl.tudelft.ewi.devhub.server.database.entities.warnings.IgnoredFileWarning;
95
import nl.tudelft.ewi.git.client.Branch;
106
import nl.tudelft.ewi.git.client.Commit;
@@ -25,6 +21,10 @@
2521
import java.util.HashMap;
2622
import java.util.Map;
2723

24+
import static org.junit.Assert.assertEquals;
25+
import static org.mockito.Matchers.anyString;
26+
import static org.mockito.Mockito.when;
27+
2828
/**
2929
* @author Jan-Willem Gmelig Meyling
3030
*/
@@ -44,7 +44,7 @@ public class IgnoredFileWarningTest {
4444
@Mock private CommitModel commitModel;
4545
@InjectMocks private IgnoredFileWarningGenerator generator;
4646

47-
private GitUsageWarning warning;
47+
private IgnoredFileWarning warning;
4848

4949
@Before
5050
public void beforeTest() throws Exception {
@@ -57,15 +57,14 @@ public void beforeTest() throws Exception {
5757

5858
directory1.clear();
5959

60-
warning = new GitUsageWarning();
60+
warning = new IgnoredFileWarning();
6161
warning.setCommit(commitEntity);
6262
}
6363

6464
@Test
6565
public void TestIngoredFile(){
6666
directory1.put("types.class",EntryType.TEXT);
6767
Collection<IgnoredFileWarning> warnings = generator.generateWarnings(commitEntity, null);
68-
IgnoredFileWarning warning = new IgnoredFileWarning();
6968
warning.setFileName("types.class");
7069
assertEquals(warning,warnings.stream().findFirst().get());
7170
}

0 commit comments

Comments
 (0)