Merge branch 'stable-3.1'
* stable-3.1:
PathToLockId: Refactor to use a FunctionalInterface
LfsAuthUserProvider: Refactor to avoid name hiding
LfsLocksHandle: Collapse duplicate if-block
LfsLocksHandler: Simplify lambda expression
LfsLocksHandler: Call Optional#isPresent before #get
Use Truth8 to assert about Optional#isPresent
LfsCipherTest: Extract repeated string to a constant
InitLfs: Remove unnecessary empty overridden method
LfsSshAuth: Remove redundant 'throws' declaration
LfsGson: Remove unnecessary 'throws' declarations
LfsDateTime: Add private constructor to hide default constructor
Remove LfsDateTimeTest and dependency on joda-time
Bazel: Mark joda-time as test only
Change-Id: I37393a8e50b2f8c014f366dcfc660e4759d4760d
diff --git a/BUILD b/BUILD
index 8eb7d2a..81ab40d 100644
--- a/BUILD
+++ b/BUILD
@@ -41,6 +41,5 @@
exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
":lfs__plugin",
"@jgit-lfs//jar",
- "@joda-time//jar",
],
)
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl
index e814f4b..798575c 100644
--- a/external_plugin_deps.bzl
+++ b/external_plugin_deps.bzl
@@ -36,9 +36,3 @@
"plugin.properties",
],
)
-
- maven_jar(
- name = "joda-time",
- artifact = "joda-time:joda-time:2.9.9",
- sha1 = "f7b520c458572890807d143670c9b24f4de90897",
- )
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/InitLfs.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/InitLfs.java
index 89e6238..f44c8d8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/InitLfs.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/InitLfs.java
@@ -36,7 +36,4 @@
public void run() throws Exception {
lfs.set(PLUGIN_KEY, name);
}
-
- @Override
- public void postRun() throws Exception {}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java
index 4a1936c..1a4c689 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java
@@ -20,6 +20,8 @@
import java.util.Locale;
public class LfsDateTime {
+ private LfsDateTime() {}
+
private static final DateTimeFormatter FORMAT =
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ")
.withZone(ZoneOffset.UTC)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsGson.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsGson.java
index 66398fe..f2af66e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsGson.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsGson.java
@@ -17,8 +17,6 @@
import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
-import com.google.gson.JsonIOException;
-import com.google.gson.JsonSyntaxException;
import com.google.inject.Singleton;
import java.io.Reader;
@@ -34,7 +32,7 @@
.create();
}
- public void toJson(Object src, Appendable writer) throws JsonIOException {
+ public void toJson(Object src, Appendable writer) {
gson.toJson(src, writer);
}
@@ -42,8 +40,7 @@
return gson.toJson(src);
}
- public <T> T fromJson(Reader json, Class<T> classOfT)
- throws JsonSyntaxException, JsonIOException {
+ public <T> T fromJson(Reader json, Class<T> classOfT) {
return gson.fromJson(json, classOfT);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/auth/LfsAuthUserProvider.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/auth/LfsAuthUserProvider.java
index fcd4736..cc19dd3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/auth/LfsAuthUserProvider.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/auth/LfsAuthUserProvider.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.lfs.auth;
+import static com.google.common.base.MoreObjects.firstNonNull;
import static com.googlesource.gerrit.plugins.lfs.auth.LfsSshRequestAuthorizer.SSH_AUTH_PREFIX;
import com.google.common.base.Strings;
@@ -30,7 +31,7 @@
@Singleton
public class LfsAuthUserProvider {
private final Provider<AnonymousUser> anonymous;
- private final Provider<CurrentUser> user;
+ private final Provider<CurrentUser> currentUser;
private final LfsSshRequestAuthorizer sshAuth;
private final AccountCache accounts;
private final IdentifiedUser.GenericFactory userFactory;
@@ -38,12 +39,12 @@
@Inject
LfsAuthUserProvider(
Provider<AnonymousUser> anonymous,
- Provider<CurrentUser> user,
+ Provider<CurrentUser> currentUser,
LfsSshRequestAuthorizer sshAuth,
AccountCache accounts,
IdentifiedUser.GenericFactory userFactory) {
this.anonymous = anonymous;
- this.user = user;
+ this.currentUser = currentUser;
this.sshAuth = sshAuth;
this.accounts = accounts;
this.userFactory = userFactory;
@@ -61,7 +62,6 @@
}
}
}
- CurrentUser currentUser = user.get();
- return currentUser != null ? currentUser : anonymous.get();
+ return firstNonNull(currentUser.get(), anonymous.get());
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/auth/LfsSshAuth.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/auth/LfsSshAuth.java
index 9fd57b9..3dd7a86 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/auth/LfsSshAuth.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/auth/LfsSshAuth.java
@@ -44,7 +44,7 @@
}
@Override
- public String authenticate(CurrentUser user, List<String> args) throws UnloggedFailure, Failure {
+ public String authenticate(CurrentUser user, List<String> args) throws Failure {
if (args.size() != 2) {
throw new UnloggedFailure(1, "Unexpected number of arguments");
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsLocksHandler.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsLocksHandler.java
index 8ba263e..932a92d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsLocksHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsLocksHandler.java
@@ -14,6 +14,8 @@
package com.googlesource.gerrit.plugins.lfs.locks;
+import static java.util.stream.Collectors.groupingBy;
+
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
@@ -29,7 +31,6 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
-import java.util.stream.Collectors;
import org.eclipse.jgit.lfs.errors.LfsException;
@Singleton
@@ -66,14 +67,10 @@
};
}
- private final PathToLockId toLockId;
private final LoadingCache<Project.NameKey, LfsProjectLocks> projectLocks;
@Inject
- LfsLocksHandler(
- PathToLockId toLockId,
- @Named(CACHE_NAME) LoadingCache<Project.NameKey, LfsProjectLocks> projectLocks) {
- this.toLockId = toLockId;
+ LfsLocksHandler(@Named(CACHE_NAME) LoadingCache<Project.NameKey, LfsProjectLocks> projectLocks) {
this.projectLocks = projectLocks;
}
@@ -99,10 +96,9 @@
}
LfsLock lock = hasLock.get();
- if (lock.owner.name.equals(user.getUserName().get())) {
- locks.deleteLock(lock);
- return new LfsLockResponse(lock);
- } else if (input.force) {
+ Optional<String> username = user.getUserName();
+ if ((username.isPresent() && lock.owner.name.equals(username.get()))
+ || Boolean.TRUE.equals(input.force)) {
locks.deleteLock(lock);
return new LfsLockResponse(lock);
}
@@ -116,18 +112,13 @@
LfsProjectLocks locks = projectLocks.getUnchecked(project);
Map<Boolean, List<LfsLock>> groupByOurs =
locks.getLocks().stream()
- .collect(
- Collectors.groupingBy(
- (in) -> {
- return in.owner.name.equals(user.getUserName().get());
- }));
+ .collect(groupingBy(in -> in.owner.name.equals(user.getUserName().get())));
return new LfsVerifyLocksResponse(groupByOurs.get(true), groupByOurs.get(false), null);
}
LfsGetLocksResponse listLocksByPath(Project.NameKey project, String path) {
log.atFine().log("Get lock for %s path in %s project", path, project);
- String lockId = toLockId.apply(path);
- return listLocksById(project, lockId);
+ return listLocksById(project, PathToLockId.CONVERTER.convert(path));
}
LfsGetLocksResponse listLocksById(Project.NameKey project, String id) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsProjectLocks.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsProjectLocks.java
index b02e089..8c0345c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsProjectLocks.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/LfsProjectLocks.java
@@ -43,19 +43,13 @@
private static final FluentLogger log = FluentLogger.forEnclosingClass();
private final LfsGson gson;
- private final PathToLockId toLockId;
private final String project;
private final Path locksPath;
private final Cache<String, LfsLock> locks;
@Inject
- LfsProjectLocks(
- LfsGson gson,
- PathToLockId toLockId,
- LfsLocksPathProvider locksPath,
- @Assisted Project.NameKey project) {
+ LfsProjectLocks(LfsGson gson, LfsLocksPathProvider locksPath, @Assisted Project.NameKey project) {
this.gson = gson;
- this.toLockId = toLockId;
this.project = project.get();
this.locksPath = Paths.get(locksPath.get(), this.project);
this.locks = CacheBuilder.newBuilder().build();
@@ -94,7 +88,7 @@
LfsLock createLock(CurrentUser user, LfsCreateLockInput input) throws LfsException {
log.atFine().log("Create lock for %s in project %s", input.path, project);
- String lockId = toLockId.apply(input.path);
+ String lockId = PathToLockId.CONVERTER.convert(input.path);
LfsLock lock = locks.getIfPresent(lockId);
if (lock != null) {
throw new LfsLockExistsException(lock);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/PathToLockId.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/PathToLockId.java
index d90ebb8..4449469 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/PathToLockId.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/locks/PathToLockId.java
@@ -14,16 +14,22 @@
package com.googlesource.gerrit.plugins.lfs.locks;
-import com.google.common.base.Function;
-import com.google.common.hash.HashCode;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
import com.google.common.hash.Hashing;
import com.google.common.io.BaseEncoding;
-import java.nio.charset.StandardCharsets;
-public class PathToLockId implements Function<String, String> {
- @Override
- public String apply(String path) {
- HashCode hash = Hashing.sha256().hashString(path, StandardCharsets.UTF_8);
- return BaseEncoding.base16().lowerCase().encode(hash.asBytes());
+public class PathToLockId {
+ private PathToLockId() {}
+
+ @FunctionalInterface
+ public interface PathToLockIdInterface {
+ String convert(String path);
}
+
+ static final PathToLockIdInterface CONVERTER =
+ path ->
+ BaseEncoding.base16()
+ .lowerCase()
+ .encode(Hashing.sha256().hashString(path, UTF_8).asBytes());
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsDateTimeTest.java b/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsDateTimeTest.java
deleted file mode 100644
index e2852c5..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsDateTimeTest.java
+++ /dev/null
@@ -1,32 +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.lfs;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import java.time.Instant;
-import org.joda.time.DateTime;
-import org.joda.time.format.ISODateTimeFormat;
-import org.junit.Test;
-
-public class LfsDateTimeTest {
- @Test
- public void format() throws Exception {
- DateTime now = DateTime.now();
- String jodaFormat = ISODateTimeFormat.dateTime().withZoneUTC().print(now);
- String javaFormat = LfsDateTime.format(Instant.ofEpochMilli(now.getMillis()));
- assertThat(javaFormat).isEqualTo(jodaFormat);
- }
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/lfs/auth/LfsAuthTokenTest.java b/src/test/java/com/googlesource/gerrit/plugins/lfs/auth/LfsAuthTokenTest.java
index bdf7c21..f527aab 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/lfs/auth/LfsAuthTokenTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/lfs/auth/LfsAuthTokenTest.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.lfs.auth;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import com.googlesource.gerrit.plugins.lfs.LfsDateTime;
import java.time.Instant;
@@ -33,7 +34,7 @@
String serialized = processor.serialize(token);
assertThat(serialized).isNotEmpty();
Optional<TestToken> deserialized = processor.deserialize(serialized);
- assertThat(deserialized.isPresent()).isTrue();
+ assertThat(deserialized).isPresent();
assertThat(token.issued).isEqualTo(deserialized.get().issued);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/lfs/auth/LfsCipherTest.java b/src/test/java/com/googlesource/gerrit/plugins/lfs/auth/LfsCipherTest.java
index 08d8dc8..3072767 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/lfs/auth/LfsCipherTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/lfs/auth/LfsCipherTest.java
@@ -15,44 +15,44 @@
package com.googlesource.gerrit.plugins.lfs.auth;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import java.util.Optional;
import org.junit.Test;
public class LfsCipherTest {
+ private static final String PLAIN_TEXT = "plain text";
+
private final LfsCipher cipher = new LfsCipher();
@Test
public void testCipherTextIsDifferentThanInput() throws Exception {
- String plain = "plain text";
- String encrypted = cipher.encrypt(plain);
+ String encrypted = cipher.encrypt(PLAIN_TEXT);
assertThat(encrypted).isNotEmpty();
- assertThat(encrypted).isNotEqualTo(plain);
+ assertThat(encrypted).isNotEqualTo(PLAIN_TEXT);
}
@Test
public void testVerifyDecodeAgainstEncodedInput() throws Exception {
- String plain = "plain text";
- String encrypted = cipher.encrypt(plain);
+ String encrypted = cipher.encrypt(PLAIN_TEXT);
Optional<String> decrypted = cipher.decrypt(encrypted);
- assertThat(decrypted.isPresent()).isTrue();
- assertThat(decrypted.get()).isEqualTo(plain);
+ assertThat(decrypted).isPresent();
+ assertThat(decrypted.get()).isEqualTo(PLAIN_TEXT);
}
@Test
public void testVerifyDecodeAgainstInvalidInput() throws Exception {
- String plain = "plain text";
- String encrypted = cipher.encrypt(plain);
+ String encrypted = cipher.encrypt(PLAIN_TEXT);
// there is a chance that two first chars in token are the same
// in such case re-generate the token
while (encrypted.charAt(0) == encrypted.charAt(1)) {
- encrypted = cipher.encrypt(plain);
+ encrypted = cipher.encrypt(PLAIN_TEXT);
}
Optional<String> decrypted =
cipher.decrypt(
encrypted.substring(1, 2) + encrypted.substring(0, 1) + encrypted.substring(2));
- assertThat(decrypted.isPresent()).isTrue();
- assertThat(decrypted.get()).isNotEqualTo(plain);
+ assertThat(decrypted).isPresent();
+ assertThat(decrypted.get()).isNotEqualTo(PLAIN_TEXT);
}
}
