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: reverted deprecated fields on testSuite #19284

Merged
merged 1 commit into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading