This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo-classloaders.git
The following commit(s) were added to refs/heads/main by this push:
new 584e5bd Fix Manifest/Resource equals/hashCode and add errorprone (#77)
584e5bd is described below
commit 584e5bd40d6d2530f992fa8aae414fe6ca38941b
Author: Christopher Tubbs <[email protected]>
AuthorDate: Tue Feb 24 15:07:13 2026 -0500
Fix Manifest/Resource equals/hashCode and add errorprone (#77)
* Add errorprone CI checks to GitHub Actions and compat checks for
Accumulo 4.0
* Update code to fix items caught by errorprone
* Rename manifests field to monitoredManifests, just to make it more
obvious that it is tracking the manifests currently being monitored
* Fix equals and hashCode methods for Manifest and Resource, to account
for comment and algorithm fields, respectively
* Add test for deduplication of resources in a json
---
.github/workflows/maven-on-demand.yaml | 2 +
.github/workflows/maven.yaml | 40 +++++++++--
modules/caching-classloader/pom.xml | 5 --
.../classloader/ccl/CachingClassLoaderFactory.java | 22 +++---
.../accumulo/classloader/ccl/LocalStore.java | 1 -
.../classloader/ccl/manifest/Manifest.java | 13 +---
.../classloader/ccl/manifest/Resource.java | 15 ++--
.../ccl/CachingClassLoaderFactoryTest.java | 7 +-
.../accumulo/classloader/ccl/LocalStoreTest.java | 43 ++++++-----
.../MiniAccumuloClusterClassLoaderFactoryTest.java | 18 ++---
.../apache/accumulo/classloader/ccl/TestUtils.java | 12 +++-
.../accumulo/classloader/ccl/URLTypesTest.java | 4 +-
.../classloader/ccl/manifest/ManifestTest.java | 51 +++++++++++--
.../classloader/vfs/examples/ExampleIterator.java | 7 ++
.../classloader/vfs/examples/ExampleIterator.java | 7 ++
.../HdfsURLStreamHandlerProvider.java | 2 +-
pom.xml | 83 +++++++++++++++++++++-
17 files changed, 252 insertions(+), 80 deletions(-)
diff --git a/.github/workflows/maven-on-demand.yaml
b/.github/workflows/maven-on-demand.yaml
index e7b1cea..c96b447 100644
--- a/.github/workflows/maven-on-demand.yaml
+++ b/.github/workflows/maven-on-demand.yaml
@@ -46,6 +46,8 @@ jobs:
distribution: temurin
java-version: 21
cache: 'maven'
+ - name: Show the first log message
+ run: git log -n1
- name: Build with Maven
timeout-minutes: 345
run: mvn -B -V -e -ntp "-Dstyle.color=always" ${{
github.event.inputs.goals }}
diff --git a/.github/workflows/maven.yaml b/.github/workflows/maven.yaml
index d992d17..4ac4e71 100644
--- a/.github/workflows/maven.yaml
+++ b/.github/workflows/maven.yaml
@@ -31,8 +31,8 @@ permissions:
contents: read
jobs:
- mvn:
- timeout-minutes: 60
+ # fast build to populate the local maven repository cache
+ fastbuild:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
@@ -42,22 +42,50 @@ jobs:
distribution: temurin
java-version: 21
cache: 'maven'
- - name: Build with Maven (verify javadoc:jar)
- run: mvn -B -V -e -ntp "-Dstyle.color=always" verify javadoc:jar
-DskipFormat -DverifyFormat
+ - name: Show the first log message
+ run: git log -n1
+ - name: Build with Maven (Fast Build)
+ timeout-minutes: 20
+ run: mvn -B -V -e -ntp "-Dstyle.color=always" clean package
dependency:resolve -DskipTests -DskipFormat -DverifyFormat
+ env:
+ MAVEN_OPTS: -Djansi.force=true
+ # more complete builds with tests
+ mvn:
+ needs: fastbuild
+ strategy:
+ matrix:
+ profile:
+ - {name: 'unit-tests', javaver: 21, args: 'verify -PskipQA
-DskipTests=false'}
+ - {name: 'qa-checks', javaver: 21, args: 'verify javadoc:jar
-DskipTests -Dspotbugs.timeout=3600000'}
+ - {name: 'compat', javaver: 21, args: 'package -DskipTests
-Dversion.accumulo=4.0.0-SNAPSHOT -Puse-apache-snapshots
-Dmaven.compiler.failOnWarning=false'}
+ - {name: 'errorprone', javaver: 21, args: 'verify
-Perrorprone,skipQA'}
+ fail-fast: false
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v6
+ - name: Set up JDK ${{ matrix.profile.javaver }}
+ uses: actions/setup-java@v5
+ with:
+ distribution: temurin
+ java-version: ${{ matrix.profile.javaver }}
+ cache: 'maven'
+ - name: Build with Maven (${{ matrix.profile.name }})
+ timeout-minutes: 60
+ run: mvn -B -V -e -ntp "-Dstyle.color=always" -DskipFormat ${{
matrix.profile.args }}
env:
MAVEN_OPTS: -Djansi.force=true
- name: Upload unit test results
if: ${{ failure() }}
uses: actions/upload-artifact@v6
with:
- name: surefire-reports
+ name: surefire-reports-${{ matrix.profile.name }}
path: ./**/target/surefire-reports/
if-no-files-found: ignore
- name: Upload integration test results
if: ${{ failure() }}
uses: actions/upload-artifact@v6
with:
- name: failsafe-reports
+ name: failsafe-reports-${{ matrix.profile.name }}
path: ./**/target/failsafe-reports/
if-no-files-found: ignore
diff --git a/modules/caching-classloader/pom.xml
b/modules/caching-classloader/pom.xml
index 35a7270..bc20416 100644
--- a/modules/caching-classloader/pom.xml
+++ b/modules/caching-classloader/pom.xml
@@ -120,11 +120,6 @@
<artifactId>jetty-server</artifactId>
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.eclipse.jetty</groupId>
- <artifactId>jetty-util</artifactId>
- <scope>test</scope>
- </dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
diff --git
a/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactory.java
b/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactory.java
index 2697150..e0d9f25 100644
---
a/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactory.java
+++
b/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactory.java
@@ -123,7 +123,7 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
// stores the latest seen manifest for a remote URL; URI types are used here
for the key
// instead of URL because URL.hashCode could trigger network activity for
hostname lookups
- private final ConcurrentHashMap<URI,Manifest> manifests = new
ConcurrentHashMap<>();
+ private final ConcurrentHashMap<URI,Manifest> monitoredManifests = new
ConcurrentHashMap<>();
// to keep this coherent with the manifests, updates to this should be done
in manifests.compute()
private final
DeduplicationCache<DeduplicationCacheKey,LocalStore,URLClassLoader>
classloaders =
@@ -148,16 +148,17 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
*/
private void monitor(final URI uri, long interval) {
LOG.trace("Monitoring manifest {} for changes at {} second intervals",
uri, interval);
- executor.schedule(() -> {
+ var unused = executor.schedule(() -> {
try {
checkMonitoredUrl(uri, interval);
} catch (Throwable t) {
LOG.error("Unhandled exception occurred in manifest monitor thread.
Removing manifest {}.",
uri, t);
- manifests.remove(uri);
+ monitoredManifests.remove(uri);
throw t;
}
}, interval, TimeUnit.SECONDS);
+ assert unused != null;
}
@Override
@@ -188,7 +189,8 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
};
try {
// check the allowed URLs pattern, getting it ready for first use, and
warning if it is bad
- allowedUrlsPattern.get();
+ var unused = allowedUrlsPattern.get();
+ assert unused != null;
} catch (RuntimeException npe) {
LOG.warn(
"Property {} is not set or contains an invalid pattern ()."
@@ -211,7 +213,7 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
try {
// get the current manifest, or create it from the URL if absent; this
has the side effect of
// creating and caching a class loader instance if it doesn't exist for
the computed manifest
- manifests.compute(manifestURI,
+ monitoredManifests.compute(manifestURI,
(key, previous) -> computeManifestAndClassLoader(classloader, key,
previous));
} catch (RuntimeException e) {
throw new ContextClassLoaderException(e.getMessage(), e);
@@ -248,7 +250,7 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
}
private void checkMonitoredUrl(URI uri, long interval) {
- Manifest current = manifests.compute(uri, (key, previous) -> {
+ Manifest current = monitoredManifests.compute(uri, (key, previous) -> {
if (previous == null) {
// manifest has been removed from the map, no need to check for update
LOG.debug("Manifest for {} not present, no longer monitoring for
changes", uri);
@@ -271,7 +273,7 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
if (!current.getChecksum().equals(update.getChecksum())) {
LOG.debug("Context manifest for {} has changed", uri);
localStore.get().storeContext(update);
- manifests.put(uri, update);
+ monitoredManifests.put(uri, update);
nextInterval = update.getMonitorIntervalSeconds();
classloaderFailures.remove(uri);
} else {
@@ -280,7 +282,7 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
// reschedule this task to run if the manifest exists.
// Atomically lock on the key and only reschedule if the value is
present.
final long finalMonitorInterval = nextInterval;
- manifests.compute(uri, (k, v) -> {
+ monitoredManifests.compute(uri, (k, v) -> {
if (v != null) {
monitor(uri, finalMonitorInterval);
}
@@ -305,7 +307,7 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
// unset the classloader reference so that the failure
// will return from getClassLoader in the calling thread
LOG.info("Grace period for failing classloader has elapsed for {}",
uri);
- manifests.remove(uri);
+ monitoredManifests.remove(uri);
classloaderFailures.remove(uri);
} else {
LOG.trace("Failing to update classloader for {} within the grace
period", uri, e);
@@ -315,7 +317,7 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
// on success or handled exception
// Atomically lock on the key and only reschedule if the value is
present.
final long finalMonitorInterval = nextInterval;
- manifests.compute(uri, (k, v) -> {
+ monitoredManifests.compute(uri, (k, v) -> {
if (v != null) {
monitor(uri, finalMonitorInterval);
}
diff --git
a/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/LocalStore.java
b/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/LocalStore.java
index cce9c91..7939b0a 100644
---
a/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/LocalStore.java
+++
b/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/LocalStore.java
@@ -206,7 +206,6 @@ public final class LocalStore {
LOG.trace("Skipped resource {} while another process or thread is
downloading it",
resource.getLocation());
waitingOnOtherDownloadsCount++;
- continue;
}
}
// avoid rapid cycling checking for other downloads to finish; wait
longer if more downloads
diff --git
a/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/manifest/Manifest.java
b/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/manifest/Manifest.java
index deb83c5..c583a55 100644
---
a/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/manifest/Manifest.java
+++
b/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/manifest/Manifest.java
@@ -19,7 +19,6 @@
package org.apache.accumulo.classloader.ccl.manifest;
import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Objects.hash;
import static java.util.Objects.requireNonNull;
import java.io.BufferedInputStream;
@@ -121,7 +120,7 @@ public class Manifest {
@Override
public int hashCode() {
- return hash(monitorIntervalSeconds, resources);
+ return toJson().hashCode();
}
@Override
@@ -129,15 +128,7 @@ public class Manifest {
if (this == obj) {
return true;
}
- if (obj == null) {
- return false;
- }
- if (getClass() != obj.getClass()) {
- return false;
- }
- Manifest other = (Manifest) obj;
- return monitorIntervalSeconds == other.monitorIntervalSeconds
- && Objects.equals(resources, other.resources);
+ return obj instanceof Manifest ? Objects.equals(toJson(), ((Manifest)
obj).toJson()) : false;
}
public String getChecksumAlgorithm() {
diff --git
a/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/manifest/Resource.java
b/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/manifest/Resource.java
index 56153e0..d4d8d7d 100644
---
a/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/manifest/Resource.java
+++
b/modules/caching-classloader/src/main/java/org/apache/accumulo/classloader/ccl/manifest/Resource.java
@@ -76,7 +76,7 @@ public class Resource {
@Override
public int hashCode() {
- return Objects.hash(location, checksum);
+ return Objects.hash(getLocation(), getAlgorithm(), getChecksum());
}
@Override
@@ -84,14 +84,13 @@ public class Resource {
if (this == obj) {
return true;
}
- if (obj == null) {
- return false;
+ if (obj instanceof Resource) {
+ Resource other = (Resource) obj;
+ return Objects.equals(getLocation(), other.getLocation())
+ && Objects.equals(getAlgorithm(), other.getAlgorithm())
+ && Objects.equals(getChecksum(), other.getChecksum());
}
- if (getClass() != obj.getClass()) {
- return false;
- }
- Resource other = (Resource) obj;
- return Objects.equals(checksum, other.checksum) &&
Objects.equals(location, other.location);
+ return false;
}
}
diff --git
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactoryTest.java
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactoryTest.java
index 3e724d3..85559b6 100644
---
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactoryTest.java
+++
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactoryTest.java
@@ -901,8 +901,11 @@ class CachingClassLoaderFactoryTest {
deleteFuture.get();
executor.shutdown();
- long workingDirsCount =
- Files.list(baseCacheDir.resolve(WORKING_DIR)).filter(p ->
p.toFile().isDirectory()).count();
+ long workingDirsCount;
+ try (var s =
+ Files.list(baseCacheDir.resolve(WORKING_DIR)).filter(p ->
p.toFile().isDirectory())) {
+ workingDirsCount = s.count();
+ }
// check that many hard link directories were created; this is
non-deterministic; at least 50 is
// nice to see (5 per thread), but depending on performance, each thread
may not create them
// that quickly; we should get at least 1 per thread, though
diff --git
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/LocalStoreTest.java
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/LocalStoreTest.java
index 0403ee0..5f8611d 100644
---
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/LocalStoreTest.java
+++
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/LocalStoreTest.java
@@ -36,13 +36,13 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import java.io.IOException;
import java.net.URI;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.LinkedHashSet;
import java.util.concurrent.CountDownLatch;
-import java.util.function.BiConsumer;
import java.util.stream.Collectors;
import org.apache.accumulo.classloader.ccl.TestUtils.TestClassInfo;
@@ -65,7 +65,7 @@ public class LocalStoreTest {
private static Path tempDir;
// a mock URL checker that allows all for test
- private static final BiConsumer<String,URL> ALLOW_ALL_URLS = (type, url) ->
{};
+ private static final void allowAllUrls(String type, URL url) {};
private static MiniDFSCluster hdfs;
private static Server jetty;
@@ -97,6 +97,7 @@ public class LocalStoreTest {
// Put C into Jetty
var jarCParentDirectory = Path.of(jarCOrigLocation.toURI()).getParent();
+ assertNotNull(jarCParentDirectory);
jetty = TestUtils.getJetty(jarCParentDirectory);
final URL jarCNewLocation = jetty.getURI().resolve("TestC.jar").toURL();
@@ -135,8 +136,10 @@ public class LocalStoreTest {
@Test
public void testPropertyNotSet() {
// test baseDir not set
- assertThrows(NullPointerException.class, () -> new LocalStore((Path) null,
ALLOW_ALL_URLS));
- assertThrows(NullPointerException.class, () -> new LocalStore((String)
null, ALLOW_ALL_URLS));
+ assertThrows(NullPointerException.class,
+ () -> new LocalStore((Path) null, LocalStoreTest::allowAllUrls));
+ assertThrows(NullPointerException.class,
+ () -> new LocalStore((String) null, LocalStoreTest::allowAllUrls));
// test URL checker not set
assertThrows(NullPointerException.class, () -> new
LocalStore(baseCacheDir, null));
}
@@ -149,7 +152,7 @@ public class LocalStoreTest {
assertEquals("working", WORKING_DIR);
assertFalse(Files.exists(baseCacheDir));
- var localStore = new LocalStore(baseCacheDir, ALLOW_ALL_URLS);
+ var localStore = new LocalStore(baseCacheDir,
LocalStoreTest::allowAllUrls);
assertTrue(Files.exists(baseCacheDir));
assertTrue(Files.exists(baseCacheDir.resolve(MANIFESTS_DIR)));
assertTrue(Files.exists(baseCacheDir.resolve(RESOURCES_DIR)));
@@ -162,10 +165,10 @@ public class LocalStoreTest {
@Test
public void testCreateBaseDirsMultipleTimes() throws Exception {
assertFalse(Files.exists(baseCacheDir));
- assertNotNull(new LocalStore(baseCacheDir, ALLOW_ALL_URLS));
- assertNotNull(new LocalStore(baseCacheDir, ALLOW_ALL_URLS));
- assertNotNull(new LocalStore(baseCacheDir, ALLOW_ALL_URLS));
- assertNotNull(new LocalStore(baseCacheDir, ALLOW_ALL_URLS));
+ assertNotNull(new LocalStore(baseCacheDir, LocalStoreTest::allowAllUrls));
+ assertNotNull(new LocalStore(baseCacheDir, LocalStoreTest::allowAllUrls));
+ assertNotNull(new LocalStore(baseCacheDir, LocalStoreTest::allowAllUrls));
+ assertNotNull(new LocalStore(baseCacheDir, LocalStoreTest::allowAllUrls));
assertTrue(Files.exists(baseCacheDir));
}
@@ -230,7 +233,7 @@ public class LocalStoreTest {
@Test
public void testStoreContext() throws Exception {
- var localStore = new LocalStore(baseCacheDir, ALLOW_ALL_URLS);
+ var localStore = new LocalStore(baseCacheDir,
LocalStoreTest::allowAllUrls);
localStore.storeContext(manifest);
// Confirm the 3 jars are cached locally
@@ -244,7 +247,7 @@ public class LocalStoreTest {
@Test
public void testClassLoader() throws Exception {
- var localStore = new LocalStore(baseCacheDir, ALLOW_ALL_URLS);
+ var localStore = new LocalStore(baseCacheDir,
LocalStoreTest::allowAllUrls);
localStore.storeContext(manifest);
var cacheKey = new DeduplicationCacheKey(URI.create("loc"), manifest);
var cl = createClassLoader(cacheKey, localStore);
@@ -256,7 +259,7 @@ public class LocalStoreTest {
@Test
public void testClassLoaderUpdate() throws Exception {
- var localStore = new LocalStore(baseCacheDir, ALLOW_ALL_URLS);
+ var localStore = new LocalStore(baseCacheDir,
LocalStoreTest::allowAllUrls);
localStore.storeContext(manifest);
var cacheKey = new DeduplicationCacheKey(URI.create("loc"), manifest);
final var cl = createClassLoader(cacheKey, localStore);
@@ -304,13 +307,19 @@ public class LocalStoreTest {
testClassLoads(updatedCl, classD);
}
+ private static long getWorkingDirCount(LocalStore localStore) throws
IOException {
+ try (var s =
Files.list(localStore.workingDir()).filter(Files::isDirectory)) {
+ return s.count();
+ }
+ }
+
@Test
public void testClassLoaderDirCleanup() throws Exception {
- var localStore = new LocalStore(baseCacheDir, ALLOW_ALL_URLS);
- assertEquals(0,
Files.list(localStore.workingDir()).filter(Files::isDirectory).count());
+ var localStore = new LocalStore(baseCacheDir,
LocalStoreTest::allowAllUrls);
+ assertEquals(0, getWorkingDirCount(localStore));
localStore.storeContext(manifest);
var cacheKey = new DeduplicationCacheKey(URI.create("loc"), manifest);
- assertEquals(0,
Files.list(localStore.workingDir()).filter(Files::isDirectory).count());
+ assertEquals(0, getWorkingDirCount(localStore));
final var endBackgroundThread = new CountDownLatch(1);
var createdClassLoader = new CountDownLatch(1);
@@ -329,12 +338,12 @@ public class LocalStoreTest {
t.start();
createdClassLoader.await();
- assertEquals(1,
Files.list(localStore.workingDir()).filter(Files::isDirectory).count());
+ assertEquals(1, getWorkingDirCount(localStore));
endBackgroundThread.countDown();
// wait for classloader to be garbage collected and its cleaner to run
- while
(Files.list(localStore.workingDir()).filter(Files::isDirectory).count() != 0) {
+ while (getWorkingDirCount(localStore) != 0) {
System.gc();
Thread.sleep(50);
}
diff --git
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/MiniAccumuloClusterClassLoaderFactoryTest.java
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/MiniAccumuloClusterClassLoaderFactoryTest.java
index 4db228a..ae9a219 100644
---
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/MiniAccumuloClusterClassLoaderFactoryTest.java
+++
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/MiniAccumuloClusterClassLoaderFactoryTest.java
@@ -86,7 +86,6 @@ public class MiniAccumuloClusterClassLoaderFactoryTest
extends SharedMiniCluster
@Override
public void configureMiniCluster(MiniAccumuloConfigImpl cfg,
org.apache.hadoop.conf.Configuration coreSite) {
- cfg.setNumTservers(3);
cfg.setProperty(Property.TSERV_NATIVEMAP_ENABLED.getKey(), "false");
cfg.setProperty(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY.getKey(),
CachingClassLoaderFactory.class.getName());
@@ -148,7 +147,7 @@ public class MiniAccumuloClusterClassLoaderFactoryTest
extends SharedMiniCluster
List<String> tservers = client.instanceOperations().getTabletServers();
Collections.sort(tservers);
- assertEquals(3, tservers.size());
+ assertTrue(tservers.size() >= 2, "Expected at least 2 tservers, but saw
" + tservers);
final String tableName = names[0];
@@ -163,16 +162,17 @@ public class MiniAccumuloClusterClassLoaderFactoryTest
extends SharedMiniCluster
TestIngest.createTable(client, params);
- // Confirm 4 tablets, spread across 3 tablet servers
+ // Confirm 4 tablets, spread across all tablet servers
client.instanceOperations().waitForBalance();
final List<TabletMetadata> tm = getLocations(((ClientContext)
client).getAmple(),
client.tableOperations().tableIdMap().get(tableName));
- assertEquals(4, tm.size()); // 3 tablets
+ assertEquals(4, tm.size());
final Set<String> tabletLocations = new TreeSet<>();
tm.forEach(t -> tabletLocations.add(t.getLocation().getHostPort()));
- assertEquals(3, tabletLocations.size()); // 3 locations
+ assertTrue(tabletLocations.size() >= 2,
+ "Expected at least 2 tablet locations, but saw " + tabletLocations);
// both collections are sorted
assertIterableEquals(tservers, tabletLocations);
@@ -281,9 +281,11 @@ public class MiniAccumuloClusterClassLoaderFactoryTest
extends SharedMiniCluster
private Set<Path> getReferencedFiles(Path workingDirPath) throws IOException
{
// get all files in subdirectories in working directory
- return Files.walk(workingDirPath).filter(p -> p.toFile().isFile())
- .filter(p -> p.getNameCount() == workingDirPath.getNameCount() +
2).map(Path::getFileName)
- .collect(Collectors.toSet());
+ try (var s = Files.walk(workingDirPath).filter(p -> p.toFile().isFile())
+ .filter(p -> p.getNameCount() == workingDirPath.getNameCount() + 2)
+ .map(Path::getFileName)) {
+ return s.collect(Collectors.toSet());
+ }
}
private int countExpectedValues(AccumuloClient client, String table, byte[]
expectedValue)
diff --git
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/TestUtils.java
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/TestUtils.java
index 0a4af81..1ad53ed 100644
---
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/TestUtils.java
+++
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/TestUtils.java
@@ -25,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.io.InputStream;
+import java.lang.reflect.Method;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -38,7 +39,6 @@ import org.apache.hadoop.hdfs.DFSConfigKeys;
import org.apache.hadoop.hdfs.MiniDFSCluster;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.ResourceHandler;
-import org.eclipse.jetty.util.resource.PathResource;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -124,9 +124,15 @@ public class TestUtils {
}
public static Server getJetty(Path resourceDirectory) throws Exception {
- PathResource directory = new PathResource(resourceDirectory);
ResourceHandler handler = new ResourceHandler();
- handler.setBaseResource(directory);
+ // work with jetty 11 or 12, so we can build with Accumulo 2.1 or 4.0
+ Method m;
+ try {
+ m = handler.getClass().getMethod("setResourceBase", String.class); //
jetty 11
+ } catch (NoSuchMethodException e) {
+ m = handler.getClass().getMethod("setBaseResourceAsString",
String.class); // jetty 12
+ }
+ m.invoke(handler, resourceDirectory.toString());
Server jetty = new Server(0);
jetty.setHandler(handler);
diff --git
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/URLTypesTest.java
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/URLTypesTest.java
index cdf64b1..eee04bc 100644
---
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/URLTypesTest.java
+++
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/URLTypesTest.java
@@ -55,9 +55,11 @@ public class URLTypesTest {
URL jarPath = URLTypesTest.class.getResource("/HelloWorld.jar");
assertNotNull(jarPath);
var p = Path.of(jarPath.toURI());
+ var parent = p.getParent();
+ assertNotNull(parent);
final long origFileSize = TestUtils.getFileSize(p);
- Server jetty = TestUtils.getJetty(p.getParent());
+ Server jetty = TestUtils.getJetty(parent);
LOG.debug("Jetty listening at: {}", jetty.getURI());
URL httpPath = jetty.getURI().resolve("HelloWorld.jar").toURL();
assertEquals(origFileSize, TestUtils.getFileSize(httpPath));
diff --git
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/manifest/ManifestTest.java
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/manifest/ManifestTest.java
index 1519f7a..4f5cbb3 100644
---
a/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/manifest/ManifestTest.java
+++
b/modules/caching-classloader/src/test/java/org/apache/accumulo/classloader/ccl/manifest/ManifestTest.java
@@ -21,17 +21,22 @@ package org.apache.accumulo.classloader.ccl.manifest;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
import java.io.ByteArrayInputStream;
import java.nio.file.Path;
+import java.security.SecureRandom;
+import java.util.regex.Pattern;
import org.junit.jupiter.api.Test;
class ManifestTest {
+ private static final SecureRandom RAND = new SecureRandom();
+
private static String mockJson(boolean withComment, boolean
withMonitorInterval,
int withResourceCount) {
final String COMMA = ",";
@@ -55,8 +60,13 @@ class ManifestTest {
if (i > 0) {
json.append(COMMA);
}
- json.append("{'location': 'file:/home/user/ClassLoaderTestA/" + i +
".jar'").append(COMMA);
- json.append("'algorithm': 'MOCK',").append("'checksum': '" + i + "'}");
+ var n = i;
+ if (withResourceCount == 10) {
+ // special value to make all resources the same to test deduplication
+ n = 0;
+ }
+ json.append("{'location': 'file:/home/user/ClassLoaderTestA/" + n +
".jar'").append(COMMA);
+ json.append("'algorithm': 'MOCK',").append("'checksum': '" + n + "'}");
}
json.append("]");
}
@@ -82,15 +92,16 @@ class ManifestTest {
@Test
void testDeserializing() throws Exception {
- var json = mockJson(true, true, 3);
+ int resourceCount = RAND.nextInt(100) + 15;
+ var json = mockJson(true, true, resourceCount);
try (var in = new ByteArrayInputStream(json.getBytes(UTF_8))) {
var manifest = Manifest.fromStream(in);
assertEquals("an optional comment", manifest.getComment());
assertEquals(5, manifest.getMonitorIntervalSeconds());
- assertEquals(3, manifest.getResources().size());
+ assertEquals(resourceCount, manifest.getResources().size());
var resources = manifest.getResources();
var iter = resources.iterator();
- for (int i = 0; i < 3; i++) {
+ for (int i = 0; i < resourceCount; i++) {
var r = iter.next();
assertEquals(i + ".jar", r.getFileName());
assertEquals("MOCK", r.getAlgorithm());
@@ -126,4 +137,34 @@ class ManifestTest {
assertNull(Manifest.fromStream(in));
}
}
+
+ @Test
+ void testDeserializationWithDeduplication() throws Exception {
+ Pattern numExpectedJar = Pattern.compile("0[.]jar");
+ var json = mockJson(true, true, 1);
+ var jsonWithDuplicates = mockJson(true, true, 10);
+ assertNotEquals(json, jsonWithDuplicates);
+ assertEquals(1, numExpectedJar.matcher(json).results().count());
+ assertEquals(10,
numExpectedJar.matcher(jsonWithDuplicates).results().count());
+ final Manifest manifest;
+ try (var in = new ByteArrayInputStream(json.getBytes(UTF_8))) {
+ manifest = Manifest.fromStream(in);
+ }
+ final Manifest manifestFromDuplicates;
+ try (var in = new
ByteArrayInputStream(jsonWithDuplicates.getBytes(UTF_8))) {
+ manifestFromDuplicates = Manifest.fromStream(in);
+ }
+ assertEquals(manifest, manifestFromDuplicates);
+ assertEquals(manifest.toJson(), manifestFromDuplicates.toJson());
+ assertEquals("an optional comment", manifest.getComment());
+ assertEquals(5, manifest.getMonitorIntervalSeconds());
+ assertEquals(1, manifest.getResources().size());
+ var resources = manifest.getResources();
+ var iter = resources.iterator();
+ var r = iter.next();
+ assertEquals("0.jar", r.getFileName());
+ assertEquals("MOCK", r.getAlgorithm());
+ assertEquals(String.valueOf(0), r.getChecksum());
+ assertFalse(iter.hasNext());
+ }
}
diff --git
a/modules/example-iterators-a/src/main/java/org/apache/accumulo/classloader/vfs/examples/ExampleIterator.java
b/modules/example-iterators-a/src/main/java/org/apache/accumulo/classloader/vfs/examples/ExampleIterator.java
index f630aea..c9647c7 100644
---
a/modules/example-iterators-a/src/main/java/org/apache/accumulo/classloader/vfs/examples/ExampleIterator.java
+++
b/modules/example-iterators-a/src/main/java/org/apache/accumulo/classloader/vfs/examples/ExampleIterator.java
@@ -38,31 +38,38 @@ public class ExampleIterator implements
SortedKeyValueIterator<Key,Value> {
protected SortedKeyValueIterator<Key,Value> source;
+ @Override
public ExampleIterator deepCopy(IteratorEnvironment env) {
throw new UnsupportedOperationException();
}
+ @Override
public Key getTopKey() {
return source.getTopKey();
}
+ @Override
public Value getTopValue() {
return new Value("foo".getBytes(UTF_8));
}
+ @Override
public boolean hasTop() {
return source.hasTop();
}
+ @Override
public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
IteratorEnvironment env) {
this.source = source;
}
+ @Override
public void next() throws IOException {
source.next();
}
+ @Override
public void seek(Range range, Collection<ByteSequence> columnFamilies,
boolean inclusive)
throws IOException {
source.seek(range, columnFamilies, inclusive);
diff --git
a/modules/example-iterators-b/src/main/java/org/apache/accumulo/classloader/vfs/examples/ExampleIterator.java
b/modules/example-iterators-b/src/main/java/org/apache/accumulo/classloader/vfs/examples/ExampleIterator.java
index 868bd89..9845ab6 100644
---
a/modules/example-iterators-b/src/main/java/org/apache/accumulo/classloader/vfs/examples/ExampleIterator.java
+++
b/modules/example-iterators-b/src/main/java/org/apache/accumulo/classloader/vfs/examples/ExampleIterator.java
@@ -38,31 +38,38 @@ public class ExampleIterator implements
SortedKeyValueIterator<Key,Value> {
protected SortedKeyValueIterator<Key,Value> source;
+ @Override
public ExampleIterator deepCopy(IteratorEnvironment env) {
throw new UnsupportedOperationException();
}
+ @Override
public Key getTopKey() {
return source.getTopKey();
}
+ @Override
public Value getTopValue() {
return new Value("bar".getBytes(UTF_8));
}
+ @Override
public boolean hasTop() {
return source.hasTop();
}
+ @Override
public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
IteratorEnvironment env) {
this.source = source;
}
+ @Override
public void next() throws IOException {
source.next();
}
+ @Override
public void seek(Range range, Collection<ByteSequence> columnFamilies,
boolean inclusive)
throws IOException {
source.seek(range, columnFamilies, inclusive);
diff --git
a/modules/hdfs-urlstreamhandler-provider/src/main/java/org/apache/accumulo/classloader/hdfsurlstreamhandlerprovider/HdfsURLStreamHandlerProvider.java
b/modules/hdfs-urlstreamhandler-provider/src/main/java/org/apache/accumulo/classloader/hdfsurlstreamhandlerprovider/HdfsURLStreamHandlerProvider.java
index 861d0f4..eb4e369 100644
---
a/modules/hdfs-urlstreamhandler-provider/src/main/java/org/apache/accumulo/classloader/hdfsurlstreamhandlerprovider/HdfsURLStreamHandlerProvider.java
+++
b/modules/hdfs-urlstreamhandler-provider/src/main/java/org/apache/accumulo/classloader/hdfsurlstreamhandlerprovider/HdfsURLStreamHandlerProvider.java
@@ -51,7 +51,7 @@ public class HdfsURLStreamHandlerProvider extends
URLStreamHandlerProvider {
private InputStream is;
- public HdfsURLConnection(URL url) {
+ HdfsURLConnection(URL url) {
super(requireNonNull(url, "null url argument"));
}
diff --git a/pom.xml b/pom.xml
index 99da47f..b2e9d53 100644
--- a/pom.xml
+++ b/pom.xml
@@ -130,7 +130,10 @@ under the License.
<rat.consoleOutput>true</rat.consoleOutput>
<sourceReleaseAssemblyDescriptor>source-release-tar</sourceReleaseAssemblyDescriptor>
<surefire.failIfNoSpecifiedTests>false</surefire.failIfNoSpecifiedTests>
+ <version.accumulo>2.1.4</version.accumulo>
<version.auto-service>1.1.1</version.auto-service>
+ <version.errorprone>2.47.0</version.errorprone>
+ <version.httpclient5>5.6</version.httpclient5>
</properties>
<dependencyManagement>
<dependencies>
@@ -138,7 +141,7 @@ under the License.
<!-- most dependencies will be provided by the accumulo installation
-->
<groupId>org.apache.accumulo</groupId>
<artifactId>accumulo-project</artifactId>
- <version>2.1.4</version>
+ <version>${version.accumulo}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
@@ -155,7 +158,7 @@ under the License.
<dependency>
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5</artifactId>
- <version>5.6</version>
+ <version>${version.httpclient5}</version>
</dependency>
</dependencies>
</dependencyManagement>
@@ -668,6 +671,29 @@ under the License.
</plugins>
</build>
<profiles>
+ <profile>
+ <!-- This profile skips all Quality Assurance checks; activate with
-PskipQA OR -DskipQA -->
+ <id>skipQA</id>
+ <activation>
+ <property>
+ <name>skipQA</name>
+ </property>
+ </activation>
+ <properties>
+ <apilyzer.skip>true</apilyzer.skip>
+ <checkstyle.skip>true</checkstyle.skip>
+ <formatter.skip>true</formatter.skip>
+ <impsort.skip>true</impsort.skip>
+ <mdep.analyze.skip>true</mdep.analyze.skip>
+ <modernizer.skip>true</modernizer.skip>
+ <rat.skip>true</rat.skip>
+ <skipITs>true</skipITs>
+ <skipTests>true</skipTests>
+ <sort.skip>true</sort.skip>
+ <spotbugs.skip>true</spotbugs.skip>
+ <warbucks.skip>true</warbucks.skip>
+ </properties>
+ </profile>
<profile>
<id>accumulo-release</id>
<properties>
@@ -893,5 +919,58 @@ under the License.
</pluginManagement>
</build>
</profile>
+ <profile>
+ <!-- This profile uses the Google ErrorProne tool to perform static code
analysis at
+ compile time. Auto-generated code is not checked.
+ See: https://errorprone.info/bugpatterns for list of available bug
patterns.-->
+ <id>errorprone</id>
+ <activation>
+ <property>
+ <name>errorprone</name>
+ </property>
+ </activation>
+ <properties>
+ <!-- forking is required for -J options to take effect -->
+ <maven.compiler.fork>true</maven.compiler.fork>
+ </properties>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs>
+ <arg>-XDcompilePolicy=simple</arg>
+ <arg>-XDaddTypeAnnotationsToSymbol=true</arg>
+ <arg>--should-stop=ifError=FLOW</arg>
+ <arg>
+ -Xplugin:ErrorProne \
+ <!-- this complains about using LinkedHashSet instead of Set
+ but we need to use that to preserve set order -->
+ -Xep:NonApiType:OFF \
+ </arg>
+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
+
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
+
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
+ </compilerArgs>
+ <annotationProcessorPaths>
+ <path>
+ <groupId>com.google.errorprone</groupId>
+ <artifactId>error_prone_core</artifactId>
+ <version>${version.errorprone}</version>
+ </path>
+ </annotationProcessorPaths>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
</profiles>
</project>
