Skip to content

Commit 5fd2deb

Browse files
Copilotjonesbusy
andcommitted
refactor: extract canMount to OCI, remove instanceof from CopyUtils, getBlobsMountPath takes ContainerRef, OCILayout.mountBlob throws on missing source
- Add abstract boolean canMount(OCI<?> other) to OCI<T> - Registry.canMount: true when source is also a Registry - OCILayout.canMount: true when source is also an OCILayout - OCILayout.mountBlob now throws OrasException when source blob not found (void-like) - ContainerRef.getBlobsMountPath now takes ContainerRef instead of String - CopyUtils: add private canMount() helper delegating to OCI.canMount - CopyUtils: tryMountBlob no longer references Registry/OCILayout/ContainerRef/LayoutRef - Update OCILayout tests for new exception-based behavior Co-authored-by: jonesbusy <[email protected]>
1 parent 2a70b02 commit 5fd2deb

6 files changed

Lines changed: 53 additions & 44 deletions

File tree

src/main/java/land/oras/ContainerRef.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,15 +380,16 @@ public String getBlobsUploadPath(Registry registry) {
380380
/**
381381
* Return the blobs mount URL for cross-repository blob mounting
382382
* @param registry The registry
383-
* @param sourceRepository The source repository to mount the blob from
383+
* @param sourceRef The source container reference to mount the blob from
384384
* @return The blobs mount URL
385385
*/
386-
public String getBlobsMountPath(Registry registry, String sourceRepository) {
386+
public String getBlobsMountPath(Registry registry, ContainerRef sourceRef) {
387387
if (digest == null) {
388388
throw new OrasException("You are required to include a digest");
389389
}
390390
return "%s/blobs/uploads/?mount=%s&from=%s".formatted(
391-
getApiPrefix(registry), digest, URLEncoder.encode(sourceRepository, StandardCharsets.UTF_8));
391+
getApiPrefix(registry), digest,
392+
URLEncoder.encode(sourceRef.getFullRepository(registry), StandardCharsets.UTF_8));
392393
}
393394

394395
/**

src/main/java/land/oras/CopyUtils.java

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,7 @@ void copyConfig(
286286

287287
/**
288288
* Attempt to mount a blob from source to target without downloading and re-uploading.
289-
* Mounting is only attempted when source and target are the same OCI type and,
290-
* for registries, when they share the same registry host.
289+
* Mounting is only attempted when {@link #canMount(OCI, OCI)} returns {@code true}.
291290
* @param source The source OCI
292291
* @param sourceRef The source reference
293292
* @param target The target OCI
@@ -307,29 +306,24 @@ boolean tryMountBlob(
307306
OCI<TargetRefType> target,
308307
TargetRefType targetRef,
309308
String digest) {
310-
// Registry-to-Registry mounting: only when pointing at the same registry host
311-
if (source instanceof Registry sourceRegistry && target instanceof Registry targetRegistry) {
312-
ContainerRef srcRef = (ContainerRef) sourceRef;
313-
ContainerRef tgtRef = (ContainerRef) targetRef;
314-
String sourceApiRegistry = srcRef.getApiRegistry(sourceRegistry);
315-
String targetApiRegistry = tgtRef.getApiRegistry(targetRegistry);
316-
if (sourceApiRegistry.equals(targetApiRegistry)) {
317-
ContainerRef layerSrcRef = srcRef.withDigest(digest);
318-
ContainerRef layerTgtRef = tgtRef.withDigest(digest);
319-
LOG.debug("Attempting mount of {} from {} to {}", digest, srcRef.getFullRepository(),
320-
tgtRef.getFullRepository());
321-
return targetRegistry.mountBlob(layerTgtRef, layerSrcRef);
322-
}
323-
}
324-
// OCILayout-to-OCILayout mounting: direct file copy between layouts
325-
if (source instanceof OCILayout && target instanceof OCILayout targetLayout) {
326-
LayoutRef srcRef = (LayoutRef) sourceRef;
327-
LayoutRef tgtRef = (LayoutRef) targetRef;
328-
LayoutRef layerSrcRef = srcRef.withDigest(digest);
329-
LayoutRef layerTgtRef = tgtRef.withDigest(digest);
330-
LOG.debug("Attempting mount of {} from {} to {}", digest, srcRef.getFolder(), tgtRef.getFolder());
331-
return targetLayout.mountBlob(layerTgtRef, layerSrcRef);
309+
if (!canMount(source, target)) {
310+
return false;
332311
}
333-
return false;
312+
// canMount guarantees source and target are the same OCI type, so the cast is safe
313+
TargetRefType sourceDigestRef = (TargetRefType) sourceRef.withDigest(digest);
314+
LOG.debug("Attempting mount of {} from {} to {}", digest, sourceRef.getRepository(), targetRef.getRepository());
315+
return target.mountBlob(targetRef.withDigest(digest), sourceDigestRef);
316+
}
317+
318+
/**
319+
* Return whether mounting is supported between the given source and target OCI instances.
320+
* This delegates to {@link OCI#canMount(OCI)} on the target so that each OCI implementation
321+
* defines its own compatibility rules without {@link CopyUtils} needing to know about concrete types.
322+
* @param source The source OCI instance
323+
* @param target The target OCI instance
324+
* @return {@code true} if the target supports mounting blobs from the source
325+
*/
326+
private static boolean canMount(OCI<?> source, OCI<?> target) {
327+
return target.canMount(source);
334328
}
335329
}

