Skip to content

Commit

Permalink
fix: reverted deprecated fields on testSuite (open-metadata#19284)
Browse files Browse the repository at this point in the history
Added deprecated fields with a notice. This is to ensure migrations that use them do not break.
  • Loading branch information
sushi30 authored Jan 9, 2025
1 parent 78e8d36 commit 7d05902
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package org.openmetadata.annotations;

import com.fasterxml.jackson.databind.JsonNode;
import com.sun.codemodel.JAnnotationUse;
import com.sun.codemodel.JClass;
import com.sun.codemodel.JDefinedClass;
import com.sun.codemodel.JFieldVar;
import com.sun.codemodel.JMethod;
import java.lang.reflect.Field;
import java.util.TreeMap;
import org.jsonschema2pojo.AbstractAnnotator;

/** Add {@link Deprecated} annotation to generated Java classes */
public class DeprecatedAnnotator extends AbstractAnnotator {

/** Add {@link Deprecated} annotation to property fields */
@Override
public void propertyField(
JFieldVar field, JDefinedClass clazz, String propertyName, JsonNode propertyNode) {
super.propertyField(field, clazz, propertyName, propertyNode);
if (propertyNode.get("deprecated") != null && propertyNode.get("deprecated").asBoolean()) {
field.annotate(Deprecated.class);
}
}

/** Add {@link Deprecated} annotation to getter methods */
@Override
public void propertyGetter(JMethod getter, JDefinedClass clazz, String propertyName) {
super.propertyGetter(getter, clazz, propertyName);
addDeprecatedAnnotationIfApplies(getter, propertyName);
}

/** Add {@link Deprecated} annotation to setter methods */
@Override
public void propertySetter(JMethod setter, JDefinedClass clazz, String propertyName) {
super.propertySetter(setter, clazz, propertyName);
addDeprecatedAnnotationIfApplies(setter, propertyName);
}

/**
* Use reflection methods to access the {@link JDefinedClass} of the {@link JMethod} object. If
* the {@link JMethod} is pointing to a field annotated with {@link Deprecated} then annotates
* the {@link JMethod} object with {@link Deprecated}
*/
private void addDeprecatedAnnotationIfApplies(JMethod jMethod, String propertyName) {
try {
Field outerClassField = JMethod.class.getDeclaredField("outer");
outerClassField.setAccessible(true);
JDefinedClass outerClass = (JDefinedClass) outerClassField.get(jMethod);

TreeMap<String, JFieldVar> insensitiveFieldsMap =
new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
insensitiveFieldsMap.putAll(outerClass.fields());

if (insensitiveFieldsMap.containsKey(propertyName)
&& insensitiveFieldsMap.get(propertyName).annotations().stream()
.anyMatch(
annotation ->
Deprecated.class.getName().equals(getAnnotationClassName(annotation)))) {
jMethod.annotate(Deprecated.class);
}
} catch (NoSuchFieldException | IllegalAccessException e) {
throw new RuntimeException(e);
}
}

private String getAnnotationClassName(JAnnotationUse annotation) {
try {
Field clazzField = JAnnotationUse.class.getDeclaredField("clazz");
clazzField.setAccessible(true);
return ((JClass) clazzField.get(annotation)).fullName();
} catch (NoSuchFieldException | IllegalAccessException e) {
throw new RuntimeException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ public class OpenMetadataAnnotator extends CompositeAnnotator {

public OpenMetadataAnnotator() {
// we can add multiple annotators
super(new ExposedAnnotator(), new MaskedAnnotator(), new PasswordAnnotator());
super(
new ExposedAnnotator(),
new MaskedAnnotator(),
new PasswordAnnotator(),
new DeprecatedAnnotator());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ public static void fixExecutableTestSuiteFQN(CollectionDAO collectionDAO) {
testSuiteRepository.listAll(
new EntityUtil.Fields(Set.of("id")), new ListFilter(Include.ALL));
for (TestSuite suite : testSuites) {
if (Boolean.TRUE.equals(suite.getBasic()) && suite.getBasicEntityReference() != null) {
String tableFQN = suite.getBasicEntityReference().getFullyQualifiedName();
if (Boolean.TRUE.equals(suite.getExecutable())
&& suite.getExecutableEntityReference() != null) {
String tableFQN = suite.getExecutableEntityReference().getFullyQualifiedName();
String suiteFQN = tableFQN + ".testSuite";
suite.setName(suiteFQN);
suite.setFullyQualifiedName(suiteFQN);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ public static void fixTestSuites(CollectionDAO collectionDAO) {
testSuiteRepository.listAll(
new EntityUtil.Fields(Set.of("id")), new ListFilter(Include.ALL));
for (TestSuite suite : testSuites) {
if (suite.getBasicEntityReference() != null
&& (!suite.getBasic() || !suite.getFullyQualifiedName().contains("testSuite"))) {
String tableFQN = suite.getBasicEntityReference().getFullyQualifiedName();
if (suite.getExecutableEntityReference() != null
&& (!suite.getExecutable() || !suite.getFullyQualifiedName().contains("testSuite"))) {
String tableFQN = suite.getExecutableEntityReference().getFullyQualifiedName();
String suiteFQN = tableFQN + ".testSuite";
suite.setName(suiteFQN);
suite.setFullyQualifiedName(suiteFQN);
suite.setBasic(true);
suite.setExecutable(true);
collectionDAO.testSuiteDAO().update(suite);
}
}
Expand All @@ -80,7 +80,7 @@ public static void fixTestSuites(CollectionDAO collectionDAO) {
try {
TestSuite existingTestSuite =
testSuiteRepository.getDao().findEntityById(existingTestSuiteRel.getId());
if (Boolean.TRUE.equals(existingTestSuite.getBasic())
if (Boolean.TRUE.equals(existingTestSuite.getExecutable())
&& existingTestSuite.getFullyQualifiedName().equals(executableTestSuiteFQN)) {
// There is a native test suite associated with this testCase.
relationWithExecutableTestSuiteExists = true;
Expand Down Expand Up @@ -111,7 +111,7 @@ public static void fixTestSuites(CollectionDAO collectionDAO) {
// check from table -> nativeTestSuite there should only one relation
List<CollectionDAO.EntityRelationshipRecord> testSuiteRels =
testSuiteRepository.findToRecords(
executableTestSuite.getBasicEntityReference().getId(),
executableTestSuite.getExecutableEntityReference().getId(),
TABLE,
Relationship.CONTAINS,
TEST_SUITE);
Expand All @@ -122,7 +122,7 @@ public static void fixTestSuites(CollectionDAO collectionDAO) {
// if testsuite cannot be retrieved but the relation exists, then this is orphaned
// relation, we will delete the relation
testSuiteRepository.deleteRelationship(
executableTestSuite.getBasicEntityReference().getId(),
executableTestSuite.getExecutableEntityReference().getId(),
TABLE,
testSuiteRel.getId(),
TEST_SUITE,
Expand Down Expand Up @@ -158,9 +158,9 @@ private static TestSuite getOrCreateExecutableTestSuite(
new CreateTestSuite()
.withName(FullyQualifiedName.buildHash(executableTestSuiteFQN))
.withDisplayName(executableTestSuiteFQN)
.withBasicEntityReference(entityLink.getEntityFQN()),
.withExecutableEntityReference(entityLink.getEntityFQN()),
"ingestion-bot")
.withBasic(true)
.withExecutable(true)
.withFullyQualifiedName(executableTestSuiteFQN);
testSuiteRepository.prepareInternal(newExecutableTestSuite, false);
testSuiteRepository
Expand All @@ -169,7 +169,7 @@ private static TestSuite getOrCreateExecutableTestSuite(
"fqnHash", newExecutableTestSuite, newExecutableTestSuite.getFullyQualifiedName());
// add relationship between executable TestSuite with Table
testSuiteRepository.addRelationship(
newExecutableTestSuite.getBasicEntityReference().getId(),
newExecutableTestSuite.getExecutableEntityReference().getId(),
newExecutableTestSuite.getId(),
Entity.TABLE,
TEST_SUITE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,16 +460,17 @@ public static TestSuite getTestSuite(CollectionDAO dao, CreateTestSuite create,
.withDescription(create.getDescription())
.withDisplayName(create.getDisplayName())
.withName(create.getName());
if (create.getBasicEntityReference() != null) {
if (create.getExecutableEntityReference() != null) {
Table table =
Entity.getEntityByName(Entity.TABLE, create.getBasicEntityReference(), "", Include.ALL);
Entity.getEntityByName(
Entity.TABLE, create.getExecutableEntityReference(), "", Include.ALL);
EntityReference entityReference =
new EntityReference()
.withId(table.getId())
.withFullyQualifiedName(table.getFullyQualifiedName())
.withName(table.getName())
.withType(Entity.TABLE);
testSuite.setBasicEntityReference(entityReference);
testSuite.setExecutableEntityReference(entityReference);
}
return testSuite;
}
Expand Down Expand Up @@ -516,9 +517,9 @@ public static void testSuitesMigration(CollectionDAO collectionDAO) {
new CreateTestSuite()
.withName(FullyQualifiedName.buildHash(nativeTestSuiteFqn))
.withDisplayName(nativeTestSuiteFqn)
.withBasicEntityReference(entityLink.getEntityFQN()),
.withExecutableEntityReference(entityLink.getEntityFQN()),
"ingestion-bot")
.withBasic(true)
.withExecutable(true)
.withFullyQualifiedName(nativeTestSuiteFqn);
testSuiteRepository.prepareInternal(newExecutableTestSuite, false);
try {
Expand All @@ -533,7 +534,7 @@ public static void testSuitesMigration(CollectionDAO collectionDAO) {
}
// add relationship between executable TestSuite with Table
testSuiteRepository.addRelationship(
newExecutableTestSuite.getBasicEntityReference().getId(),
newExecutableTestSuite.getExecutableEntityReference().getId(),
newExecutableTestSuite.getId(),
Entity.TABLE,
TEST_SUITE,
Expand Down Expand Up @@ -564,7 +565,7 @@ private static void migrateExistingTestSuitesToLogical(CollectionDAO collectionD
ListFilter filter = new ListFilter(Include.ALL);
List<TestSuite> testSuites = testSuiteRepository.listAll(new Fields(Set.of("id")), filter);
for (TestSuite testSuite : testSuites) {
testSuite.setBasic(false);
testSuite.setExecutable(false);
List<CollectionDAO.EntityRelationshipRecord> ingestionPipelineRecords =
collectionDAO
.relationshipDAO()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,16 @@ public static void runTestSuiteMigration(
suite = JsonUtils.readValue(pgObject.getValue(), TestSuite.class);
}
// Only Test Suite which are executable needs to be updated
if (Boolean.TRUE.equals(suite.getBasic())) {
if (suite.getBasicEntityReference() != null) {
if (Boolean.TRUE.equals(suite.getExecutable())) {
if (suite.getExecutableEntityReference() != null) {
updateTestSuite(handle, suite, updateSql);
} else {
String entityName = StringUtils.replaceOnce(suite.getDisplayName(), ".testSuite", "");
try {
Table table = collectionDAO.tableDAO().findEntityByName(entityName, Include.ALL);
// Update Test Suite
suite.setBasic(true);
suite.setBasicEntityReference(table.getEntityReference());
suite.setExecutable(true);
suite.setExecutableEntityReference(table.getEntityReference());
updateTestSuite(handle, suite, updateSql);
removeDuplicateTestCases(collectionDAO, handle, getSql);
} catch (Exception ex) {
Expand All @@ -133,9 +133,9 @@ public static void runTestSuiteMigration(
}

public static void updateTestSuite(Handle handle, TestSuite suite, String updateSql) {
if (suite.getBasicEntityReference() != null) {
if (suite.getExecutableEntityReference() != null) {
try {
EntityReference executableEntityRef = suite.getBasicEntityReference();
EntityReference executableEntityRef = suite.getExecutableEntityReference();
// Run new Migrations
suite.setName(String.format("%s.testSuite", executableEntityRef.getName()));
suite.setFullyQualifiedName(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
"description": "Entity reference the test suite needs to execute the test against. Only applicable if the test suite is basic.",
"$ref": "../../type/basic.json#/definitions/fullyQualifiedEntityName"
},
"executableEntityReference": {
"description": "DEPRECATED in 1.6.2: use 'basicEntityReference'",
"$ref": "../../type/basic.json#/definitions/fullyQualifiedEntityName",
"deprecated": true
},
"domain": {
"description": "Fully qualified name of the domain the Table belongs to.",
"type": "string"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,20 @@
"type": "boolean",
"default": false
},
"executable": {
"description": "DEPRECATED in 1.6.2: Use 'basic'",
"type": "boolean",
"deprecated": true
},
"basicEntityReference": {
"description": "Entity reference the test suite needs to execute the test against. Only applicable if the test suite is basic.",
"$ref": "../type/entityReference.json"
},
"executableEntityReference": {
"description": "DEPRECATED in 1.6.2: Use 'basicEntityReference'.",
"$ref": "../type/entityReference.json",
"deprecated": true
},
"summary": {
"description": "Summary of the previous day test cases execution for this test suite.",
"$ref": "./basic.json#/definitions/testSummary"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ export interface CreateTestSuite {
* Fully qualified name of the domain the Table belongs to.
*/
domain?: string;
/**
* DEPRECATED in 1.6.2: use 'basicEntityReference'
*/
executableEntityReference?: string;
/**
* Name that identifies this test suite.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ export interface TestSuite {
* the table it belongs to.
*/
domain?: EntityReference;
/**
* DEPRECATED in 1.6.2: Use 'basic'
*/
executable?: boolean;
/**
* DEPRECATED in 1.6.2: Use 'basicEntityReference'.
*/
executableEntityReference?: EntityReference;
/**
* FullyQualifiedName same as `name`.
*/
Expand Down Expand Up @@ -127,6 +135,8 @@ export interface TestSuite {
* Domain the test Suite belongs to. When not set, the test Suite inherits the domain from
* the table it belongs to.
*
* DEPRECATED in 1.6.2: Use 'basicEntityReference'.
*
* Owners of this TestCase definition.
*
* This schema defines the EntityReferenceList type used for referencing an entity.
Expand Down

0 comments on commit 7d05902

Please sign in to comment.