From c2e1bdac2c23080b083fbf37a6f02dd225ebf505 Mon Sep 17 00:00:00 2001 From: Jakob Vogel Date: Wed, 18 Dec 2024 23:59:11 +0100 Subject: [PATCH] =?UTF-8?q?Fixes=20blob=20download=20on=20nodes=20without?= =?UTF-8?q?=20variant=20conversion=20=F0=9F=91=A8=E2=80=8D=F0=9F=9A=92?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When converting "virtual" blob information into physical keys, the framework generally distinguishes between blocking and non-blocking operation. In the former case, if a variant does not exist yet, the node is required to have conversion enabled as an exception would be thrown else. In the latter case of non-blocking operation, even on nodes with conversion enabled, no conversion would be attempted. This requires the invoker to explicitly deal with the case when conversion is required. Before, the `download` method simply used a blocking call, obviously assuming that it would return a `null` object in case conversion is disabled. This assumption is wrong. We need to make sure that the current node has conversion enabled before invoking a blocking call. For nodes without conversion, we need to delegate immediately. Note that there are several possible ways to implement this. We could also immediately check for whether conversion is enabled and directly resolve the physical key in a blocking manner. However, I personally consider the code as suggested more flexible, as it would allow to attempt to delegate even on a node with conversion if that conversion has failed gracefully for any reason. SIRI-1022 --- .../biz/storage/layer2/BasicBlobStorageSpace.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/main/java/sirius/biz/storage/layer2/BasicBlobStorageSpace.java b/src/main/java/sirius/biz/storage/layer2/BasicBlobStorageSpace.java index 3efb5efae..6e0a3275f 100644 --- a/src/main/java/sirius/biz/storage/layer2/BasicBlobStorageSpace.java +++ b/src/main/java/sirius/biz/storage/layer2/BasicBlobStorageSpace.java @@ -1074,8 +1074,20 @@ public Optional download(String blobKey, String variant) { touch(blobKey); try { - Tuple physicalKey = resolvePhysicalKey(blobKey, variant, false); + // first, attempt to resolve the physical key for the given blob and variant in a non-blocking manner; if + // the variant has been created before, independent of whether this node has conversion enabled, this will + // lead to a physical key that can be used to download the blob + Tuple physicalKey = resolvePhysicalKey(blobKey, variant, true); + + // if a physical key cannot be determined, the blob has not yet been created in the requested variant; we + // now need to check if this node is allowed to perform the conversion itself; if so, we run the physical + // key lookup again in a blocking manner to ensure that the conversion is performed + if (conversionEnabled && physicalKey == null) { + physicalKey = resolvePhysicalKey(blobKey, variant, false); + } + // if the physical key is still null, this node is not allowed to perform the conversion itself or + // conversion on this node failed gracefully; in this case, we attempt to delegate the conversion if (physicalKey == null) { return tryDelegateDownload(blobKey, variant, MAX_CONVERSION_DELEGATE_ATTEMPTS); }