src/main/java/land/oras/OCI.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,15 @@ public abstract Manifest pushArtifact(
400400
*/
401401
public abstract boolean mountBlob(T targetRef, T sourceRef);
402402

403+
/**
404+
* Return whether this OCI instance supports mounting blobs from the given source OCI instance.
405+
* Implementations should return {@code true} only when mounting is viable, e.g. both are
406+
* registries sharing the same host, or both are OCI layouts on the local filesystem.
407+
* @param other The source OCI instance to check compatibility with
408+
* @return {@code true} if mounting from {@code other} is supported
409+
*/
410+
public abstract boolean canMount(OCI<?> other);
411+
403412
/**
404413
* Get the referrers of a container
405414
* @param ref The ref

src/main/java/land/oras/OCILayout.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,8 +350,7 @@ public boolean mountBlob(LayoutRef targetRef, LayoutRef sourceRef) {
350350
.resolve(algorithm.getPrefix())
351351
.resolve(SupportedAlgorithm.getDigest(digest));
352352
if (!Files.exists(sourceBlobPath)) {
353-
LOG.info("Source blob not found at {}, upload required", sourceBlobPath);
354-
return false;
353+
throw new OrasException("Source blob not found at: %s".formatted(sourceBlobPath));
355354
}
356355
try {
357356
Files.copy(sourceBlobPath, targetBlobPath);
@@ -362,6 +361,11 @@ public boolean mountBlob(LayoutRef targetRef, LayoutRef sourceRef) {
362361
}
363362
}
364363

364+
@Override
365+
public boolean canMount(OCI<?> other) {
366+
return other instanceof OCILayout;
367+
}
368+
365369
@Override
366370
public Tags getTags(LayoutRef ref) {
367371
Index index = Index.fromPath(getIndexPath());

src/main/java/land/oras/Registry.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -670,9 +670,8 @@ public boolean mountBlob(ContainerRef targetRef, ContainerRef sourceRef) {
670670
LOG.info("Blob already exists: {}", digest);
671671
return true;
672672
}
673-
String sourceRepository = sourceRef.getFullRepository();
674673
URI uri = URI.create(
675-
"%s://%s".formatted(getScheme(), ref.getBlobsMountPath(this, sourceRepository)));
674+
"%s://%s".formatted(getScheme(), ref.getBlobsMountPath(this, sourceRef)));
676675
HttpClient.ResponseWrapper<String> response = client.post(
677676
uri,
678677
new byte[0],
@@ -681,17 +680,22 @@ public boolean mountBlob(ContainerRef targetRef, ContainerRef sourceRef) {
681680
authProvider);
682681
logResponse(response);
683682
if (response.statusCode() == 201) {
684-
LOG.info("Blob mounted successfully from {}: {}", sourceRepository, digest);
683+
LOG.info("Blob mounted successfully from {}: {}", sourceRef.getFullRepository(), digest);
685684
return true;
686685
}
687686
if (response.statusCode() == 202) {
688-
LOG.info("Mount not possible for blob {} from {}, upload required", digest, sourceRepository);
687+
LOG.info("Mount not possible for blob {} from {}, upload required", digest, sourceRef.getFullRepository());
689688
return false;
690689
}
691690
handleError(response);
692691
return false;
693692
}
694693

694+
@Override
695+
public boolean canMount(OCI<?> other) {
696+
return other instanceof Registry;
697+
}
698+
695699
/**
696700
* Return if the registry contains already the blob
697701
* @param containerRef The container

src/test/java/land/oras/OCILayoutTest.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,16 +1138,15 @@ void shouldMountBlobFromAnotherLayout() throws IOException {
11381138
// Mount blob into target layout
11391139
OCILayout targetLayout = OCILayout.Builder.builder().defaults(targetPath).build();
11401140
LayoutRef targetRef = LayoutRef.of(targetLayout, digest);
1141-
boolean mounted = targetLayout.mountBlob(targetRef, sourceRef);
1141+
targetLayout.mountBlob(targetRef, sourceRef);
11421142

11431143
// Assert mount succeeded
1144-
assertTrue(mounted, "Blob should be mounted successfully");
11451144
assertBlobExists(targetPath, digest);
11461145
assertBlobContent(targetPath, digest, "mount-test-content");
11471146
}
11481147

11491148
@Test
1150-
void shouldMountBlobReturnTrueIfAlreadyExists() {
1149+
void shouldMountBlobIdempotentIfAlreadyExists() {
11511150

11521151
Path layoutPathDir = layoutPath.resolve("shouldMountBlobExisting");
11531152

@@ -1161,13 +1160,12 @@ void shouldMountBlobReturnTrueIfAlreadyExists() {
11611160
ociLayout.pushBlob(ref, content);
11621161
assertBlobExists(layoutPathDir, digest);
11631162

1164-
// Mounting again should return true (already exists)
1165-
boolean mounted = ociLayout.mountBlob(ref, ref);
1166-
assertTrue(mounted, "Mount should return true when blob already exists");
1163+
// Mounting again should be a no-op (already exists)
1164+
assertDoesNotThrow(() -> ociLayout.mountBlob(ref, ref));
11671165
}
11681166

11691167
@Test
1170-
void shouldMountBlobReturnFalseWhenSourceNotFound() {
1168+
void shouldThrowWhenMountingBlobWithMissingSource() {
11711169

11721170
Path sourcePath = layoutPath.resolve("mountMissingSource");
11731171
Path targetPath = layoutPath.resolve("mountMissingTarget");
@@ -1180,9 +1178,8 @@ void shouldMountBlobReturnFalseWhenSourceNotFound() {
11801178
LayoutRef sourceRef = LayoutRef.of(sourceLayout, digest);
11811179
LayoutRef targetRef = LayoutRef.of(targetLayout, digest);
11821180

1183-
// Mounting a non-existent blob should return false
1184-
boolean mounted = targetLayout.mountBlob(targetRef, sourceRef);
1185-
assertFalse(mounted, "Mount should return false when source blob does not exist");
1181+
// Mounting a non-existent blob should throw OrasException
1182+
assertThrows(OrasException.class, () -> targetLayout.mountBlob(targetRef, sourceRef));
11861183
assertBlobAbsent(targetPath, digest);
11871184
}
11881185
}

0 commit comments

Comments
 (0)