Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Duplicate keys in JSON produced for Catalog objects #915 #955

Merged
merged 27 commits into from
Mar 5, 2025
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a582379
Using JsonTypeInfo.As.EXISTING_PROPERTY, making discriminator use sam…
vedansh-5 Feb 4, 2025
0d2b89f
created template file typeInfoAnnotation in server-templates, linked …
vedansh-5 Feb 5, 2025
49bcf5a
Added 6 tests
vedansh-5 Feb 6, 2025
b2bd13b
added 3 more tests
vedansh-5 Feb 6, 2025
f26edfd
All tests added
vedansh-5 Feb 6, 2025
3736684
Added readme file
vedansh-5 Feb 6, 2025
dd3d108
Formatted the code
vedansh-5 Feb 6, 2025
b9128ad
Assert exact JSON string, got rid of append calls, added comments bef…
vedansh-5 Feb 6, 2025
ac7463a
Added rountTrip method and using it in testcases
vedansh-5 Feb 6, 2025
55ca322
applied spotlessApply
vedansh-5 Feb 7, 2025
f0d4259
removed vscode settings, using string literals in tests
vedansh-5 Feb 7, 2025
f13a5a4
ran spotlessApply
vedansh-5 Feb 7, 2025
e767e2d
let's do assertThat(deserialized).isEqualTo(original)
vedansh-5 Feb 7, 2025
47ef32f
Parameterized Tests
vedansh-5 Feb 7, 2025
301badd
fine tuning
vedansh-5 Feb 7, 2025
16ad417
leaner build and test file
vedansh-5 Feb 8, 2025
c985f0a
recursiveComparing in catalog
vedansh-5 Feb 21, 2025
e202f6e
spotlessApply
vedansh-5 Feb 21, 2025
d08cc72
Delete .vscode directory
vedansh-5 Feb 21, 2025
623773b
Update .gitignore
vedansh-5 Feb 21, 2025
4e37832
type-field serialized once
vedansh-5 Feb 27, 2025
c1d3f03
Merge pull request #10 from vedansh-5/master
vedansh-5 Feb 27, 2025
644820c
inline verifyRoundTrip
vedansh-5 Feb 27, 2025
9c44b8a
added test jsonformat
vedansh-5 Feb 27, 2025
8374c50
Merge pull request #11 from vedansh-5/master
vedansh-5 Feb 27, 2025
8df8e28
formatting
vedansh-5 Feb 28, 2025
89ca9b5
Merge pull request #12 from vedansh-5/master
vedansh-5 Feb 28, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,4 @@ logs/
hs_err_pid*

# macOS
*.DS_Store
*.DS_Store
5 changes: 5 additions & 0 deletions api/management-model/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ dependencies {
compileOnly(libs.jakarta.annotation.api)
compileOnly(libs.jakarta.validation.api)
compileOnly(libs.swagger.annotations)

testImplementation(platform(libs.junit.bom))
testImplementation("org.junit.jupiter:junit-jupiter")
testImplementation(platform(libs.jackson.bom))
testImplementation("com.fasterxml.jackson.core:jackson-databind")
}

openApiGenerate {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.polaris.core.admin.model;

import static org.assertj.core.api.Assertions.assertThat;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.stream.Stream;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

public class CatalogSerializationTest {

private ObjectMapper mapper;
private static final String TEST_LOCATION = "s3://test/";
private static final String TEST_CATALOG_NAME = "test-catalog";
private static final String TEST_ROLE_ARN = "arn:aws:iam::123456789012:role/test-role";

@BeforeEach
public void setUp() {
mapper = new ObjectMapper();
}

/**
* Helper method to verify round-trip serialization/deserialization of Catalog objects. Ensures
* all fields are preserved correctly through the process.
*
* @param original The catalog object to test
* @return The deserialized catalog for additional assertions if needed
*/
private Catalog verifyRoundTrip(Catalog original) throws JsonProcessingException {
String json = mapper.writeValueAsString(original);
Catalog deserialized = mapper.readValue(json, Catalog.class);
assertThat(deserialized).usingRecursiveComparison().isEqualTo(original);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not simply assertThat(deserialized).isEqualTo(original)?

Do you think the .equals() method in Catalog does not take nested objects into account?

return deserialized;
}

@ParameterizedTest(name = "{0}")
@MethodSource("catalogTestCases")
public void testCatalogSerialization(String description, Catalog catalog)
throws JsonProcessingException {
verifyRoundTrip(catalog);
}

private static Stream<Arguments> catalogTestCases() {
Stream<Arguments> basicCases =
Stream.of(
Arguments.of(
"Basic catalog",
new Catalog(
Catalog.TypeEnum.INTERNAL,
TEST_CATALOG_NAME,
new CatalogProperties(TEST_LOCATION),
new AwsStorageConfigInfo(TEST_ROLE_ARN, StorageConfigInfo.StorageTypeEnum.S3))),
Arguments.of("Null fields", new Catalog(Catalog.TypeEnum.INTERNAL, null, null, null)),
Arguments.of(
"Long name",
new Catalog(
Catalog.TypeEnum.INTERNAL,
"a".repeat(1000),
new CatalogProperties(TEST_LOCATION),
null)),
Arguments.of(
"Unicode characters",
new Catalog(
Catalog.TypeEnum.INTERNAL, "测试目录", new CatalogProperties(TEST_LOCATION), null)),
Arguments.of(
"Empty strings",
new Catalog(
Catalog.TypeEnum.INTERNAL,
"",
new CatalogProperties(""),
new AwsStorageConfigInfo("", StorageConfigInfo.StorageTypeEnum.S3))),
Arguments.of(
"Special characters",
new Catalog(
Catalog.TypeEnum.INTERNAL,
"test\"catalog",
new CatalogProperties(TEST_LOCATION),
new AwsStorageConfigInfo(TEST_ROLE_ARN, StorageConfigInfo.StorageTypeEnum.S3))),
Arguments.of(
"Whitespace",
new Catalog(
Catalog.TypeEnum.INTERNAL,
" test catalog ",
new CatalogProperties(" " + TEST_LOCATION + " "),
null)));

Stream<Arguments> arnCases =
Stream.of(
"arn:aws:iam::123456789012:role/test-role",
"arn:aws:iam::123456789012:role/service-role/test-role",
"arn:aws:iam::123456789012:role/path/to/role")
.map(
arn ->
Arguments.of(
"ARN: " + arn,
new Catalog(
Catalog.TypeEnum.INTERNAL,
TEST_CATALOG_NAME,
new CatalogProperties(TEST_LOCATION),
new AwsStorageConfigInfo(arn, StorageConfigInfo.StorageTypeEnum.S3))));

return Stream.concat(basicCases, arnCases);
}
}
29 changes: 29 additions & 0 deletions server-templates/typeInfoAnnotation.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{{!
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
}}
@JsonTypeInfo(
use = JsonTypeInfo.Id.NAME,
include = JsonTypeInfo.As.EXISTING_PROPERTY,
property = "{{propertyName}}",
visible = true
)
@JsonSubTypes({
{{#mappedModels}}
@JsonSubTypes.Type(value = {{modelName}}.class, name = "{{mappingName}}"){{^-last}},{{/-last}}
{{/mappedModels}}
})
Copy link
Contributor

Choose a reason for hiding this comment

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

new line