Skip to content

Make storage type names case-insensitive - #5419

Open
zhang-arvin wants to merge 1 commit into
apache:mainfrom
zhang-arvin:fix/case-insensitive-storage-type-996
Open

zhang-arvin wants to merge 1 commit into
apache:mainfrom
zhang-arvin:fix/case-insensitive-storage-type-996

Conversation

@zhang-arvin

Copy link
Copy Markdown

What changes were proposed in this pull request?

Fixes #996: Storage type names are treated as case-sensitive (e.g., 's3' and 'S3' are treated differently), causing API calls to fail with "unsupported storage type" message.

Changes

Added a custom Jackson deserializer for StorageConfigInfo that normalizes the storageType field to uppercase during deserialization, so case variations like 's3', 'S3', 'gcs', 'GCS', 'azure', 'AZURE', 'file', 'FILE' are all accepted.

How was this patch tested?

  • Verified the deserializer is registered in the ObjectMapper configuration
  • The change is backward-compatible: existing uppercase values continue to work unchanged

@dimas-b dimas-b left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution, @zhang-arvin !

ObjectNode node = (ObjectNode) treeNode;
JsonNode storageTypeNode = node.get("storageType");
if (storageTypeNode != null && storageTypeNode.isTextual()) {
node.put("storageType", storageTypeNode.asText().toUpperCase());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use toUpperCase(Locale.ROOT) to avoid behaviour dependencies on the execution env.

Comment on lines +270 to +273
JsonParser treeParser = treeNode.traverse(p.getCodec());
treeParser.nextToken();
return ctxt.readValue(treeParser, StorageConfigInfo.class);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add unit tests, please?

@snazy

snazy commented Sep 7, 2026

Copy link
Copy Markdown
Member

See previous PR #1022.

The values of the storage types is well defined in the spec and are effectively an API contract.
I'd suggest to just use the uppercase values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using case insensitive storage type names

3 participants