Skip to content

Commit

Permalink
RepositoryCache: support "canonical id"
Browse files Browse the repository at this point in the history 
    Make the repository cache support the concept of a canonical id,
    i.e., files are not considered a cache, just because they have the
    correct content, if a different canonical id is specified.

    Design document: https://github.com/bazelbuild/proposals/blob/master/designs/2019-04-29-cache.md

    Related #5144.

    Change-Id: Ie4e4cef14a5810f73add26ca8b97ce7aeb18b6e3
    PiperOrigin-RevId: 249017336
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 2f75332 commit 21e16ae
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 39 deletions.
34 changes: 10 additions & 24 deletions ...l/src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ boolean hasCanonicalId(String cacheKey, KeyType keyType, String canonicalId) {
}

public synchronized Path get(String cacheKey, Path targetPath, KeyType keyType)
throws IOException, InterruptedException {
throws IOException {
return get(cacheKey, targetPath, keyType, null);
}

Expand All @@ -150,11 +150,7 @@ public synchronized Path get(String cacheKey, Path targetPath, KeyType keyType)
*/
@Nullable
public synchronized Path get(
String cacheKey, Path targetPath, KeyType keyType, String canonicalId)
throws IOException, InterruptedException {
if (Thread.interrupted()) {
throw new InterruptedException();
}
String cacheKey, Path targetPath, KeyType keyType, String canonicalId) throws IOException {
Preconditions.checkState(isEnabled());

assertKeyIsValid(cacheKey, keyType);
Expand Down Expand Up @@ -196,7 +192,7 @@ public synchronized Path get(
}

public synchronized void put(String cacheKey, Path sourcePath, KeyType keyType)
throws IOException, InterruptedException {
throws IOException {
put(cacheKey, sourcePath, keyType, null);
}

Expand All @@ -211,12 +207,7 @@ public synchronized void put(String cacheKey, Path sourcePath, KeyType keyType)
* @throws IOException
*/
public synchronized void put(
String cacheKey, Path sourcePath, KeyType keyType, String canonicalId)
throws IOException, InterruptedException {
// Check for interrupts while waiting for the monitor of this synchronized method
if (Thread.interrupted()) {
throw new InterruptedException();
}
String cacheKey, Path sourcePath, KeyType keyType, String canonicalId) throws IOException {
Preconditions.checkState(isEnabled());

assertKeyIsValid(cacheKey, keyType);
Expand All @@ -238,8 +229,7 @@ public synchronized void put(
}
}

public synchronized String put(Path sourcePath, KeyType keyType)
throws IOException, InterruptedException {
public synchronized String put(Path sourcePath, KeyType keyType) throws IOException {
return put(sourcePath, keyType, null);
}

Expand All @@ -254,7 +244,7 @@ public synchronized String put(Path sourcePath, KeyType keyType)
* @return The key for the cached entry.
*/
public synchronized String put(Path sourcePath, KeyType keyType, String canonicalId)
throws IOException, InterruptedException {
throws IOException {
String cacheKey = getChecksum(keyType, sourcePath);
put(cacheKey, sourcePath, keyType, canonicalId);
return cacheKey;
Expand All @@ -273,11 +263,11 @@ private void ensureCacheDirectoryExists(KeyType keyType) throws IOException {
* @param expectedChecksum The expected checksum of the file.
* @param filePath The path to the file.
* @param keyType The type of hash function. e.g. SHA-1, SHA-256
* @throws IOException If the checksum does not match or the file cannot be hashed, an exception
* is thrown.
* @throws IOException If the checksum does not match or the file cannot be hashed, an
* exception is thrown.
*/
public static void assertFileChecksum(String expectedChecksum, Path filePath, KeyType keyType)
throws IOException, InterruptedException {
throws IOException {
Preconditions.checkArgument(!expectedChecksum.isEmpty());

String actualChecksum;
Expand All @@ -302,8 +292,7 @@ public static void assertFileChecksum(String expectedChecksum, Path filePath, Ke
* @param path The path to the file.
* @throws IOException
*/
public static String getChecksum(KeyType keyType, Path path)
throws IOException, InterruptedException {
public static String getChecksum(KeyType keyType, Path path) throws IOException {
Hasher hasher = keyType.newHasher();
byte[] byteBuffer = new byte[BUFFER_SIZE];
try (InputStream stream = path.getInputStream()) {
Expand All @@ -313,9 +302,6 @@ public static String getChecksum(KeyType keyType, Path path)
// If more than 0 bytes were read, add them to the hash.
hasher.putBytes(byteBuffer, 0, numBytesRead);
}
if (Thread.interrupted()) {
throw new InterruptedException();
}
numBytesRead = stream.read(byteBuffer);
}
}
Expand Down
36 changes: 21 additions & 15 deletions ...c/test/java/com/google/devtools/build/lib/bazel/repository/cache/RepositoryCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,11 @@ public void testNonExistentCacheValue() {
assertThat(repositoryCache.exists(fakeSha256, KeyType.SHA256)).isFalse();
}

/** Test that the put method correctly stores the downloaded file into the cache. */
/**
* Test that the put method correctly stores the downloaded file into the cache.
*/
@Test
public void testPutCacheValue() throws Exception {
public void testPutCacheValue() throws IOException {
repositoryCache.put(downloadedFileSha256, downloadedFile, KeyType.SHA256);

Path cacheEntry = KeyType.SHA256.getCachePath(contentAddressableCachePath).getChild(downloadedFileSha256);
Expand All @@ -85,7 +87,7 @@ public void testPutCacheValue() throws Exception {
* Test that the put mehtod without cache key correctly stores the downloaded file into the cache.
*/
@Test
public void testPutCacheValueWithoutHash() throws Exception {
public void testPutCacheValueWithoutHash() throws IOException {
String cacheKey = repositoryCache.put(downloadedFile, KeyType.SHA256);
assertThat(cacheKey).isEqualTo(downloadedFileSha256);

Expand All @@ -98,11 +100,11 @@ public void testPutCacheValueWithoutHash() throws Exception {
}

/**
* Test that the put method is idempotent, i.e. two successive put calls should not affect the
* final state in the cache.
* Test that the put method is idempotent, i.e. two successive put calls
* should not affect the final state in the cache.
*/
@Test
public void testPutCacheValueIdempotent() throws Exception {
public void testPutCacheValueIdempotent() throws IOException {
repositoryCache.put(downloadedFileSha256, downloadedFile, KeyType.SHA256);
repositoryCache.put(downloadedFileSha256, downloadedFile, KeyType.SHA256);

Expand All @@ -113,9 +115,11 @@ public void testPutCacheValueIdempotent() throws Exception {
.isEqualTo(FileSystemUtils.readContent(cacheValue, Charset.defaultCharset()));
}

/** Test that the get method correctly retrieves the cached file from the cache. */
/**
* Test that the get method correctly retrieves the cached file from the cache.
*/
@Test
public void testGetCacheValue() throws Exception {
public void testGetCacheValue() throws IOException {
// Inject file into cache
repositoryCache.put(downloadedFileSha256, downloadedFile, KeyType.SHA256);

Expand All @@ -131,9 +135,11 @@ public void testGetCacheValue() throws Exception {
assertThat((Object) actualTargetPath).isEqualTo(targetPath);
}

/** Test that the get method retrieves a null if the value is not cached. */
/**
* Test that the get method retrieves a null if the value is not cached.
*/
@Test
public void testGetNullCacheValue() throws Exception {
public void testGetNullCacheValue() throws IOException {
Path targetDirectory = scratch.dir("/external");
Path targetPath = targetDirectory.getChild(downloadedFile.getBaseName());
Path actualTargetPath = repositoryCache.get(downloadedFileSha256, targetPath, KeyType.SHA256);
Expand All @@ -142,15 +148,15 @@ public void testGetNullCacheValue() throws Exception {
}

@Test
public void testInvalidSha256Throws() throws Exception {
public void testInvalidSha256Throws() throws IOException {
String invalidSha = "foo";
thrown.expect(IOException.class);
thrown.expectMessage("Invalid key \"foo\" of type SHA-256");
repositoryCache.put(invalidSha, downloadedFile, KeyType.SHA256);
}

@Test
public void testPoisonedCache() throws Exception {
public void testPoisonedCache() throws IOException {
Path poisonedEntry = KeyType.SHA256
.getCachePath(contentAddressableCachePath).getChild(downloadedFileSha256);
Path poisonedValue = poisonedEntry.getChild(RepositoryCache.DEFAULT_CACHE_FILENAME);
Expand All @@ -167,18 +173,18 @@ public void testPoisonedCache() throws Exception {
}

@Test
public void testGetChecksum() throws Exception {
public void testGetChecksum() throws IOException {
String actualChecksum = RepositoryCache.getChecksum(KeyType.SHA256, downloadedFile);
assertThat(actualChecksum).isEqualTo(downloadedFileSha256);
}

@Test
public void testAssertFileChecksumPass() throws Exception {
public void testAssertFileChecksumPass() throws IOException {
RepositoryCache.assertFileChecksum(downloadedFileSha256, downloadedFile, KeyType.SHA256);
}

@Test
public void testAssertFileChecksumFail() throws Exception {
public void testAssertFileChecksumFail() throws IOException {
thrown.expect(IOException.class);
thrown.expectMessage("does not match expected");
RepositoryCache.assertFileChecksum(
Expand Down

0 comments on commit 21e16ae

Please sign in to comment.