Treat missing default-base-location in UpdateCatalogRequest as no-change to default-base-location

polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java

@@ -26,6 +26,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.iceberg.catalog.Namespace;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.exceptions.AlreadyExistsException;
@@ -674,8 +675,23 @@ private void validateUpdateCatalogDiffOrThrow(
     String defaultBaseLocation = currentCatalogEntity.getDefaultBaseLocation();
     if (updateRequest.getProperties() != null) {
       updateBuilder.setProperties(updateRequest.getProperties());
-      defaultBaseLocation =
+      String newDefaultBaseLocation =
           updateRequest.getProperties().get(CatalogEntity.DEFAULT_BASE_LOCATION_KEY);
+      // Since defaultBaseLocation is a required field during construction of a catalog, and the
+      // syntax of the Catalog API model splits default-base-location out from other keys in
+      // additionalProperties, it's easy for client libraries to focus on adding/merging
+      // additionalProperties while neglecting to "echo" the default-base-location from the
+      // fetched catalog, it's most user-friendly to treat a null or empty default-base-location
+      // as meaning no intended change to the default-base-location.
+      if (StringUtils.isNotEmpty(newDefaultBaseLocation)) {
+        // New base location is already in the updated properties; we'll also potentially
+        // plumb it into the logic for setting an updated StorageConfigurationInfo.
+        defaultBaseLocation = newDefaultBaseLocation;
+      } else {
+        // No default-base-location present at all in the properties of the update request,
+        // so we must restore it explicitly in the updateBuilder.
+        updateBuilder.setDefaultBaseLocation(defaultBaseLocation);
+      }
     }
     if (updateRequest.getStorageConfigInfo() != null) {
       updateBuilder.setStorageConfigurationInfo(

polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisServiceImplIntegrationTest.java

@@ -601,6 +601,66 @@ public void testCreateCatalogWithDisallowedStorageConfig() throws JsonProcessing
     }
   }
 
+  @Test
+  public void testUpdateCatalogWithoutDefaultBaseLocationInUpdate() throws JsonProcessingException {
+    AwsStorageConfigInfo awsConfigModel =
+        AwsStorageConfigInfo.builder()
+            .setRoleArn("arn:aws:iam::123456789012:role/my-role")
+            .setExternalId("externalId")
+            .setUserArn("userArn")
+            .setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
+            .setAllowedLocations(List.of("s3://my-old-bucket/path/to/data"))
+            .build();
+    String catalogName = "mycatalog";
+    Catalog catalog =
+        PolarisCatalog.builder()
+            .setType(Catalog.TypeEnum.INTERNAL)
+            .setName(catalogName)
+            .setProperties(new CatalogProperties("s3://bucket/path/to/data"))
+            .setStorageConfigInfo(awsConfigModel)
+            .build();
+    try (Response response =
+        newRequest("http://localhost:%d/api/management/v1/catalogs", userToken)
+            .post(Entity.json(new CreateCatalogRequest(catalog)))) {
+      assertThat(response).returns(Response.Status.CREATED.getStatusCode(), Response::getStatus);
+    }
+
+    // 200 successful GET after creation
+    Catalog fetchedCatalog = null;
+    try (Response response =
+        newRequest("http://localhost:%d/api/management/v1/catalogs/" + catalogName, userToken)
+            .get()) {
+      assertThat(response).returns(Response.Status.OK.getStatusCode(), Response::getStatus);
+      fetchedCatalog = response.readEntity(Catalog.class);
+
+      assertThat(fetchedCatalog.getName()).isEqualTo(catalogName);
+      assertThat(fetchedCatalog.getProperties().toMap())
+          .isEqualTo(Map.of("default-base-location", "s3://bucket/path/to/data"));
+      assertThat(fetchedCatalog.getEntityVersion()).isGreaterThan(0);
+    }
+
+    // Create an UpdateCatalogRequest that only sets a new property foo=bar but omits
+    // default-base-location.
+    UpdateCatalogRequest updateRequest =
+        new UpdateCatalogRequest(
+            fetchedCatalog.getEntityVersion(), Map.of("foo", "bar"), null /* storageConfigIno */);
+
+    // Successfully update
+    Catalog updatedCatalog = null;
+    try (Response response =
+        newRequest("http://localhost:%d/api/management/v1/catalogs/" + catalogName, userToken)
+            .put(Entity.json(updateRequest))) {
+      assertThat(response).returns(Response.Status.OK.getStatusCode(), Response::getStatus);
+      updatedCatalog = response.readEntity(Catalog.class);
+
+      assertThat(updatedCatalog.getName()).isEqualTo(catalogName);
+      // Check that default-base-location is preserved in addition to adding the new property
+      assertThat(updatedCatalog.getProperties().toMap())
+          .isEqualTo(Map.of("default-base-location", "s3://bucket/path/to/data", "foo", "bar"));
+      assertThat(updatedCatalog.getEntityVersion()).isGreaterThan(0);
+    }
+  }
+
   @Test
   public void testUpdateCatalogWithDisallowedStorageConfig() throws JsonProcessingException {
     AwsStorageConfigInfo awsConfigModel =