Merge branch 'stable-2.14' into stable-2.15 * stable-2.14: Make transitive starlark loads explicit Change-Id: Iaf2394c81b33a84786cad8efc4fbb2fa48bdf771
diff --git a/BUILD b/BUILD index 69fb88c..9e3a045 100644 --- a/BUILD +++ b/BUILD
@@ -6,14 +6,10 @@ srcs = glob(["src/main/java/**/*.java"]), manifest_entries = [ "Gerrit-PluginName: uploadvalidator", - "Gerrit-ApiVersion: 2.14", + "Gerrit-ApiVersion: 2.15", "Gerrit-Module: com.googlesource.gerrit.plugins.uploadvalidator.Module", ], resources = glob(["src/main/resources/**/*"]), - deps = [ - "@commons_io//jar", - "@tika_core//jar", - ], ) TEST_SRCS = "src/test/java/**/*Test.java"
diff --git a/WORKSPACE b/WORKSPACE index ead800c..0a12f0e 100644 --- a/WORKSPACE +++ b/WORKSPACE
@@ -3,7 +3,7 @@ load("//:bazlets.bzl", "load_bazlets") load_bazlets( - commit = "62c3c8e0b4b767d1f3b78998ad1aba42a44a9661", + commit = "70d6ae0cee03c456fa4b32c807f3d13c032ef498", #local_path = "/home/<user>/projects/bazlets", )
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl index 9ae3517..93746e1 100644 --- a/external_plugin_deps.bzl +++ b/external_plugin_deps.bzl
@@ -2,12 +2,6 @@ def external_plugin_deps(): maven_jar( - name = "tika_core", - artifact = "org.apache.tika:tika-core:1.12", - sha1 = "5ab95580d22fe1dee79cffbcd98bb509a32da09b", - ) - - maven_jar( name = "commons_io", artifact = "commons-io:commons-io:1.4", sha1 = "a8762d07e76cfde2395257a5da47ba7c1dbd3dce",
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java index b47d259..d5156f9 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java
@@ -40,6 +40,7 @@ import com.google.inject.name.Named; import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.text.MessageFormat; @@ -52,10 +53,12 @@ import java.util.concurrent.ExecutionException; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.eclipse.jgit.diff.RawText; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; public class BlockedKeywordValidator implements CommitValidationListener { private static final String KEY_CHECK_BLOCKED_KEYWORD = "blockedKeyword"; @@ -127,7 +130,12 @@ Arrays.asList(cfg.getStringList(KEY_CHECK_BLOCKED_KEYWORD_PATTERN))); try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = - performValidation(repo, receiveEvent.commit, blockedKeywordPatterns.values(), cfg); + performValidation( + repo, + receiveEvent.commit, + receiveEvent.revWalk, + blockedKeywordPatterns.values(), + cfg); if (!messages.isEmpty()) { throw new CommitValidationException( "includes files containing blocked keywords", messages); @@ -144,16 +152,19 @@ List<CommitValidationMessage> performValidation( Repository repo, RevCommit c, + RevWalk revWalk, ImmutableCollection<Pattern> blockedKeywordPartterns, PluginConfig cfg) throws IOException, ExecutionException { List<CommitValidationMessage> messages = new LinkedList<>(); checkCommitMessageForBlockedKeywords(blockedKeywordPartterns, messages, c.getFullMessage()); - Map<String, ObjectId> content = CommitUtils.getChangedContent(repo, c); + Map<String, ObjectId> content = CommitUtils.getChangedContent(repo, c, revWalk); for (String path : content.keySet()) { - ObjectLoader ol = repo.open(content.get(path)); - if (contentTypeUtil.isBinary(ol, path, cfg)) { - continue; + ObjectLoader ol = revWalk.getObjectReader().open(content.get(path)); + try (InputStream in = ol.openStream()) { + if (RawText.isBinary(in) || contentTypeUtil.isBlacklistedBinaryContentType(ol, path, cfg)) { + continue; + } } checkFileForBlockedKeywords(blockedKeywordPartterns, messages, path, ol); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java new file mode 100644 index 0000000..6deb42c --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailValidator.java
@@ -0,0 +1,155 @@ +// Copyright (C) 2018 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.googlesource.gerrit.plugins.uploadvalidator; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Strings; +import com.google.gerrit.extensions.annotations.Exports; +import com.google.gerrit.extensions.annotations.PluginName; +import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType; +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.server.config.PluginConfig; +import com.google.gerrit.server.config.PluginConfigFactory; +import com.google.gerrit.server.config.ProjectConfigEntry; +import com.google.gerrit.server.events.CommitReceivedEvent; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.git.validators.CommitValidationException; +import com.google.gerrit.server.git.validators.CommitValidationListener; +import com.google.gerrit.server.git.validators.CommitValidationMessage; +import com.google.gerrit.server.project.NoSuchProjectException; +import com.google.inject.AbstractModule; +import com.google.inject.Inject; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.regex.Pattern; + +public class ChangeEmailValidator implements CommitValidationListener { + public static AbstractModule module() { + return new AbstractModule() { + @Override + public void configure() { + DynamicSet.bind(binder(), CommitValidationListener.class).to(ChangeEmailValidator.class); + bind(ProjectConfigEntry.class) + .annotatedWith(Exports.named(KEY_ALLOWED_AUTHOR_EMAIL_PATTERN)) + .toInstance( + new ProjectConfigEntry( + "Author Email Pattern", + null, + ProjectConfigEntryType.ARRAY, + null, + false, + "Commits with author email not matching one of these pattterns will be rejected.")); + bind(ProjectConfigEntry.class) + .annotatedWith(Exports.named(KEY_ALLOWED_COMMITTER_EMAIL_PATTERN)) + .toInstance( + new ProjectConfigEntry( + "Committer Email Pattern", + null, + ProjectConfigEntryType.ARRAY, + null, + false, + "Commits with committer email not matching one of these patterns will be rejected.")); + } + }; + } + + public static final String KEY_ALLOWED_AUTHOR_EMAIL_PATTERN = "allowedAuthorEmailPattern"; + public static final String KEY_ALLOWED_COMMITTER_EMAIL_PATTERN = "allowedCommitterEmailPattern"; + private final String pluginName; + private final PluginConfigFactory cfgFactory; + private final GitRepositoryManager repoManager; + private final ValidatorConfig validatorConfig; + + @Inject + ChangeEmailValidator( + @PluginName String pluginName, + PluginConfigFactory cfgFactory, + GitRepositoryManager repoManager, + ValidatorConfig validatorConfig) { + this.pluginName = pluginName; + this.cfgFactory = cfgFactory; + this.repoManager = repoManager; + this.validatorConfig = validatorConfig; + } + + @VisibleForTesting + static String[] getAllowedAuthorEmailPatterns(PluginConfig cfg) { + return cfg.getStringList(KEY_ALLOWED_AUTHOR_EMAIL_PATTERN); + } + + @VisibleForTesting + static String[] getAllowedCommitterEmailPatterns(PluginConfig cfg) { + return cfg.getStringList(KEY_ALLOWED_COMMITTER_EMAIL_PATTERN); + } + + @VisibleForTesting + static boolean isAuthorActive(PluginConfig cfg) { + return cfg.getStringList(KEY_ALLOWED_AUTHOR_EMAIL_PATTERN).length > 0; + } + + @VisibleForTesting + static boolean isCommitterActive(PluginConfig cfg) { + return cfg.getStringList(KEY_ALLOWED_COMMITTER_EMAIL_PATTERN).length > 0; + } + + @Override + public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent) + throws CommitValidationException { + try { + PluginConfig cfg = + cfgFactory.getFromProjectConfigWithInheritance( + receiveEvent.project.getNameKey(), pluginName); + if (isAuthorActive(cfg) + && validatorConfig.isEnabledForRef( + receiveEvent.user, + receiveEvent.getProjectNameKey(), + receiveEvent.getRefName(), + KEY_ALLOWED_AUTHOR_EMAIL_PATTERN)) { + if (!performValidation( + receiveEvent.commit.getAuthorIdent().getEmailAddress(), + getAllowedAuthorEmailPatterns(cfg))) { + throw new CommitValidationException( + "Author Email <" + + receiveEvent.commit.getAuthorIdent().getEmailAddress() + + "> - is not allowed for this Project."); + } + } + if (isCommitterActive(cfg) + && validatorConfig.isEnabledForRef( + receiveEvent.user, + receiveEvent.getProjectNameKey(), + receiveEvent.getRefName(), + KEY_ALLOWED_COMMITTER_EMAIL_PATTERN)) { + if (!performValidation( + receiveEvent.commit.getCommitterIdent().getEmailAddress(), + getAllowedCommitterEmailPatterns(cfg))) { + throw new CommitValidationException( + "Committer Email <" + + receiveEvent.commit.getCommitterIdent().getEmailAddress() + + "> - is not allowed for this Project."); + } + } + } catch (NoSuchProjectException e) { + throw new CommitValidationException("Failed to check for Change Email Patterns ", e); + } + return Collections.emptyList(); + } + + @VisibleForTesting + static boolean performValidation(String email, String[] allowedEmailPatterns) { + return Arrays.stream(allowedEmailPatterns) + .anyMatch(s -> Pattern.matches(s, Strings.nullToEmpty(email))); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/CommitUtils.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/CommitUtils.java index 02db4e8..5505343 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/CommitUtils.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/CommitUtils.java
@@ -26,6 +26,7 @@ import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.filter.TreeFilter; +/** Utility class for checking whether commits are different. */ public class CommitUtils { /** @@ -38,8 +39,9 @@ * parents. * @throws IOException */ - public static Set<String> getChangedPaths(Repository repo, RevCommit c) throws IOException { - Map<String, ObjectId> content = getChangedContent(repo, c); + public static Set<String> getChangedPaths(Repository repo, RevCommit c, RevWalk revWalk) + throws IOException { + Map<String, ObjectId> content = getChangedContent(repo, c, revWalk); return content.keySet(); } @@ -59,13 +61,14 @@ * @return A Map containing all files which differ between the passed commit and its parents. * @throws IOException */ - public static Map<String, ObjectId> getChangedContent(Repository repo, RevCommit c) - throws IOException { + public static Map<String, ObjectId> getChangedContent( + Repository repo, RevCommit c, RevWalk revWalk) throws IOException { final Map<String, ObjectId> content = new HashMap<>(); visitChangedEntries( repo, c, + revWalk, new TreeWalkVisitor() { @Override public void onVisit(TreeWalk tw) { @@ -92,29 +95,18 @@ * @param visitor A TreeWalkVisitor with the desired action * @throws IOException */ - public static void visitChangedEntries(Repository repo, RevCommit c, TreeWalkVisitor visitor) - throws IOException { - try (TreeWalk tw = new TreeWalk(repo)) { + public static void visitChangedEntries( + Repository repo, RevCommit c, RevWalk revWalk, TreeWalkVisitor visitor) throws IOException { + try (TreeWalk tw = new TreeWalk(revWalk.getObjectReader())) { tw.setRecursive(true); tw.setFilter(TreeFilter.ANY_DIFF); tw.addTree(c.getTree()); if (c.getParentCount() > 0) { - @SuppressWarnings("resource") - RevWalk rw = null; - try { - for (RevCommit p : c.getParents()) { - if (p.getTree() == null) { - if (rw == null) { - rw = new RevWalk(repo); - } - rw.parseHeaders(p); - } - tw.addTree(p.getTree()); + for (RevCommit p : c.getParents()) { + if (p.getTree() == null) { + revWalk.parseHeaders(p); } - } finally { - if (rw != null) { - rw.close(); - } + tw.addTree(p.getTree()); } while (tw.next()) { if (isDifferentToAllParents(c, tw)) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeUtil.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeUtil.java index 368a72d..ddb18db 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeUtil.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeUtil.java
@@ -22,6 +22,7 @@ import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.ProjectConfigEntry; +import com.google.gerrit.server.mime.FileTypeRegistry; import com.google.inject.AbstractModule; import com.google.inject.Inject; import com.google.inject.name.Named; @@ -29,10 +30,6 @@ import java.io.InputStream; import java.util.concurrent.ExecutionException; import java.util.regex.Pattern; -import org.apache.tika.Tika; -import org.apache.tika.config.TikaConfig; -import org.apache.tika.io.TikaInputStream; -import org.apache.tika.metadata.Metadata; import org.eclipse.jgit.lib.ObjectLoader; public class ContentTypeUtil { @@ -68,24 +65,24 @@ } private final LoadingCache<String, Pattern> patternCache; - private final Tika tika = new Tika(TikaConfig.getDefaultConfig()); + private final FileTypeRegistry mimeUtil; @Inject - ContentTypeUtil(@Named(CACHE_NAME) LoadingCache<String, Pattern> patternCache) { + ContentTypeUtil( + @Named(CACHE_NAME) LoadingCache<String, Pattern> patternCache, FileTypeRegistry mimeUtil) { this.patternCache = patternCache; + this.mimeUtil = mimeUtil; } - public boolean isBinary(ObjectLoader ol, String pathname, PluginConfig cfg) + public boolean isBlacklistedBinaryContentType(ObjectLoader ol, String pathname, PluginConfig cfg) throws IOException, ExecutionException { - try (InputStream is = ol.openStream()) { - return matchesAny(getContentType(is, pathname), getBinaryTypes(cfg)); - } + return matchesAny(getContentType(ol, pathname), getBinaryTypes(cfg)); } - public String getContentType(InputStream is, String pathname) throws IOException { - Metadata metadata = new Metadata(); - metadata.set(Metadata.RESOURCE_NAME_KEY, pathname); - return tika.detect(TikaInputStream.get(is), metadata); + public String getContentType(ObjectLoader ol, String pathname) throws IOException { + try (InputStream is = ol.openStream()) { + return mimeUtil.getMimeType(pathname, is).toString(); + } } @VisibleForTesting
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidator.java index 8893f00..1cc5fd4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidator.java
@@ -38,9 +38,9 @@ import java.util.concurrent.ExecutionException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; -import org.eclipse.jgit.lib.ObjectStream; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; public class ContentTypeValidator implements CommitValidationListener { @@ -129,7 +129,12 @@ KEY_BLOCKED_CONTENT_TYPE)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = - performValidation(repo, receiveEvent.commit, getBlockedTypes(cfg), isWhitelist(cfg)); + performValidation( + repo, + receiveEvent.commit, + receiveEvent.revWalk, + getBlockedTypes(cfg), + isWhitelist(cfg)); if (!messages.isEmpty()) { throw new CommitValidationException("contains blocked content type", messages); } @@ -143,20 +148,18 @@ @VisibleForTesting List<CommitValidationMessage> performValidation( - Repository repo, RevCommit c, String[] blockedTypes, boolean whitelist) + Repository repo, RevCommit c, RevWalk revWalk, String[] blockedTypes, boolean whitelist) throws IOException, ExecutionException { List<CommitValidationMessage> messages = new LinkedList<>(); - Map<String, ObjectId> content = CommitUtils.getChangedContent(repo, c); + Map<String, ObjectId> content = CommitUtils.getChangedContent(repo, c, revWalk); for (String path : content.keySet()) { - ObjectLoader ol = repo.open(content.get(path)); - try (ObjectStream os = ol.openStream()) { - String contentType = contentTypeUtil.getContentType(os, path); - if ((contentTypeUtil.matchesAny(contentType, blockedTypes) && !whitelist) - || (!contentTypeUtil.matchesAny(contentType, blockedTypes) && whitelist)) { - messages.add( - new CommitValidationMessage( - "found blocked content type (" + contentType + ") in file: " + path, true)); - } + ObjectLoader ol = revWalk.getObjectReader().open(content.get(path)); + String contentType = contentTypeUtil.getContentType(ol, path); + if ((contentTypeUtil.matchesAny(contentType, blockedTypes) && !whitelist) + || (!contentTypeUtil.matchesAny(contentType, blockedTypes) && whitelist)) { + messages.add( + new CommitValidationMessage( + "found blocked content type (" + contentType + ") in file: " + path, true)); } } return messages;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidator.java index c89326e..0b93e6d 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidator.java
@@ -47,6 +47,7 @@ import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.TreeWalk; public class DuplicatePathnameValidator implements CommitValidationListener { @@ -187,7 +188,8 @@ KEY_REJECT_DUPLICATE_PATHNAMES)) { locale = getLocale(cfg); try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { - List<CommitValidationMessage> messages = performValidation(repo, receiveEvent.commit); + List<CommitValidationMessage> messages = + performValidation(repo, receiveEvent.commit, receiveEvent.revWalk); if (!messages.isEmpty()) { throw new CommitValidationException("contains duplicate pathnames", messages); } @@ -200,10 +202,11 @@ } @VisibleForTesting - List<CommitValidationMessage> performValidation(Repository repo, RevCommit c) throws IOException { + List<CommitValidationMessage> performValidation(Repository repo, RevCommit c, RevWalk revWalk) + throws IOException { List<CommitValidationMessage> messages = new LinkedList<>(); - Set<String> pathnames = CommitUtils.getChangedPaths(repo, c); + Set<String> pathnames = CommitUtils.getChangedPaths(repo, c, revWalk); checkForDuplicatesInSet(pathnames, messages); if (!messages.isEmpty() || c.getParentCount() == 0) { return messages;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidator.java index f3652c7..716f32e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidator.java
@@ -36,6 +36,7 @@ import java.util.List; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; public class FileExtensionValidator implements CommitValidationListener { @@ -106,7 +107,8 @@ KEY_BLOCKED_FILE_EXTENSION)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = - performValidation(repo, receiveEvent.commit, getBlockedExtensions(cfg)); + performValidation( + repo, receiveEvent.commit, receiveEvent.revWalk, getBlockedExtensions(cfg)); if (!messages.isEmpty()) { throw new CommitValidationException( "contains files with blocked file extensions", messages); @@ -120,9 +122,10 @@ } static List<CommitValidationMessage> performValidation( - Repository repo, RevCommit c, List<String> blockedFileExtensions) throws IOException { + Repository repo, RevCommit c, RevWalk revWalk, List<String> blockedFileExtensions) + throws IOException { List<CommitValidationMessage> messages = new LinkedList<>(); - for (String file : CommitUtils.getChangedPaths(repo, c)) { + for (String file : CommitUtils.getChangedPaths(repo, c, revWalk)) { for (String blockedExtension : blockedFileExtensions) { if (file.toLowerCase().endsWith(blockedExtension.toLowerCase())) { messages.add(new CommitValidationMessage("blocked file: " + file, true));
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidator.java index ac914d5..846288f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidator.java
@@ -37,6 +37,7 @@ import java.util.regex.Pattern; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; public class InvalidFilenameValidator implements CommitValidationListener { @@ -102,7 +103,10 @@ try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = performValidation( - repo, receiveEvent.commit, cfg.getStringList(KEY_INVALID_FILENAME_PATTERN)); + repo, + receiveEvent.commit, + receiveEvent.revWalk, + cfg.getStringList(KEY_INVALID_FILENAME_PATTERN)); if (!messages.isEmpty()) { throw new CommitValidationException( "contains files with an invalid filename", messages); @@ -116,13 +120,13 @@ } static List<CommitValidationMessage> performValidation( - Repository repo, RevCommit c, String[] patterns) throws IOException { + Repository repo, RevCommit c, RevWalk revWalk, String[] patterns) throws IOException { List<Pattern> invalidFilenamePatterns = new ArrayList<>(); for (String s : patterns) { invalidFilenamePatterns.add(Pattern.compile(s)); } List<CommitValidationMessage> messages = new LinkedList<>(); - for (String file : CommitUtils.getChangedPaths(repo, c)) { + for (String file : CommitUtils.getChangedPaths(repo, c, revWalk)) { for (Pattern p : invalidFilenamePatterns) { if (p.matcher(file).find()) { messages.add(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidator.java index 95fb586..2f602b0 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidator.java
@@ -31,6 +31,7 @@ import com.google.inject.AbstractModule; import com.google.inject.Inject; import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.util.Collections; @@ -38,10 +39,12 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ExecutionException; +import org.eclipse.jgit.diff.RawText; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; public class InvalidLineEndingValidator implements CommitValidationListener { @@ -109,7 +112,7 @@ KEY_CHECK_REJECT_WINDOWS_LINE_ENDINGS)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = - performValidation(repo, receiveEvent.commit, cfg); + performValidation(repo, receiveEvent.commit, receiveEvent.revWalk, cfg); if (!messages.isEmpty()) { throw new CommitValidationException( "contains files with a Windows line ending", messages); @@ -123,14 +126,17 @@ } @VisibleForTesting - List<CommitValidationMessage> performValidation(Repository repo, RevCommit c, PluginConfig cfg) + List<CommitValidationMessage> performValidation( + Repository repo, RevCommit c, RevWalk revWalk, PluginConfig cfg) throws IOException, ExecutionException { List<CommitValidationMessage> messages = new LinkedList<>(); - Map<String, ObjectId> content = CommitUtils.getChangedContent(repo, c); + Map<String, ObjectId> content = CommitUtils.getChangedContent(repo, c, revWalk); for (String path : content.keySet()) { - ObjectLoader ol = repo.open(content.get(path)); - if (contentTypeUtil.isBinary(ol, path, cfg)) { - continue; + ObjectLoader ol = revWalk.getObjectReader().open(content.get(path)); + try (InputStream in = ol.openStream()) { + if (RawText.isBinary(in) || contentTypeUtil.isBlacklistedBinaryContentType(ol, path, cfg)) { + continue; + } } try (InputStreamReader isr = new InputStreamReader(ol.openStream(), StandardCharsets.UTF_8)) { if (doesInputStreanContainCR(isr)) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidator.java index 2899613..2216451 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidator.java
@@ -34,6 +34,7 @@ import java.util.List; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; public class MaxPathLengthValidator implements CommitValidationListener { @@ -96,7 +97,7 @@ int maxPathLength = cfg.getInt(KEY_MAX_PATH_LENGTH, 0); try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { List<CommitValidationMessage> messages = - performValidation(repo, receiveEvent.commit, maxPathLength); + performValidation(repo, receiveEvent.commit, receiveEvent.revWalk, maxPathLength); if (!messages.isEmpty()) { throw new CommitValidationException( "contains files with too long paths (max path length: " + maxPathLength + ")", @@ -111,9 +112,9 @@ } static List<CommitValidationMessage> performValidation( - Repository repo, RevCommit c, int maxPathLength) throws IOException { + Repository repo, RevCommit c, RevWalk revWalk, int maxPathLength) throws IOException { List<CommitValidationMessage> messages = new LinkedList<>(); - for (String file : CommitUtils.getChangedPaths(repo, c)) { + for (String file : CommitUtils.getChangedPaths(repo, c, revWalk)) { if (file.length() > maxPathLength) { messages.add(new CommitValidationMessage("path too long: " + file, true)); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/Module.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/Module.java index 5b6677a..e15ecb2 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/Module.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/Module.java
@@ -27,6 +27,7 @@ install(FooterValidator.module()); install(MaxPathLengthValidator.module()); install(FileExtensionValidator.module()); + install(ChangeEmailValidator.module()); install(InvalidFilenameValidator.module()); install(SubmoduleValidator.module()); install(SymlinkValidator.module());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidator.java index 9e4a31c..08d4d3f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidator.java
@@ -36,6 +36,7 @@ import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.TreeWalk; public class SubmoduleValidator implements CommitValidationListener { @@ -97,7 +98,8 @@ receiveEvent.getRefName(), KEY_CHECK_SUBMODULE)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { - List<CommitValidationMessage> messages = performValidation(repo, receiveEvent.commit); + List<CommitValidationMessage> messages = + performValidation(repo, receiveEvent.commit, receiveEvent.revWalk); if (!messages.isEmpty()) { throw new CommitValidationException("contains submodules", messages); } @@ -118,13 +120,14 @@ return (tw.getRawMode(0) & FileMode.TYPE_MASK) == FileMode.TYPE_GITLINK; } - static List<CommitValidationMessage> performValidation(Repository repo, RevCommit c) - throws IOException { + static List<CommitValidationMessage> performValidation( + Repository repo, RevCommit c, RevWalk revWalk) throws IOException { final List<CommitValidationMessage> messages = new LinkedList<>(); CommitUtils.visitChangedEntries( repo, c, + revWalk, new TreeWalkVisitor() { @Override public void onVisit(TreeWalk tw) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidator.java index 057efff..7b7d893 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidator.java
@@ -36,6 +36,7 @@ import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.TreeWalk; public class SymlinkValidator implements CommitValidationListener { @@ -98,7 +99,8 @@ receiveEvent.getRefName(), KEY_CHECK_SYMLINK)) { try (Repository repo = repoManager.openRepository(receiveEvent.project.getNameKey())) { - List<CommitValidationMessage> messages = performValidation(repo, receiveEvent.commit); + List<CommitValidationMessage> messages = + performValidation(repo, receiveEvent.commit, receiveEvent.revWalk); if (!messages.isEmpty()) { throw new CommitValidationException("contains symbolic links", messages); } @@ -119,13 +121,14 @@ return (tw.getRawMode(0) & FileMode.TYPE_MASK) == FileMode.TYPE_SYMLINK; } - static List<CommitValidationMessage> performValidation(Repository repo, RevCommit c) - throws IOException { + static List<CommitValidationMessage> performValidation( + Repository repo, RevCommit c, RevWalk revWalk) throws IOException { final List<CommitValidationMessage> messages = new LinkedList<>(); CommitUtils.visitChangedEntries( repo, c, + revWalk, new TreeWalkVisitor() { @Override public void onVisit(TreeWalk tw) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java index e30e177..c4ae334 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java
@@ -14,19 +14,24 @@ package com.googlesource.gerrit.plugins.uploadvalidator; +import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.RefConfigSection; import com.google.gerrit.extensions.annotations.Exports; import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.ProjectConfigEntry; +import com.google.gerrit.server.group.InternalGroup; import com.google.gerrit.server.project.RefPatternMatcher; +import com.google.gerrit.server.query.group.InternalGroupQuery; +import com.google.gwtorm.server.OrmException; import com.google.inject.AbstractModule; import com.google.inject.Inject; +import com.google.inject.Provider; import java.util.Arrays; +import java.util.Optional; import java.util.regex.Pattern; import java.util.stream.Stream; import org.slf4j.Logger; @@ -37,7 +42,7 @@ private static final String KEY_PROJECT = "project"; private static final String KEY_REF = "ref"; private final ConfigFactory configFactory; - private final GroupCache groupCache; + private final GroupByNameFinder groupByNameFinder; public static AbstractModule module() { return new AbstractModule() { @@ -63,14 +68,15 @@ null, false, "Only refs that match this regex will be validated.")); + bind(GroupByNameFinder.class).to(GroupByNameFromIndexFinder.class); } }; } @Inject - public ValidatorConfig(ConfigFactory configFactory, GroupCache groupCache) { + public ValidatorConfig(ConfigFactory configFactory, GroupByNameFinder groupByNameFinder) { this.configFactory = configFactory; - this.groupCache = groupCache; + this.groupByNameFinder = groupByNameFinder; } public boolean isEnabledForRef( @@ -121,7 +127,7 @@ return matchCriteria(config, "ref", ref, true, true); } - private boolean activeForEmail(PluginConfig config, String email) { + private boolean activeForEmail(PluginConfig config, @Nullable String email) { return matchCriteria(config, "email", email, true, false); } @@ -134,15 +140,22 @@ } private boolean matchCriteria( - PluginConfig config, String criteria, String value, boolean allowRegex, boolean refMatcher) { - boolean match = true; - for (String s : config.getStringList(criteria)) { - if ((allowRegex && match(value, s, refMatcher)) || (!allowRegex && s.equals(value))) { - return true; - } - match = false; + PluginConfig config, + String criteria, + @Nullable String value, + boolean allowRegex, + boolean refMatcher) { + String[] c = config.getStringList(criteria); + if (c.length == 0) { + return true; } - return match; + if (value == null) { + return false; + } + if (allowRegex) { + return Arrays.stream(c).anyMatch(s -> match(value, s, refMatcher)); + } + return Arrays.asList(c).contains(value); } private static boolean match(String value, String pattern, boolean refMatcher) { @@ -163,10 +176,32 @@ } private AccountGroup.UUID groupUUID(String groupNameOrUUID) { - AccountGroup group = groupCache.get(new AccountGroup.NameKey(groupNameOrUUID)); - if (group == null) { - return new AccountGroup.UUID(groupNameOrUUID); + Optional<InternalGroup> group = + groupByNameFinder.get(new AccountGroup.NameKey(groupNameOrUUID)); + return group.map(InternalGroup::getGroupUUID).orElse(new AccountGroup.UUID(groupNameOrUUID)); + } + + interface GroupByNameFinder { + Optional<InternalGroup> get(AccountGroup.NameKey groupName); + } + + static class GroupByNameFromIndexFinder implements GroupByNameFinder { + + private final Provider<InternalGroupQuery> groupQueryProvider; + + @Inject + GroupByNameFromIndexFinder(Provider<InternalGroupQuery> groupQueryProvider) { + this.groupQueryProvider = groupQueryProvider; } - return group.getGroupUUID(); + + @Override + public Optional<InternalGroup> get(AccountGroup.NameKey groupName) { + try { + return groupQueryProvider.get().byName(groupName); + } catch (OrmException e) { + log.warn(String.format("Cannot lookup group %s by name", groupName.get()), e); + } + return Optional.empty(); + } } }
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md index 5e4fdf3..67da3ad 100644 --- a/src/main/resources/Documentation/about.md +++ b/src/main/resources/Documentation/about.md
@@ -12,5 +12,6 @@ - reject submodules - required footers - maximum allowed path length +- allowing committer and author email addresses Pushes of commits that violate these settings are rejected by Gerrit.
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md index 9c20e5c..20b8cd1 100644 --- a/src/main/resources/Documentation/build.md +++ b/src/main/resources/Documentation/build.md
@@ -60,15 +60,6 @@ ln -s ../../@PLUGIN@ . ``` -Put the external dependency Bazel build file into the Gerrit /plugins -directory, replacing the existing empty one. - -``` - cd gerrit/plugins - rm external_plugin_deps.bzl - ln -s @PLUGIN@/external_plugin_deps.bzl . -``` - From Gerrit source tree issue the command: ```
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 04c04a1..95c6775 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -32,156 +32,189 @@ rejectSubmodule = false rejectDuplicatePathnames = false rejectDuplicatePathnamesLocale = en + allowedAuthorEmailPattern = .*@example\\.com$ + allowedAuthorEmailPattern = admin@example\\.com + allowedCommitterEmailPattern = .*gerrit\\.com + allowedCommitterEmailPattern = admin@gerrit\\..* ``` -plugin.@PLUGIN@.blockedFileExtension -: File extension to be blocked. +plugin.@PLUGIN@.allowedAuthorEmailPattern +: Author Email to Allow. - The values for this check are case insensitive. You can define the - blocked file extensions with or without a leading dot. This check - only test if the filename ends with one of the defined values. + The check looks for a match based on the described specifics. + If there are no matches the push will be rejected. + + Note that all email addresses contain the dot character, and if + included in the pattern needs to be properly escaped as shown in + the examples. + + This check is using `java.util.regex.Pattern` which is described + [here][1]. + +plugin.@PLUGIN@.allowedCommitterEmailPattern +: Committer Email to Allow. + + The check looks for a match based on the described specifics. + If there are no matches the push will be rejected. + + Note that all email addresses contain the dot character, and if + included in the pattern needs to be properly escaped as shown in + the examples. + + This check is using `java.util.regex.Pattern` which is described + [here][1]. + +plugin.@PLUGIN@.blockedFileExtension +: File extension to be blocked. + + The values for this check are case insensitive. You can define the + blocked file extensions with or without a leading dot. This check + only test if the filename ends with one of the defined values. plugin.@PLUGIN@.requiredFooter -: Footer that is required. +: Footer that is required. - This is the footer in the commit message. + This is the footer in the commit message. plugin.@PLUGIN@.maxPathLength -: Maximum allowed path length. '0' means no limit. +: Maximum allowed path length. '0' means no limit. - Defaults to '0'. + Defaults to '0'. plugin.@PLUGIN@.invalidFilenamePattern -: Patterns for invalid filenames. +: Patterns for invalid filenames. - This check is using `java.util.regex.Pattern` which is described - [here][1]. + This check is using `java.util.regex.Pattern` which is described + [here][1]. plugin.@PLUGIN@.rejectWindowsLineEndings -: Reject Windows line endings. +: Reject Windows line endings. - This check looks for carriage return (CR) characters in pushed - files. If the check finds a carriage return (CR) character - the push will be rejected. + This check looks for carriage return (CR) characters in pushed + files. If the check finds a carriage return (CR) character + the push will be rejected. - This check does not run on [binary files][4] + This check does not run on [binary files][4] - The default value is false. This means the check will not be executed. + The default value is false. This means the check will not be executed. <a name="binary_type"> plugin.@PLUGIN@.binaryType -: Binary types. +: Binary types. - Some checks should not run on binary files (e. g. InvalidLineEndingCheck). - Using this option it is possible to configure which content types are - considered binary types. + Some checks should not run on binary files (e. g. InvalidLineEndingCheck). + Using this option it is possible to configure which content types are + considered binary types. - To detect content types [Apache Tika library][2] is used. + If there is a NUL byte in the first 8k then the file will be considered + binary regardless of this setting. - Content type can be specified as a string, wildcard or a regular expression, - for example: + To detect content types, the [MimeUtil2 library][2] is used. - - application/zip - - application/* - - ^application/(pdf|xml) + Content type can be specified as a string, wildcard or a regular expression, + for example: - As usual, the '^' prefix is used to denote that the value is a regular - expression. + - application/zip + - application/* + - ^application/(pdf|xml) - Full list of supported content types can be found [here][3]. + As usual, the '^' prefix is used to denote that the value is a regular + expression. + + Full list of supported content types can be found [here][3]. plugin.@PLUGIN@.rejectSymlink -: Reject symbolic links. +: Reject symbolic links. - This check looks for symbolic links in the set of pushed files. If - the check finds a symbolic link the push will be rejected. + This check looks for symbolic links in the set of pushed files. If + the check finds a symbolic link the push will be rejected. - The default value is false. This means the check will not be executed. + The default value is false. This means the check will not be executed. plugin.@PLUGIN@.rejectSubmodule -: Reject submodules. +: Reject submodules. - This check looks for submodules in the set of pushed commits. If - the check finds a submodule the push will be rejected. + This check looks for submodules in the set of pushed commits. If + the check finds a submodule the push will be rejected. - The default value is false. This means the check will not be executed. + The default value is false. This means the check will not be executed. plugin.@PLUGIN@.blockedKeywordPattern -: Patterns for blocked keywords. +: Patterns for blocked keywords. - This check looks for blocked keywords in files. If the check finds an - blocked keyword the push will be rejected. + This check looks for blocked keywords in files. If the check finds an + blocked keyword the push will be rejected. - To find a keyword it is possible to pass a regular expressions by - blockedKeywordPattern. + To find a keyword it is possible to pass a regular expressions by + blockedKeywordPattern. - To detect blocked keywords, this check is using - `java.util.regex.Pattern` which is described [here][1]. + To detect blocked keywords, this check is using + `java.util.regex.Pattern` which is described [here][1]. - This check does not run on [binary files][4] + This check does not run on [binary files][4] [1]: https://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html -[2]: https://tika.apache.org/ -[3]: https://tika.apache.org/1.12/formats.html#Full_list_of_Supported_Formats +[2]: https://mvnrepository.com/artifact/eu.medsea.mimeutil/mime-util +[3]: https://gerrit.googlesource.com/gerrit/+/refs/heads/master/gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties [4]: #binary_type plugin.@PLUGIN@.blockedContentType -: Blocked content type. +: Blocked content type. - This check looks for blocked content types. If the check finds a - blocked content type the push will be rejected. + This check looks for blocked content types. If the check finds a + blocked content type the push will be rejected. - To detect content types [Apache Tika library][2] is used. + To detect content types the [MimeUtil2 library][2] is used. - Content type can be specified as a string, wildcard or a regular expression, - for example: + Content type can be specified as a string, wildcard or a regular expression, + for example: - - application/zip - - application/* - - ^application/(pdf|xml) + - application/zip + - application/* + - ^application/(pdf|xml) - As usual, the '^' prefix is used to denote that the value is a regular - expression. + As usual, the '^' prefix is used to denote that the value is a regular + expression. - Full list of supported content types can be found [here][3]. + Full list of supported content types can be found [here][3]. plugin.@PLUGIN@.blockedContentTypeWhitelist -: Blocked content type whitelist. +: Blocked content type whitelist. - If this option is checked, the entered content types are interpreted as - a whitelist. Otherwise the entered content types are interpreted as a - blacklist and commits that contains one of these content types will be - rejected. + If this option is checked, the entered content types are interpreted as + a whitelist. Otherwise the entered content types are interpreted as a + blacklist and commits that contains one of these content types will be + rejected. - There must be specified at least one blocked content type pattern, - otherwise this option will be ignored. + There must be specified at least one blocked content type pattern, + otherwise this option will be ignored. - The default value is false. This means the entered content types are - interpreted as a blacklist. + The default value is false. This means the entered content types are + interpreted as a blacklist. plugin.@PLUGIN@.rejectDuplicatePathnames -: Reject duplicate pathnames. +: Reject duplicate pathnames. - This check looks for duplicate pathnames which only differ in case - in the tree of the commit as these can cause problems on case - insensitive filesystems commonly used e.g. on Windows or Mac. If the - check finds duplicate pathnames the push will be rejected. + This check looks for duplicate pathnames which only differ in case + in the tree of the commit as these can cause problems on case + insensitive filesystems commonly used e.g. on Windows or Mac. If the + check finds duplicate pathnames the push will be rejected. - The default value is false. This means duplicate pathnames ignoring - case are allowed. + The default value is false. This means duplicate pathnames ignoring + case are allowed. plugin.@PLUGIN@.rejectDuplicatePathnamesLocale -: Reject duplicate pathnames locale. +: Reject duplicate pathnames locale. - When the validator checks for duplicate pathnames it will convert - the pathnames to lower case. In some cases this leads to a [problem][5]. + When the validator checks for duplicate pathnames it will convert + the pathnames to lower case. In some cases this leads to a [problem][5]. - To avoid these kind of problems, this option is used to specify a - locale which is used when converting a pathname to lower case. + To avoid these kind of problems, this option is used to specify a + locale which is used when converting a pathname to lower case. - Full list of supported locales can be found [here][6]. + Full list of supported locales can be found [here][6]. - The default value is "en" (English). + The default value is "en" (English). [5]: http://bugs.java.com/view_bug.do?bug_id=6208680 [6]: http://www.oracle.com/technetwork/java/javase/javase7locales-334809.html @@ -267,47 +300,47 @@ Skip of the rules is controlled by: plugin.@PLUGIN@.skipGroup -: Group names or UUIDs allowed to skip the rules. +: Group names or UUIDs allowed to skip the rules. - Groups that are allowed to skip the rules. + Groups that are allowed to skip the rules. - Multiple values are supported. - Default: nobody is allowed to skip the rules (empty). + Multiple values are supported. + Default: nobody is allowed to skip the rules (empty). - NOTE: When skipGroup isn't defined, all the other skip settings are ignored. + NOTE: When skipGroup isn't defined, all the other skip settings are ignored. plugin.@PLUGIN@.skipRef -: Ref name, pattern or regexp of the branch to skip. +: Ref name, pattern or regexp of the branch to skip. - List of specific ref names, ref patterns, or regular expressions - of the branches where Groups defined in skipGroup are allowed to - skip the rules. + List of specific ref names, ref patterns, or regular expressions + of the branches where Groups defined in skipGroup are allowed to + skip the rules. - Multiple values are supported. - Default: skip validation on all branches for commits pushed by a member of - a group listed in skipGroup + Multiple values are supported. + Default: skip validation on all branches for commits pushed by a member of + a group listed in skipGroup plugin.@PLUGIN@.skipValidation -: Specific validation to be skipped. +: Specific validation to be skipped. - List of specific validation operations allowed to be skipped by - the Groups defined in skipGroup on the branches defined in skipRef. + List of specific validation operations allowed to be skipped by + the Groups defined in skipGroup on the branches defined in skipRef. - Validations can be one of the following strings: + Validations can be one of the following strings: - - blockedContentType - - blockedFileExtension - - blockedKeyword - - invalidFilename - - maxPathLength - - rejectDuplicatePathnames - - rejectSubmodule - - rejectSymlink - - rejectWindowsLineEndings - - requiredFooter + - blockedContentType + - blockedFileExtension + - blockedKeyword + - invalidFilename + - maxPathLength + - rejectDuplicatePathnames + - rejectSubmodule + - rejectSymlink + - rejectWindowsLineEndings + - requiredFooter - Multiple values are supported. - Default: groups defined at skipGroup can skip all the validation rules. + Multiple values are supported. + Default: groups defined at skipGroup can skip all the validation rules. NOTE: Skip of the validations are inherited from parent projects. The definition of the skip criteria on All-Projects automatically apply to every project.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java index 609cf72..20f2e83 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java
@@ -32,6 +32,7 @@ import java.util.regex.Pattern; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; public class BlockedKeywordValidatorTest extends ValidatorTestCase { @@ -43,7 +44,7 @@ .build(); } - private RevCommit makeCommit() throws IOException, GitAPIException { + private RevCommit makeCommit(RevWalk rw) throws IOException, GitAPIException { Map<File, byte[]> files = new HashMap<>(); // invalid files String content = "http://foo.bar.tld/?pw=myp4ssw0rdTefoobarstline2\n"; @@ -66,26 +67,33 @@ files.put( new File(repo.getDirectory().getParent(), "foobar.txt"), content.getBytes(StandardCharsets.UTF_8)); - return TestUtils.makeCommit(repo, "Commit foobar with test files.", files); + return TestUtils.makeCommit(rw, repo, "Commit foobar with test files.", files); } @Test public void testKeywords() throws Exception { - RevCommit c = makeCommit(); - BlockedKeywordValidator validator = - new BlockedKeywordValidator( - null, new ContentTypeUtil(PATTERN_CACHE), PATTERN_CACHE, null, null, null); - List<CommitValidationMessage> m = - validator.performValidation(repo, c, getPatterns().values(), EMPTY_PLUGIN_CONFIG); - Set<String> expected = - ImmutableSet.of( - "ERROR: blocked keyword(s) found in: foo.txt (Line: 1)" - + " (found: myp4ssw0rd, foobar)", - "ERROR: blocked keyword(s) found in: bar.txt (Line: 5)" + " (found: $Id: foo bar$)", - "ERROR: blocked keyword(s) found in: " - + Patch.COMMIT_MSG - + " (Line: 1) (found: foobar)"); - assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommit(rw); + BlockedKeywordValidator validator = + new BlockedKeywordValidator( + null, + new ContentTypeUtil(PATTERN_CACHE, new FakeMimeUtilFileTypeRegistry()), + PATTERN_CACHE, + null, + null, + null); + List<CommitValidationMessage> m = + validator.performValidation(repo, c, rw, getPatterns().values(), EMPTY_PLUGIN_CONFIG); + Set<String> expected = + ImmutableSet.of( + "ERROR: blocked keyword(s) found in: foo.txt (Line: 1)" + + " (found: myp4ssw0rd, foobar)", + "ERROR: blocked keyword(s) found in: bar.txt (Line: 5)" + " (found: $Id: foo bar$)", + "ERROR: blocked keyword(s) found in: " + + Patch.COMMIT_MSG + + " (Line: 1) (found: foobar)"); + assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); + } } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailTest.java new file mode 100644 index 0000000..0b208f2 --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ChangeEmailTest.java
@@ -0,0 +1,81 @@ +// Copyright (C) 2018 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.uploadvalidator; + +import static com.google.common.truth.Truth.assertThat; +import static com.googlesource.gerrit.plugins.uploadvalidator.TestUtils.EMPTY_PLUGIN_CONFIG; + +import org.junit.Test; + +public class ChangeEmailTest { + private static final String[] allowedEmailPatterns = { + ".*@example\\.com.*", + "testing\\.com", + "tester@testing\\.com", + ".*google\\.com", + "tester@gerrit\\..*" + }; + + @Test + public void testEmailValid() throws Exception { + assertThat( + ChangeEmailValidator.performValidation("tester@example.com.net", allowedEmailPatterns)) + .isTrue(); + assertThat(ChangeEmailValidator.performValidation("tester@testing.com", allowedEmailPatterns)) + .isTrue(); + assertThat(ChangeEmailValidator.performValidation("tester@google.com", allowedEmailPatterns)) + .isTrue(); + assertThat(ChangeEmailValidator.performValidation("tester@gerrit.net", allowedEmailPatterns)) + .isTrue(); + } + + @Test + public void testEmailInvalid() throws Exception { + assertThat(ChangeEmailValidator.performValidation("tester@example.org", allowedEmailPatterns)) + .isFalse(); + assertThat(ChangeEmailValidator.performValidation("test@testing.com", allowedEmailPatterns)) + .isFalse(); + assertThat( + ChangeEmailValidator.performValidation("tester@google.com.net", allowedEmailPatterns)) + .isFalse(); + assertThat( + ChangeEmailValidator.performValidation( + "emailtester@gerritnet.com", allowedEmailPatterns)) + .isFalse(); + } + + @Test + public void testEmailNull() throws Exception { + assertThat(ChangeEmailValidator.performValidation(null, allowedEmailPatterns)).isFalse(); + } + + @Test + public void testEmailEmpty() throws Exception { + assertThat(ChangeEmailValidator.performValidation("", allowedEmailPatterns)).isFalse(); + } + + @Test + public void validatorBehaviorWhenAuthorConfigEmpty() { + assertThat(ChangeEmailValidator.isAuthorActive(EMPTY_PLUGIN_CONFIG)).isFalse(); + assertThat(ChangeEmailValidator.getAllowedAuthorEmailPatterns(EMPTY_PLUGIN_CONFIG)).isEmpty(); + } + + @Test + public void validatorBehaviorWhenCommitterConfigEmpty() { + assertThat(ChangeEmailValidator.isCommitterActive(EMPTY_PLUGIN_CONFIG)).isFalse(); + assertThat(ChangeEmailValidator.getAllowedCommitterEmailPatterns(EMPTY_PLUGIN_CONFIG)) + .isEmpty(); + } +}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeUtilTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeUtilTest.java index 91df4c6..a8e2713 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeUtilTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeUtilTest.java
@@ -18,16 +18,19 @@ import static com.googlesource.gerrit.plugins.uploadvalidator.TestUtils.EMPTY_PLUGIN_CONFIG; import static com.googlesource.gerrit.plugins.uploadvalidator.TestUtils.PATTERN_CACHE; +import com.google.gerrit.server.mime.MimeUtilFileTypeRegistry; +import com.google.inject.Inject; import java.util.concurrent.ExecutionException; import org.junit.Before; import org.junit.Test; public class ContentTypeUtilTest { private ContentTypeUtil ctu; + @Inject private MimeUtilFileTypeRegistry mimeUtil; @Before public void setUp() { - ctu = new ContentTypeUtil(PATTERN_CACHE); + ctu = new ContentTypeUtil(PATTERN_CACHE, mimeUtil); } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidatorTest.java index 96168f2..b57ef49 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ContentTypeValidatorTest.java
@@ -27,6 +27,7 @@ import java.util.Map; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Before; import org.junit.Test; @@ -59,30 +60,39 @@ @Before public void setUp() { validator = - new ContentTypeValidator(null, new ContentTypeUtil(PATTERN_CACHE), null, null, null); + new ContentTypeValidator( + null, + new ContentTypeUtil(PATTERN_CACHE, new FakeMimeUtilFileTypeRegistry()), + null, + null, + null); } @Test public void testBlocked() throws Exception { String[] patterns = new String[] {"application/pdf", "application/xml", "text/html"}; - List<CommitValidationMessage> m = - validator.performValidation(repo, makeCommit(), patterns, false); - assertThat(TestUtils.transformMessages(m)) - .containsExactly( - "ERROR: found blocked content type (application/pdf) in file: foo.pdf", - "ERROR: found blocked content type (application/xml) in file: foo.xml", - "ERROR: found blocked content type (text/html) in file: foo.html"); + try (RevWalk rw = new RevWalk(repo)) { + List<CommitValidationMessage> m = + validator.performValidation(repo, makeCommit(rw), rw, patterns, false); + assertThat(TestUtils.transformMessages(m)) + .containsExactly( + "ERROR: found blocked content type (application/pdf) in file: foo.pdf", + "ERROR: found blocked content type (application/xml) in file: foo.xml", + "ERROR: found blocked content type (text/html) in file: foo.html"); + } } @Test public void testWhitelist() throws Exception { String[] patterns = new String[] {"application/pdf", "application/xml"}; - List<CommitValidationMessage> m = - validator.performValidation(repo, makeCommit(), patterns, true); - assertThat(TestUtils.transformMessages(m)) - .containsExactly("ERROR: found blocked content type (text/html) in file: foo.html"); + try (RevWalk rw = new RevWalk(repo)) { + List<CommitValidationMessage> m = + validator.performValidation(repo, makeCommit(rw), rw, patterns, true); + assertThat(TestUtils.transformMessages(m)) + .containsExactly("ERROR: found blocked content type (text/html) in file: foo.html"); + } } @Test @@ -91,7 +101,7 @@ assertThat(ContentTypeValidator.isWhitelist(EMPTY_PLUGIN_CONFIG)).isFalse(); } - private RevCommit makeCommit() throws IOException, GitAPIException { + private RevCommit makeCommit(RevWalk rw) throws IOException, GitAPIException { Map<File, byte[]> files = new HashMap<>(); String content = "<?xml version=\"1.0\"?><a><b>c</b></a>"; @@ -102,6 +112,6 @@ TestUtils.createEmptyFile("foo.html", repo), content.getBytes(StandardCharsets.UTF_8)); files.put(TestUtils.createEmptyFile("foo.pdf", repo), TEST_PDF); - return TestUtils.makeCommit(repo, "Commit with test files.", files); + return TestUtils.makeCommit(rw, repo, "Commit with test files.", files); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidatorTest.java index cafce5a..4b787b9 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/DuplicatePathnameValidatorTest.java
@@ -36,6 +36,7 @@ import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.TreeWalk; import org.junit.Before; import org.junit.Test; @@ -57,7 +58,11 @@ List<CommitValidationMessage> messages, List<String> visitedPaths) throws Exception { - RevCommit c = makeCommit(createEmptyDirCacheEntries(existingTreePaths, testRepo), testRepo); + RevCommit c = + makeCommit( + testRepo.getRevWalk(), + createEmptyDirCacheEntries(existingTreePaths, testRepo), + testRepo); try (TreeWalk tw = new TreeWalk(repo)) { tw.setRecursive(false); tw.addTree(c.getTree()); @@ -130,41 +135,47 @@ filenames.add("A"); filenames.add("F1/ab"); filenames.add("f2/sF1/aB"); - RevCommit c = makeCommit(createEmptyDirCacheEntries(filenames, testRepo), testRepo); - List<CommitValidationMessage> m = validator.performValidation(repo, c); - assertThat(m).hasSize(4); - // During checking inside of the commit it's unknown which file is checked - // first, because of that, both capabilities must be checked. - assertThat(transformMessages(m)) - .containsAnyOf(transformMessage(conflict("A", "a")), transformMessage(conflict("a", "A"))); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommit(rw, createEmptyDirCacheEntries(filenames, testRepo), testRepo); + List<CommitValidationMessage> m = validator.performValidation(repo, c, rw); + assertThat(m).hasSize(4); + // During checking inside of the commit it's unknown which file is checked + // first, because of that, both capabilities must be checked. + assertThat(transformMessages(m)) + .containsAnyOf( + transformMessage(conflict("A", "a")), transformMessage(conflict("a", "A"))); - assertThat(transformMessages(m)) - .containsAnyOf( - transformMessage(conflict("F1", "f1")), transformMessage(conflict("f1", "F1"))); + assertThat(transformMessages(m)) + .containsAnyOf( + transformMessage(conflict("F1", "f1")), transformMessage(conflict("f1", "F1"))); - assertThat(transformMessages(m)) - .containsAnyOf( - transformMessage(conflict("F1/ab", "f1/ab")), - transformMessage(conflict("f1/ab", "F1/ab"))); + assertThat(transformMessages(m)) + .containsAnyOf( + transformMessage(conflict("F1/ab", "f1/ab")), + transformMessage(conflict("f1/ab", "F1/ab"))); - assertThat(transformMessages(m)) - .containsAnyOf( - transformMessage(conflict("f2/sF1/aB", "f2/sF1/ab")), - transformMessage(conflict("f2/sF1/ab", "f2/sF1/aB"))); + assertThat(transformMessages(m)) + .containsAnyOf( + transformMessage(conflict("f2/sF1/aB", "f2/sF1/ab")), + transformMessage(conflict("f2/sF1/ab", "f2/sF1/aB"))); + } } @Test public void testCheckRenaming() throws Exception { - RevCommit c = makeCommit(createEmptyDirCacheEntries(INITIAL_PATHNAMES, testRepo), testRepo); - DirCacheEntry[] entries = new DirCacheEntry[INITIAL_PATHNAMES.size()]; - for (int x = 0; x < INITIAL_PATHNAMES.size(); x++) { - // Rename files - entries[x] = - createDirCacheEntry(INITIAL_PATHNAMES.get(x).toUpperCase(), EMPTY_CONTENT, testRepo); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = + makeCommit(rw, createEmptyDirCacheEntries(INITIAL_PATHNAMES, testRepo), testRepo); + DirCacheEntry[] entries = new DirCacheEntry[INITIAL_PATHNAMES.size()]; + for (int x = 0; x < INITIAL_PATHNAMES.size(); x++) { + // Rename files + entries[x] = + createDirCacheEntry(INITIAL_PATHNAMES.get(x).toUpperCase(), EMPTY_CONTENT, testRepo); + } + RevCommit c1 = makeCommit(rw, entries, testRepo, c); + List<CommitValidationMessage> m = validator.performValidation(repo, c1, rw); + assertThat(m).isEmpty(); } - RevCommit c1 = makeCommit(entries, testRepo, c); - List<CommitValidationMessage> m = validator.performValidation(repo, c1); - assertThat(m).isEmpty(); } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java index 09cf4e8..8bdb16c 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java
@@ -35,6 +35,16 @@ } @Test + public void isEnabledForMissingEmailByDefault() throws Exception { + IdentifiedUser missingEmail = new FakeUserProvider().get(null); + ValidatorConfig config = + getConfig("[plugin \"uploadvalidator\"]\n" + "blockedFileExtension = jar"); + + assertThat(config.isEnabledForRef(missingEmail, projectName, "anyRef", "blockedFileExtension")) + .isTrue(); + } + + @Test public void isEnabledForSingleEmail() throws Exception { ValidatorConfig config = getConfig( @@ -84,6 +94,21 @@ } @Test + public void missingEmailDoesNotMatchRegex() throws Exception { + IdentifiedUser missingEmail = new FakeUserProvider().get(null); + ValidatorConfig config = + getConfig( + "[plugin \"uploadvalidator\"]\n" + + " email = .*$\n" + + " blockedFileExtension = jar"); + + assertThat( + config.isEnabledForRef( + missingEmail, projectName, "refs/heads/anyref", "blockedFileExtension")) + .isFalse(); + } + + @Test public void isEnabledForMultipleEmails() throws Exception { IdentifiedUser exampleOrgUser = new FakeUserProvider().get("a@example.org"); IdentifiedUser xUser = new FakeUserProvider().get("x@example.com"); @@ -108,9 +133,7 @@ } private ValidatorConfig getConfig(String defaultConfig) throws ConfigInvalidException { - ValidatorConfig config = - new ValidatorConfig( - new FakeConfigFactory(projectName, defaultConfig), new FakeGroupCacheUUIDByName()); - return config; + return new ValidatorConfig( + new FakeConfigFactory(projectName, defaultConfig), new FakeGroupByNameFinder()); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java new file mode 100644 index 0000000..69f1b74 --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java
@@ -0,0 +1,40 @@ +// Copyright (C) 2017 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.uploadvalidator; + +import com.google.common.collect.ImmutableSet; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.server.group.InternalGroup; +import java.util.Objects; +import java.util.Optional; + +public class FakeGroupByNameFinder implements ValidatorConfig.GroupByNameFinder { + + private final Optional<InternalGroup> onlyGroup; + + public FakeGroupByNameFinder() { + onlyGroup = Optional.empty(); + } + + public FakeGroupByNameFinder(AccountGroup accountGroup) { + onlyGroup = + Optional.of(InternalGroup.create(accountGroup, ImmutableSet.of(), ImmutableSet.of())); + } + + @Override + public Optional<InternalGroup> get(AccountGroup.NameKey groupName) { + return onlyGroup.filter(group -> Objects.equals(group.getNameKey(), groupName)); + } +}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupCacheUUIDByName.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupCacheUUIDByName.java deleted file mode 100644 index 02daaf1..0000000 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupCacheUUIDByName.java +++ /dev/null
@@ -1,64 +0,0 @@ -// Copyright (C) 2017 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.googlesource.gerrit.plugins.uploadvalidator; - -import com.google.common.collect.ImmutableList; -import com.google.gerrit.reviewdb.client.AccountGroup; -import com.google.gerrit.reviewdb.client.AccountGroup.Id; -import com.google.gerrit.reviewdb.client.AccountGroup.NameKey; -import com.google.gerrit.reviewdb.client.AccountGroup.UUID; -import com.google.gerrit.server.account.GroupCache; -import java.io.IOException; - -public class FakeGroupCacheUUIDByName implements GroupCache { - private AccountGroup accountGroup; - - public FakeGroupCacheUUIDByName(AccountGroup accountGroup) { - this.accountGroup = accountGroup; - } - - public FakeGroupCacheUUIDByName() { - // TODO Auto-generated constructor stub - } - - @Override - public AccountGroup get(Id groupId) { - return null; - } - - @Override - public AccountGroup get(NameKey name) { - return accountGroup != null && accountGroup.getNameKey().equals(name) ? accountGroup : null; - } - - @Override - public AccountGroup get(UUID uuid) { - return null; - } - - @Override - public ImmutableList<AccountGroup> all() { - return null; - } - - @Override - public void onCreateGroup(NameKey newGroupName) throws IOException {} - - @Override - public void evict(AccountGroup group) throws IOException {} - - @Override - public void evictAfterRename(NameKey oldName, NameKey newName) throws IOException {} -}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeMimeUtilFileTypeRegistry.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeMimeUtilFileTypeRegistry.java new file mode 100644 index 0000000..d1d50bc --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeMimeUtilFileTypeRegistry.java
@@ -0,0 +1,57 @@ +// Copyright (C) 2017 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.uploadvalidator; + +import com.google.gerrit.server.mime.FileTypeRegistry; +import com.google.inject.Singleton; +import eu.medsea.mimeutil.MimeType; +import java.io.InputStream; + +@Singleton +class FakeMimeUtilFileTypeRegistry implements FileTypeRegistry { + + @Override + public MimeType getMimeType(String path, byte[] content) { + if (path.endsWith(".pdf")) { + return new MimeType("application/pdf"); + } + if (path.endsWith(".xml")) { + return new MimeType("application/xml"); + } + if (path.endsWith(".html")) { + return new MimeType("text/html"); + } + return new MimeType("application/octet-stream"); + } + + @Override + public MimeType getMimeType(String path, InputStream is) { + if (path.endsWith(".pdf")) { + return new MimeType("application/pdf"); + } + if (path.endsWith(".xml")) { + return new MimeType("application/xml"); + } + if (path.endsWith(".html")) { + return new MimeType("text/html"); + } + return new MimeType("application/octet-stream"); + } + + @Override + public boolean isSafeInline(MimeType type) { + return false; + } +}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidatorTest.java index ed09b61..65bc8fb 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FileExtensionValidatorTest.java
@@ -28,6 +28,7 @@ import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.NoFilepatternException; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; public class FileExtensionValidatorTest extends ValidatorTestCase { @@ -36,7 +37,7 @@ private static final List<String> BLOCKED_EXTENSIONS_UC = Lists.newArrayList("JPEG", "PDF", "ZIP", "EXE", "TAR.GZ"); - private RevCommit makeCommit(List<String> blockedExtensions) + private RevCommit makeCommit(RevWalk rw, List<String> blockedExtensions) throws NoFilepatternException, IOException, GitAPIException { Set<File> files = new HashSet<>(); for (String extension : blockedExtensions) { @@ -47,31 +48,35 @@ files.add(new File(repo.getDirectory().getParent(), "foo.core.tmp")); files.add(new File(repo.getDirectory().getParent(), "foo.c")); files.add(new File(repo.getDirectory().getParent(), "foo.txt")); - return TestUtils.makeCommit(repo, "Commit with empty test files.", files); + return TestUtils.makeCommit(rw, repo, "Commit with empty test files.", files); } @Test public void testBlockedExtensions() throws Exception { - RevCommit c = makeCommit(BLOCKED_EXTENSIONS_LC); - List<CommitValidationMessage> m = - FileExtensionValidator.performValidation(repo, c, BLOCKED_EXTENSIONS_LC); - List<String> expected = new ArrayList<>(); - for (String extension : BLOCKED_EXTENSIONS_LC) { - expected.add("ERROR: blocked file: foo." + extension); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommit(rw, BLOCKED_EXTENSIONS_LC); + List<CommitValidationMessage> m = + FileExtensionValidator.performValidation(repo, c, rw, BLOCKED_EXTENSIONS_LC); + List<String> expected = new ArrayList<>(); + for (String extension : BLOCKED_EXTENSIONS_LC) { + expected.add("ERROR: blocked file: foo." + extension); + } + assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); } - assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); } @Test public void testBlockedExtensionsCaseInsensitive() throws Exception { - RevCommit c = makeCommit(BLOCKED_EXTENSIONS_UC); - List<CommitValidationMessage> m = - FileExtensionValidator.performValidation(repo, c, BLOCKED_EXTENSIONS_LC); - List<String> expected = new ArrayList<>(); - for (String extension : BLOCKED_EXTENSIONS_UC) { - expected.add("ERROR: blocked file: foo." + extension); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommit(rw, BLOCKED_EXTENSIONS_UC); + List<CommitValidationMessage> m = + FileExtensionValidator.performValidation(repo, c, rw, BLOCKED_EXTENSIONS_LC); + List<String> expected = new ArrayList<>(); + for (String extension : BLOCKED_EXTENSIONS_UC) { + expected.add("ERROR: blocked file: foo." + extension); + } + assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); } - assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidatorTest.java index 45003b7..a84ad32 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidFilenameValidatorTest.java
@@ -25,6 +25,7 @@ import java.util.Set; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; public class InvalidFilenameValidatorTest extends ValidatorTestCase { @@ -40,27 +41,29 @@ return filenames; } - private RevCommit makeCommit() throws IOException, GitAPIException { + private RevCommit makeCommit(RevWalk rw) throws IOException, GitAPIException { Set<File> files = new HashSet<>(); for (String filenames : getInvalidFilenames()) { files.add(new File(repo.getDirectory().getParent(), filenames)); } // valid filename files.add(new File(repo.getDirectory().getParent(), "test")); - return TestUtils.makeCommit(repo, "Commit with empty test files.", files); + return TestUtils.makeCommit(rw, repo, "Commit with empty test files.", files); } @Test public void test() throws Exception { String[] invalidFilenamePattern = {"\\[|\\]|\\*|#", "[%:@]"}; - RevCommit c = makeCommit(); - List<CommitValidationMessage> m = - InvalidFilenameValidator.performValidation(repo, c, invalidFilenamePattern); - Set<String> expected = new HashSet<>(); - for (String filenames : getInvalidFilenames()) { - expected.add("ERROR: invalid characters found in filename: " + filenames); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommit(rw); + List<CommitValidationMessage> m = + InvalidFilenameValidator.performValidation(repo, c, rw, invalidFilenamePattern); + Set<String> expected = new HashSet<>(); + for (String filenames : getInvalidFilenames()) { + expected.add("ERROR: invalid characters found in filename: " + filenames); + } + assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); } - assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidatorTest.java index 9b35e7a..17209d2 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/InvalidLineEndingValidatorTest.java
@@ -27,10 +27,11 @@ import java.util.Map; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; public class InvalidLineEndingValidatorTest extends ValidatorTestCase { - private RevCommit makeCommit() throws IOException, GitAPIException { + private RevCommit makeCommit(RevWalk rw) throws IOException, GitAPIException { Map<File, byte[]> files = new HashMap<>(); // invalid line endings String content = "Testline1\r\n" + "Testline2\n" + "Testline3\r\n" + "Testline4"; @@ -43,17 +44,25 @@ files.put( new File(repo.getDirectory().getParent(), "bar.txt"), content.getBytes(StandardCharsets.UTF_8)); - return TestUtils.makeCommit(repo, "Commit with test files.", files); + return TestUtils.makeCommit(rw, repo, "Commit with test files.", files); } @Test public void testCarriageReturn() throws Exception { - RevCommit c = makeCommit(); - InvalidLineEndingValidator validator = - new InvalidLineEndingValidator(null, new ContentTypeUtil(PATTERN_CACHE), null, null, null); - List<CommitValidationMessage> m = validator.performValidation(repo, c, EMPTY_PLUGIN_CONFIG); - assertThat(TestUtils.transformMessages(m)) - .containsExactly("ERROR: found carriage return (CR) character in file: foo.txt"); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommit(rw); + InvalidLineEndingValidator validator = + new InvalidLineEndingValidator( + null, + new ContentTypeUtil(PATTERN_CACHE, new FakeMimeUtilFileTypeRegistry()), + null, + null, + null); + List<CommitValidationMessage> m = + validator.performValidation(repo, c, rw, EMPTY_PLUGIN_CONFIG); + assertThat(TestUtils.transformMessages(m)) + .containsExactly("ERROR: found carriage return (CR) character in file: foo.txt"); + } } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidatorTest.java index 27289be..f4578c9 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/MaxPathLengthValidatorTest.java
@@ -27,6 +27,7 @@ import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; public class MaxPathLengthValidatorTest extends ValidatorTestCase { @@ -37,34 +38,39 @@ return (TOO_LONG.length() + GOOD.length()) / 2; } - private RevCommit makeCommit() throws IOException, GitAPIException { + private RevCommit makeCommit(RevWalk rw) throws IOException, GitAPIException { Set<File> files = new HashSet<>(); files.add(TestUtils.createEmptyFile(TOO_LONG, repo)); files.add(TestUtils.createEmptyFile(GOOD, repo)); - return TestUtils.makeCommit(repo, "Commit with empty test files.", files); + return TestUtils.makeCommit(rw, repo, "Commit with empty test files.", files); } @Test public void testAddTooLongPath() throws Exception { - RevCommit c = makeCommit(); - List<CommitValidationMessage> m = - MaxPathLengthValidator.performValidation(repo, c, getMaxPathLength()); - Set<String> expected = ImmutableSet.of("ERROR: path too long: " + TOO_LONG); - assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommit(rw); + List<CommitValidationMessage> m = + MaxPathLengthValidator.performValidation(repo, c, rw, getMaxPathLength()); + Set<String> expected = ImmutableSet.of("ERROR: path too long: " + TOO_LONG); + assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); + } } @Test public void testDeleteTooLongPath() throws Exception { - RevCommit c = makeCommit(); - try (Git git = new Git(repo)) { - Set<File> files = new HashSet<>(); - files.add(TestUtils.createEmptyFile(TOO_LONG, repo)); - TestUtils.removeFiles(git, files); - c = git.commit().setMessage("Delete file which is too long").call(); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommit(rw); + try (Git git = new Git(repo)) { + Set<File> files = new HashSet<>(); + files.add(TestUtils.createEmptyFile(TOO_LONG, repo)); + TestUtils.removeFiles(git, files); + c = git.commit().setMessage("Delete file which is too long").call(); + rw.parseCommit(c); + } + List<CommitValidationMessage> m = + MaxPathLengthValidator.performValidation(repo, c, rw, getMaxPathLength()); + assertThat(m).isEmpty(); } - List<CommitValidationMessage> m = - MaxPathLengthValidator.performValidation(repo, c, getMaxPathLength()); - assertThat(m).isEmpty(); } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java index 9949778..e6e1be7 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java
@@ -99,9 +99,7 @@ private ValidatorConfig getConfig(String defaultConfig, Project.NameKey projName) throws ConfigInvalidException { - ValidatorConfig config = - new ValidatorConfig( - new FakeConfigFactory(projName, defaultConfig), new FakeGroupCacheUUIDByName()); - return config; + return new ValidatorConfig( + new FakeConfigFactory(projName, defaultConfig), new FakeGroupByNameFinder()); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java index fc9b486..acc2bdc 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java
@@ -104,9 +104,7 @@ } private ValidatorConfig getConfig(String defaultConfig) throws ConfigInvalidException { - ValidatorConfig config = - new ValidatorConfig( - new FakeConfigFactory(projectName, defaultConfig), new FakeGroupCacheUUIDByName()); - return config; + return new ValidatorConfig( + new FakeConfigFactory(projectName, defaultConfig), new FakeGroupByNameFinder()); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java index 2b0f5cd..4942391 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java
@@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.gerrit.common.TimeUtil; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.IdentifiedUser; @@ -28,7 +29,7 @@ @Test public void dontSkipByDefault() throws Exception { ValidatorConfig validatorConfig = - new ValidatorConfig(new FakeConfigFactory(projectName, ""), new FakeGroupCacheUUIDByName()); + new ValidatorConfig(new FakeConfigFactory(projectName, ""), new FakeGroupByNameFinder()); assertThat(validatorConfig.isEnabledForRef(anyUser, projectName, "anyRef", "anyOp")).isTrue(); } @@ -43,7 +44,7 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName()); + new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); assertThat( validatorConfig.isEnabledForRef( @@ -62,11 +63,12 @@ ValidatorConfig validatorConfig = new ValidatorConfig( new FakeConfigFactory(projectName, config), - new FakeGroupCacheUUIDByName( + new FakeGroupByNameFinder( new AccountGroup( new AccountGroup.NameKey("testGroupName"), new AccountGroup.Id(1), - new AccountGroup.UUID("testGroupId")))); + new AccountGroup.UUID("testGroupId"), + TimeUtil.nowTs()))); assertThat( validatorConfig.isEnabledForRef( @@ -84,7 +86,7 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName()); + new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); assertThat( validatorConfig.isEnabledForRef( @@ -102,7 +104,7 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName()); + new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); assertThat(validatorConfig.isEnabledForRef(anyUser, projectName, "anyRef", "anotherOp")) .isTrue(); @@ -118,7 +120,7 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName()); + new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); assertThat( validatorConfig.isEnabledForRef( @@ -136,7 +138,7 @@ ValidatorConfig validatorConfig = new ValidatorConfig( - new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName()); + new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder()); assertThat( validatorConfig.isEnabledForRef(
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidatorTest.java index 11c1651..a21d46f 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SubmoduleValidatorTest.java
@@ -27,39 +27,44 @@ import org.eclipse.jgit.api.SubmoduleAddCommand; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; public class SubmoduleValidatorTest extends ValidatorTestCase { - private RevCommit makeCommitWithSubmodule() throws IOException, GitAPIException { + private RevCommit makeCommitWithSubmodule(RevWalk rw) throws IOException, GitAPIException { try (Git git = new Git(repo)) { SubmoduleAddCommand addCommand = git.submoduleAdd(); addCommand.setURI(repo.getDirectory().getCanonicalPath()); addCommand.setPath("modules/library"); addCommand.call().close(); git.add().addFilepattern(".").call(); - return git.commit().setMessage("Commit with submodule.").call(); + return rw.parseCommit(git.commit().setMessage("Commit with submodule.").call()); } } @Test public void testWithSubmodule() throws Exception { - RevCommit c = makeCommitWithSubmodule(); - List<CommitValidationMessage> m = SubmoduleValidator.performValidation(repo, c); - assertThat(TestUtils.transformMessages(m)) - .containsExactly("ERROR: submodules are not allowed: modules/library"); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommitWithSubmodule(rw); + List<CommitValidationMessage> m = SubmoduleValidator.performValidation(repo, c, rw); + assertThat(TestUtils.transformMessages(m)) + .containsExactly("ERROR: submodules are not allowed: modules/library"); + } } - private RevCommit makeCommitWithoutSubmodule() throws IOException, GitAPIException { + private RevCommit makeCommitWithoutSubmodule(RevWalk rw) throws IOException, GitAPIException { Map<File, byte[]> files = new HashMap<>(); files.put(new File(repo.getDirectory().getParent(), "foo.txt"), null); - return TestUtils.makeCommit(repo, "Commit with empty test files.", files); + return TestUtils.makeCommit(rw, repo, "Commit with empty test files.", files); } @Test public void testWithoutSubmodule() throws Exception { - RevCommit c = makeCommitWithoutSubmodule(); - List<CommitValidationMessage> m = SubmoduleValidator.performValidation(repo, c); - assertThat(m).isEmpty(); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommitWithoutSubmodule(rw); + List<CommitValidationMessage> m = SubmoduleValidator.performValidation(repo, c, rw); + assertThat(m).isEmpty(); + } } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidatorTest.java index fde0b4b..1432c42 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SymlinkValidatorTest.java
@@ -29,10 +29,11 @@ import java.util.Set; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; public class SymlinkValidatorTest extends ValidatorTestCase { - private RevCommit makeCommitWithSymlink() throws IOException, GitAPIException { + private RevCommit makeCommitWithSymlink(RevWalk rw) throws IOException, GitAPIException { Map<File, byte[]> files = new HashMap<>(); File link = new File(repo.getDirectory().getParent(), "foo.txt"); Files.createSymbolicLink(link.toPath(), Paths.get("bar.txt")); @@ -42,31 +43,37 @@ Files.createSymbolicLink(link.toPath(), Paths.get("folder")); files.put(link, null); - return TestUtils.makeCommit(repo, "Commit with symlink.", files); + return TestUtils.makeCommit(rw, repo, "Commit with symlink.", files); } @Test public void testWithSymlink() throws Exception { - RevCommit c = makeCommitWithSymlink(); - List<CommitValidationMessage> m = SymlinkValidator.performValidation(repo, c); - Set<String> expected = - ImmutableSet.of( - "ERROR: Symbolic links are not allowed: foo.txt", - "ERROR: Symbolic links are not allowed: symbolicFolder"); - assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommitWithSymlink(rw); + List<CommitValidationMessage> m = + SymlinkValidator.performValidation(repo, rw.parseCommit(c), rw); + Set<String> expected = + ImmutableSet.of( + "ERROR: Symbolic links are not allowed: foo.txt", + "ERROR: Symbolic links are not allowed: symbolicFolder"); + assertThat(TestUtils.transformMessages(m)).containsExactlyElementsIn(expected); + } } - private RevCommit makeCommitWithoutSymlink() throws IOException, GitAPIException { + private RevCommit makeCommitWithoutSymlink(RevWalk rw) throws IOException, GitAPIException { Map<File, byte[]> files = new HashMap<>(); files.put(new File(repo.getDirectory().getParent(), "foo.txt"), null); - return TestUtils.makeCommit(repo, "Commit with empty test files.", files); + return TestUtils.makeCommit(rw, repo, "Commit with empty test files.", files); } @Test public void testWithoutSymlink() throws Exception { - RevCommit c = makeCommitWithoutSymlink(); - List<CommitValidationMessage> m = SymlinkValidator.performValidation(repo, c); - assertThat(m).isEmpty(); + try (RevWalk rw = new RevWalk(repo)) { + RevCommit c = makeCommitWithoutSymlink(rw); + List<CommitValidationMessage> m = + SymlinkValidator.performValidation(repo, rw.parseCommit(c), rw); + assertThat(m).isEmpty(); + } } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/TestUtils.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/TestUtils.java index a3b9089..22f95f0 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/TestUtils.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/TestUtils.java
@@ -39,6 +39,7 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevTree; +import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; public class TestUtils { @@ -64,22 +65,23 @@ return repository; } - public static RevCommit makeCommit(Repository repo, String message, Set<File> files) + public static RevCommit makeCommit(RevWalk rw, Repository repo, String message, Set<File> files) throws IOException, GitAPIException { Map<File, byte[]> tmp = new HashMap<>(); for (File f : files) { tmp.put(f, null); } - return makeCommit(repo, message, tmp); + return makeCommit(rw, repo, message, tmp); } - public static RevCommit makeCommit(Repository repo, String message, Map<File, byte[]> files) + public static RevCommit makeCommit( + RevWalk rw, Repository repo, String message, Map<File, byte[]> files) throws IOException, GitAPIException { try (Git git = new Git(repo)) { if (files != null) { addFiles(git, files); } - return git.commit().setMessage(message).call(); + return rw.parseCommit(git.commit().setMessage(message).call()); } } @@ -138,17 +140,17 @@ return repo.file(pathname, repo.blob(content)); } - public static RevCommit makeCommit(DirCacheEntry[] entries, TestRepository<Repository> repo) - throws Exception { - return makeCommit(entries, repo, (RevCommit[]) null); + public static RevCommit makeCommit( + RevWalk rw, DirCacheEntry[] entries, TestRepository<Repository> repo) throws Exception { + return makeCommit(rw, entries, repo, (RevCommit[]) null); } public static RevCommit makeCommit( - DirCacheEntry[] entries, TestRepository<Repository> repo, RevCommit... parents) + RevWalk rw, DirCacheEntry[] entries, TestRepository<Repository> repo, RevCommit... parents) throws Exception { final RevTree ta = repo.tree(entries); RevCommit c = (parents == null) ? repo.commit(ta) : repo.commit(ta, parents); repo.parseBody(c); - return c; + return rw.parseCommit(c); } }