From a4a2dcc91dde57af89fa8fe2862a5f2668ddd5fd Mon Sep 17 00:00:00 2001
From: Sriharsha Chintalapani <harsha@getcollate.io>
Date: Tue, 21 Apr 2026 16:04:01 -0700
Subject: [PATCH] NoOpAssetService: never return null from
 generateDownloadUrlWithExpiry
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In review round 4 I changed this method to return asset.getUrl() when
the asset is non-null. But Asset.url is optional in the schema, so
asset.getUrl() itself can be null — which breaks the implied "never
returns null" contract downstream callers rely on (AttachmentResource
only null-checks defensively).

Normalize null and blank URLs to an empty string so the method's
non-null, non-blank contract holds even when storage is disabled and
the asset was never populated with a URL.
---
 .../service/attachments/NoOpAssetService.java     | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/attachments/NoOpAssetService.java b/openmetadata-service/src/main/java/org/openmetadata/service/attachments/NoOpAssetService.java
index 765f1291b28..4de20b27079 100644
--- a/openmetadata-service/src/main/java/org/openmetadata/service/attachments/NoOpAssetService.java
+++ b/openmetadata-service/src/main/java/org/openmetadata/service/attachments/NoOpAssetService.java
@@ -22,12 +22,19 @@ public class NoOpAssetService implements AssetService {
   }
 
   /**
-   * Return the asset's own URL (which is empty when object storage is disabled) instead
-   * of a synthetic CDN URL. Returning a fake URL would let clients issue downloads that
-   * can never succeed and would mask the misconfiguration.
+   * Return the asset's own URL when present, otherwise an empty string. We deliberately
+   * avoid returning a synthetic CDN URL here — a fake URL would let clients issue
+   * downloads that can never succeed and would mask the "storage disabled"
+   * misconfiguration. {@link org.openmetadata.schema.attachments.Asset#getUrl()} is
+   * optional in the schema, so normalize null/blank to "" to preserve the non-null
+   * contract callers rely on.
    */
   @Override
   public String generateDownloadUrlWithExpiry(Asset asset, Duration expiry) {
-    return asset == null ? "" : asset.getUrl();
+    if (asset == null) {
+      return "";
+    }
+    String url = asset.getUrl();
+    return url == null || url.isBlank() ? "" : url;
   }
 }