diff --git a/bootstrap/sql/migrations/native/1.12.0/mysql/schemaChanges.sql b/bootstrap/sql/migrations/native/1.12.0/mysql/schemaChanges.sql index d3f07c9400e..00f5875ec99 100644 --- a/bootstrap/sql/migrations/native/1.12.0/mysql/schemaChanges.sql +++ b/bootstrap/sql/migrations/native/1.12.0/mysql/schemaChanges.sql @@ -30,3 +30,23 @@ CREATE TABLE IF NOT EXISTS audit_log_event ( KEY idx_audit_log_service_name_ts (service_name, event_ts DESC), KEY idx_audit_log_created_at (created_at) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci; + + + +-- Add enabled field to test_definition table for Rules Library feature +-- This allows administrators to enable/disable test definitions in the rules library + +-- Add virtual column for enabled field +-- CAST is needed to convert JSON boolean (true/false) to TINYINT (1/0) +ALTER TABLE test_definition + ADD COLUMN enabled TINYINT(1) + GENERATED ALWAYS AS (COALESCE(CAST(json_extract(json, '$.enabled') AS UNSIGNED), 1)) + VIRTUAL; + +-- Add index for filtering enabled/disabled test definitions +CREATE INDEX idx_test_definition_enabled ON test_definition(enabled); + +-- Set all existing test definitions to enabled by default +UPDATE test_definition + SET json = JSON_SET(json, '$.enabled', true) + WHERE json_extract(json, '$.enabled') IS NULL; diff --git a/bootstrap/sql/migrations/native/1.12.0/postgres/schemaChanges.sql b/bootstrap/sql/migrations/native/1.12.0/postgres/schemaChanges.sql index 853602d8250..1a45e60ed43 100644 --- a/bootstrap/sql/migrations/native/1.12.0/postgres/schemaChanges.sql +++ b/bootstrap/sql/migrations/native/1.12.0/postgres/schemaChanges.sql @@ -23,6 +23,29 @@ CREATE TABLE IF NOT EXISTS audit_log_event ( created_at BIGINT DEFAULT (EXTRACT(EPOCH FROM NOW()) * 1000)::BIGINT ); +-- Add indexes for efficient filtering +CREATE INDEX IF NOT EXISTS idx_audit_log_actor_type_ts ON audit_log_event (actor_type, event_ts DESC); +CREATE INDEX IF NOT EXISTS idx_audit_log_service_name_ts ON audit_log_event (service_name, event_ts DESC); +CREATE INDEX IF NOT EXISTS idx_audit_log_created_at ON audit_log_event (created_at); + + +-- Add enabled field to test_definition table for Rules Library feature +-- This allows administrators to enable/disable test definitions in the rules library + +-- Add generated column for enabled field with default true for existing rows +ALTER TABLE test_definition + ADD COLUMN IF NOT EXISTS enabled BOOLEAN GENERATED ALWAYS AS ( + COALESCE((json ->> 'enabled')::boolean, true) + ) STORED; + +-- Add index for filtering enabled/disabled test definitions +CREATE INDEX IF NOT EXISTS idx_test_definition_enabled ON test_definition(enabled); + +-- Set all existing test definitions to enabled by default +UPDATE test_definition + SET json = jsonb_set(json::jsonb, '{enabled}', 'true'::jsonb, true)::json + WHERE json ->> 'enabled' IS NULL; + CREATE UNIQUE INDEX IF NOT EXISTS idx_audit_log_event_change_event_id ON audit_log_event (change_event_id); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java index d8ce7967ea5..41fbf55fd87 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java @@ -6286,12 +6286,14 @@ public interface CollectionDAO { String testPlatform = filter.getQueryParam("testPlatform"); String supportedDataType = filter.getQueryParam("supportedDataType"); String supportedService = filter.getQueryParam("supportedService"); + String enabled = filter.getQueryParam("enabled"); String condition = filter.getCondition(); if (entityType == null && testPlatform == null && supportedDataType == null - && supportedService == null) { + && supportedService == null + && enabled == null) { return EntityDAO.super.listBefore(filter, limit, beforeName, beforeId); } @@ -6331,6 +6333,12 @@ public interface CollectionDAO { + "OR json->>'supportedServices' LIKE :supportedServiceLike) "); } + if (enabled != null) { + String enabledValue = Boolean.parseBoolean(enabled) ? "TRUE" : "FALSE"; + mysqlCondition.append("AND enabled=" + enabledValue + " "); + psqlCondition.append("AND enabled=" + enabledValue + " "); + } + return listBefore( getTableName(), filter.getQueryParams(), @@ -6347,12 +6355,14 @@ public interface CollectionDAO { String testPlatform = filter.getQueryParam("testPlatform"); String supportedDataType = filter.getQueryParam("supportedDataType"); String supportedService = filter.getQueryParam("supportedService"); + String enabled = filter.getQueryParam("enabled"); String condition = filter.getCondition(); if (entityType == null && testPlatform == null && supportedDataType == null - && supportedService == null) { + && supportedService == null + && enabled == null) { return EntityDAO.super.listAfter(filter, limit, afterName, afterId); } @@ -6392,6 +6402,12 @@ public interface CollectionDAO { + "OR json->>'supportedServices' LIKE :supportedServiceLike) "); } + if (enabled != null) { + String enabledValue = Boolean.parseBoolean(enabled) ? "TRUE" : "FALSE"; + mysqlCondition.append("AND enabled=" + enabledValue + " "); + psqlCondition.append("AND enabled=" + enabledValue + " "); + } + return listAfter( getTableName(), filter.getQueryParams(), @@ -6408,12 +6424,14 @@ public interface CollectionDAO { String testPlatform = filter.getQueryParam("testPlatform"); String supportedDataType = filter.getQueryParam("supportedDataType"); String supportedService = filter.getQueryParam("supportedService"); + String enabled = filter.getQueryParam("enabled"); String condition = filter.getCondition(); if (entityType == null && testPlatform == null && supportedDataType == null - && supportedService == null) { + && supportedService == null + && enabled == null) { return EntityDAO.super.listCount(filter); } @@ -6452,6 +6470,13 @@ public interface CollectionDAO { + "OR json->>'supportedServices' IS NULL " + "OR json->>'supportedServices' LIKE :supportedServiceLike) "); } + + if (enabled != null) { + String enabledValue = Boolean.parseBoolean(enabled) ? "TRUE" : "FALSE"; + mysqlCondition.append("AND enabled=").append(enabledValue).append(" "); + psqlCondition.append("AND enabled=").append(enabledValue).append(" "); + } + return listCount( getTableName(), filter.getQueryParams(), diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java index 354e8e65c13..053a9cabb31 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java @@ -482,6 +482,14 @@ public class TestCaseRepository extends EntityRepository { Entity.getEntity(test.getTestDefinition(), "", Include.NON_DELETED); test.setTestDefinition(testDefinition.getEntityReference()); + // Validate that the test definition is enabled (only for new test cases, not updates) + if (!update && Boolean.FALSE.equals(testDefinition.getEnabled())) { + throw new IllegalArgumentException( + String.format( + "Test definition '%s' is disabled and cannot be used to create test cases", + testDefinition.getName())); + } + validateTestParameters( test.getParameterValues(), testDefinition.getParameterDefinition(), diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestDefinitionRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestDefinitionRepository.java index 00177d5bfb2..777fae849d2 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestDefinitionRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestDefinitionRepository.java @@ -2,14 +2,19 @@ package org.openmetadata.service.jdbi3; import static org.openmetadata.service.Entity.TEST_DEFINITION; +import jakarta.ws.rs.BadRequestException; +import lombok.extern.slf4j.Slf4j; import org.jdbi.v3.sqlobject.transaction.Transaction; import org.openmetadata.common.utils.CommonUtil; import org.openmetadata.schema.tests.TestDefinition; +import org.openmetadata.schema.type.Include; +import org.openmetadata.schema.type.ProviderType; import org.openmetadata.schema.type.change.ChangeSource; import org.openmetadata.service.Entity; import org.openmetadata.service.resources.dqtests.TestDefinitionResource; import org.openmetadata.service.util.EntityUtil; +@Slf4j public class TestDefinitionRepository extends EntityRepository { public TestDefinitionRepository() { super( @@ -37,6 +42,55 @@ public class TestDefinitionRepository extends EntityRepository { if (CommonUtil.nullOrEmpty(entity.getTestPlatforms())) { throw new IllegalArgumentException("testPlatforms must not be empty"); } + // Set enabled to true by default if not specified + if (entity.getEnabled() == null) { + entity.setEnabled(true); + } + + // For updates to system test definitions, only allow changes to the enabled field + if (update && entity.getProvider() == ProviderType.SYSTEM) { + TestDefinition existing = find(entity.getId(), Include.ALL); + if (existing != null) { + validateSystemTestDefinitionUpdate(existing, entity); + } + } + } + + private void validateSystemTestDefinitionUpdate(TestDefinition existing, TestDefinition updated) { + // Check if any field other than 'enabled' is being changed + if (!existing.getEntityType().equals(updated.getEntityType())) { + throw new BadRequestException( + "System test definitions cannot have their entity type modified"); + } + if (!existing.getTestPlatforms().equals(updated.getTestPlatforms())) { + throw new BadRequestException( + "System test definitions cannot have their test platforms modified"); + } + if (!CommonUtil.nullOrEmpty(existing.getSupportedDataTypes()) + && !existing.getSupportedDataTypes().equals(updated.getSupportedDataTypes())) { + throw new BadRequestException( + "System test definitions cannot have their supported data types modified"); + } + if (!CommonUtil.nullOrEmpty(existing.getParameterDefinition()) + && !existing.getParameterDefinition().equals(updated.getParameterDefinition())) { + throw new BadRequestException( + "System test definitions cannot have their parameter definitions modified"); + } + if (existing.getDataQualityDimension() != null + && !existing.getDataQualityDimension().equals(updated.getDataQualityDimension())) { + throw new BadRequestException( + "System test definitions cannot have their data quality dimension modified"); + } + if (!CommonUtil.nullOrEmpty(existing.getSupportedServices()) + && !existing.getSupportedServices().equals(updated.getSupportedServices())) { + throw new BadRequestException( + "System test definitions cannot have their supported services modified"); + } + if (existing.getSqlExpression() != null + && !existing.getSqlExpression().equals(updated.getSqlExpression())) { + throw new BadRequestException( + "System test definitions cannot have their SQL expression modified"); + } } @Override @@ -49,6 +103,15 @@ public class TestDefinitionRepository extends EntityRepository { // No relationships to store beyond what is stored in the super class } + @Override + protected void preDelete(TestDefinition entity, String deletedBy) { + // Prevent deletion of system test definitions + if (entity.getProvider() == ProviderType.SYSTEM) { + throw new BadRequestException( + "System test definitions cannot be deleted. They can only be disabled by setting enabled=false."); + } + } + @Override public EntityRepository.EntityUpdater getUpdater( TestDefinition original, @@ -67,13 +130,30 @@ public class TestDefinitionRepository extends EntityRepository { @Transaction @Override public void entitySpecificUpdate(boolean consolidatingChanges) { - recordChange("testPlatforms", original.getTestPlatforms(), updated.getTestPlatforms()); - recordChange( - "supportedDataTypes", original.getSupportedDataTypes(), updated.getSupportedDataTypes()); - recordChange( - "parameterDefinition", - original.getParameterDefinition(), - updated.getParameterDefinition()); + // For system test definitions, only allow enabled field changes + if (original.getProvider() == ProviderType.SYSTEM) { + // Only record enabled field changes for system test definitions + recordChange("enabled", original.getEnabled(), updated.getEnabled()); + } else { + // For user/automation test definitions, allow all changes + recordChange("testPlatforms", original.getTestPlatforms(), updated.getTestPlatforms()); + recordChange( + "supportedDataTypes", + original.getSupportedDataTypes(), + updated.getSupportedDataTypes()); + recordChange( + "parameterDefinition", + original.getParameterDefinition(), + updated.getParameterDefinition()); + recordChange("enabled", original.getEnabled(), updated.getEnabled()); + recordChange( + "dataQualityDimension", + original.getDataQualityDimension(), + updated.getDataQualityDimension()); + recordChange( + "supportedServices", original.getSupportedServices(), updated.getSupportedServices()); + recordChange("sqlExpression", original.getSqlExpression(), updated.getSqlExpression()); + } } } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestDefinitionResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestDefinitionResource.java index ebe32616db5..e7d88887231 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestDefinitionResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestDefinitionResource.java @@ -155,7 +155,13 @@ public class TestDefinitionResource "Filter test definitions by supported service. Returns test definitions that either " + "have an empty supportedServices list (supporting all services) or include the specified service.") @QueryParam("supportedService") - String supportedServiceParam) { + String supportedServiceParam, + @Parameter( + description = + "Filter by enabled status (true/false). If not specified, returns all test definitions.", + schema = @Schema(type = "boolean")) + @QueryParam("enabled") + Boolean enabledParam) { ListFilter filter = new ListFilter(include); if (entityType != null) { filter.addQueryParam("entityType", entityType); @@ -169,6 +175,9 @@ public class TestDefinitionResource if (supportedServiceParam != null) { filter.addQueryParam("supportedService", supportedServiceParam); } + if (enabledParam != null) { + filter.addQueryParam("enabled", String.valueOf(enabledParam)); + } return super.listInternal( uriInfo, securityContext, fieldsParam, filter, limitParam, before, after); } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValueMaxToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/columnValueMaxToBeBetween.json index 2332c7900e3..eacb005bd6b 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValueMaxToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValueMaxToBeBetween.json @@ -30,6 +30,7 @@ ], "supportsDynamicAssertion": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Accuracy" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValueMeanToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/columnValueMeanToBeBetween.json index f35498444d4..bc582eabdc6 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValueMeanToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValueMeanToBeBetween.json @@ -30,6 +30,7 @@ ], "supportsDynamicAssertion": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Accuracy" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValueMedianToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/columnValueMedianToBeBetween.json index f439aebcb3f..343a8ab3d88 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValueMedianToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValueMedianToBeBetween.json @@ -30,6 +30,7 @@ ], "supportsDynamicAssertion": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Accuracy" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValueMinToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/columnValueMinToBeBetween.json index e7abf583220..556d4cca4c4 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValueMinToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValueMinToBeBetween.json @@ -30,6 +30,7 @@ ], "supportsDynamicAssertion": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Accuracy" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValueStdDevToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/columnValueStdDevToBeBetween.json index ce2ada6653c..b8dae291d26 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValueStdDevToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValueStdDevToBeBetween.json @@ -30,6 +30,7 @@ ], "supportsDynamicAssertion": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Accuracy" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValueToBeAtExpectedLocation.json b/openmetadata-service/src/main/resources/json/data/tests/columnValueToBeAtExpectedLocation.json index 9f6d884c891..03a1752b177 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValueToBeAtExpectedLocation.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValueToBeAtExpectedLocation.json @@ -39,6 +39,7 @@ ], "supportsRowLevelPassedFailed": false, "provider": "system", + "enabled": true, "dataQualityDimension": "Accuracy" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesLengthsToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesLengthsToBeBetween.json index 1dc8f80ec5a..cdefe2d58c4 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesLengthsToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesLengthsToBeBetween.json @@ -31,5 +31,6 @@ "supportsRowLevelPassedFailed": true, "supportsDynamicAssertion": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Accuracy" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesMissingCountToBeEqual.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesMissingCountToBeEqual.json index 0f39df21a97..373ba15a50b 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesMissingCountToBeEqual.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesMissingCountToBeEqual.json @@ -22,5 +22,6 @@ } ], "provider": "system", + "enabled": true, "dataQualityDimension": "Completeness" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesSumToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesSumToBeBetween.json index 0ffec350725..f7c8cd5d22d 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesSumToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesSumToBeBetween.json @@ -30,6 +30,7 @@ ], "supportsDynamicAssertion": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Accuracy" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeBetween.json index 0aec8128c36..32cbc096630 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeBetween.json @@ -31,5 +31,6 @@ "supportsRowLevelPassedFailed": true, "supportsDynamicAssertion": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Accuracy" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeInSet.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeInSet.json index dc114ff7b5c..c75774cbeea 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeInSet.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeInSet.json @@ -24,5 +24,6 @@ ], "supportsRowLevelPassedFailed": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Validity" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeNotInSet.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeNotInSet.json index 1906f0e8842..36fa69924fb 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeNotInSet.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeNotInSet.json @@ -17,5 +17,6 @@ ], "supportsRowLevelPassedFailed": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Validity" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeNotNull.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeNotNull.json index 72902ee7a97..e7074d34a18 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeNotNull.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeNotNull.json @@ -8,5 +8,6 @@ "supportedDataTypes": ["NUMBER","TINYINT","SMALLINT","INT","BIGINT","BYTEINT","BYTES","FLOAT","DOUBLE","DECIMAL","NUMERIC","TIMESTAMP","TIMESTAMPZ","TIME","DATE","DATETIME","INTERVAL","STRING","MEDIUMTEXT","TEXT","CHAR","VARCHAR","BOOLEAN","BINARY","VARBINARY","ARRAY","BLOB","LONGBLOB","MEDIUMBLOB","MAP","STRUCT","UNION","SET","GEOGRAPHY","ENUM","JSON","UUID","VARIANT","GEOMETRY","POINT","POLYGON","LOWCARDINALITY"], "supportsRowLevelPassedFailed": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Completeness" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeUnique.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeUnique.json index 54342ca35fb..2b525413ba6 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeUnique.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToBeUnique.json @@ -51,5 +51,6 @@ ], "supportsRowLevelPassedFailed": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Uniqueness" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToMatchRegex.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToMatchRegex.json index ae5f4860fb5..54ff8fcc392 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToMatchRegex.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToMatchRegex.json @@ -17,5 +17,6 @@ ], "supportsRowLevelPassedFailed": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Validity" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToNotMatchRegex.json b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToNotMatchRegex.json index ca0ec67c1b3..b9ddfcd31d2 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/columnValuesToNotMatchRegex.json +++ b/openmetadata-service/src/main/resources/json/data/tests/columnValuesToNotMatchRegex.json @@ -17,5 +17,6 @@ ], "supportsRowLevelPassedFailed": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Validity" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/tableColumnCountToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/tableColumnCountToBeBetween.json index e0ebddb1001..ebe8934fd35 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/tableColumnCountToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/tableColumnCountToBeBetween.json @@ -28,5 +28,6 @@ } ], "provider": "system", + "enabled": true, "dataQualityDimension": "Integrity" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/tableColumnCountToEqual.json b/openmetadata-service/src/main/resources/json/data/tests/tableColumnCountToEqual.json index 77f1f563b58..c0910d1a43c 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/tableColumnCountToEqual.json +++ b/openmetadata-service/src/main/resources/json/data/tests/tableColumnCountToEqual.json @@ -15,5 +15,6 @@ } ], "provider": "system", + "enabled": true, "dataQualityDimension": "Integrity" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/tableColumnNameToExist.json b/openmetadata-service/src/main/resources/json/data/tests/tableColumnNameToExist.json index 00b2bbf31c9..378a600f1a4 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/tableColumnNameToExist.json +++ b/openmetadata-service/src/main/resources/json/data/tests/tableColumnNameToExist.json @@ -15,6 +15,7 @@ } ], "provider": "system", + "enabled": true, "dataQualityDimension": "Integrity" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/tableColumnToMatchSet.json b/openmetadata-service/src/main/resources/json/data/tests/tableColumnToMatchSet.json index 494be43bc2b..5b7695b03d8 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/tableColumnToMatchSet.json +++ b/openmetadata-service/src/main/resources/json/data/tests/tableColumnToMatchSet.json @@ -21,6 +21,7 @@ } ], "provider": "system", + "enabled": true, "dataQualityDimension": "Integrity" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/tableCustomSQLQuery.json b/openmetadata-service/src/main/resources/json/data/tests/tableCustomSQLQuery.json index 24bfbfee81f..985f17574b2 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/tableCustomSQLQuery.json +++ b/openmetadata-service/src/main/resources/json/data/tests/tableCustomSQLQuery.json @@ -46,6 +46,7 @@ ], "supportsRowLevelPassedFailed": true, "provider": "system", + "enabled": true, "dataQualityDimension": "SQL" } \ No newline at end of file diff --git a/openmetadata-service/src/main/resources/json/data/tests/tableDiff.json b/openmetadata-service/src/main/resources/json/data/tests/tableDiff.json index 6dc599a34ea..48c933c2f8f 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/tableDiff.json +++ b/openmetadata-service/src/main/resources/json/data/tests/tableDiff.json @@ -59,6 +59,7 @@ } ], "provider": "system", + "enabled": true, "dataQualityDimension": "Consistency", "supportedServices": [ "Snowflake", diff --git a/openmetadata-service/src/main/resources/json/data/tests/tableRowCountToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/tableRowCountToBeBetween.json index 25c06eb85e1..db1f9af15eb 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/tableRowCountToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/tableRowCountToBeBetween.json @@ -29,5 +29,6 @@ ], "supportsDynamicAssertion": true, "provider": "system", + "enabled": true, "dataQualityDimension": "Integrity" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/tableRowCountToEqual.json b/openmetadata-service/src/main/resources/json/data/tests/tableRowCountToEqual.json index 93eef74cb98..40aee0ea421 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/tableRowCountToEqual.json +++ b/openmetadata-service/src/main/resources/json/data/tests/tableRowCountToEqual.json @@ -15,5 +15,6 @@ } ], "provider": "system", + "enabled": true, "dataQualityDimension": "Integrity" } diff --git a/openmetadata-service/src/main/resources/json/data/tests/tableRowInsertedCountToBeBetween.json b/openmetadata-service/src/main/resources/json/data/tests/tableRowInsertedCountToBeBetween.json index 589cc3da2ed..bffaa630961 100644 --- a/openmetadata-service/src/main/resources/json/data/tests/tableRowInsertedCountToBeBetween.json +++ b/openmetadata-service/src/main/resources/json/data/tests/tableRowInsertedCountToBeBetween.json @@ -51,6 +51,7 @@ } ], "provider": "system", + "enabled": true, "dataQualityDimension": "Integrity" } \ No newline at end of file diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java index 296df6465d1..f111fee9929 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java @@ -59,6 +59,7 @@ import static org.openmetadata.service.util.TestUtils.dateToTimestamp; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.flipkart.zjsonpatch.JsonDiff; import es.org.elasticsearch.client.Request; import es.org.elasticsearch.client.Response; import es.org.elasticsearch.client.RestClient; @@ -87,7 +88,6 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.TestMethodOrder; @@ -100,7 +100,6 @@ import org.openmetadata.schema.api.policies.CreatePolicy; import org.openmetadata.schema.api.teams.CreateRole; import org.openmetadata.schema.api.teams.CreateTeam; import org.openmetadata.schema.api.teams.CreateUser; -import org.openmetadata.schema.api.tests.CreateLogicalTestCases; import org.openmetadata.schema.api.tests.CreateTestCase; import org.openmetadata.schema.api.tests.CreateTestCaseResolutionStatus; import org.openmetadata.schema.api.tests.CreateTestCaseResult; @@ -144,8 +143,6 @@ import org.openmetadata.schema.type.TableData; import org.openmetadata.schema.type.TagLabel; import org.openmetadata.schema.type.TaskStatus; import org.openmetadata.schema.type.TestDefinitionEntityType; -import org.openmetadata.schema.type.csv.CsvImportResult; -import org.openmetadata.schema.utils.EntityInterfaceUtil; import org.openmetadata.schema.utils.JsonUtils; import org.openmetadata.schema.utils.ResultList; import org.openmetadata.search.IndexMapping; @@ -163,7 +160,6 @@ import org.openmetadata.service.search.SearchIndexUtils; import org.openmetadata.service.search.SearchRepository; import org.openmetadata.service.search.indexes.TestCaseIndex; import org.openmetadata.service.security.SecurityUtil; -import org.openmetadata.service.util.FullyQualifiedName; import org.openmetadata.service.util.TestUtils; import org.openmetadata.service.util.incidentSeverityClassifier.IncidentSeverityClassifierInterface; import org.testcontainers.shaded.com.google.common.collect.ImmutableMap; @@ -5560,355 +5556,143 @@ public class TestCaseResourceTest extends EntityResourceTest", tableFqn)) - .withTestDefinition(tableRowCountDef.getFullyQualifiedName()) - .withDisplayName("Table Row Count Test") - .withParameterValues( - listOf( - new TestCaseParameterValue().withName("minValue").withValue("10"), - new TestCaseParameterValue().withName("maxValue").withValue("100"))); + ObjectMapper mapper = new ObjectMapper(); + JsonNode patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); - CreateTestCase createColumnTest = - createRequest(getEntityName(test, 2)) - .withEntityLink(String.format("<#E::table::%s::columns::c1>", tableFqn)) - .withTestDefinition(columnUniquenessDef.getFullyQualifiedName()) - .withDisplayName("Column Uniqueness Test") - .withDescription("Column uniqueness test description"); + WebTarget target = + getResource("dataQuality/testDefinitions").path(testDefinition.getId().toString()); + TestUtils.patch(target, patch, TestDefinition.class, ADMIN_AUTH_HEADERS); - TestCase tableTest = createEntity(createTableTest, ADMIN_AUTH_HEADERS); - TestCase columnTest = createEntity(createColumnTest, ADMIN_AUTH_HEADERS); + // Try to create a test case with the disabled definition + CreateTestCase create = createRequest(getEntityName(test)); + create.setEntityLink(TABLE_LINK); + create.setTestDefinition(testDefinition.getFullyQualifiedName()); - // Export test cases for the table - String exportedCsv = exportCsv(tableFqn); - assertNotNull(exportedCsv); - assertTrue(exportedCsv.contains(tableTest.getName())); - assertTrue(exportedCsv.contains(columnTest.getName())); - // CSV quote-escapes values by doubling quotes, so "x" becomes ""x"" - String csvEscapedTableFqn = tableFqn.replace("\"", "\"\""); - String csvEscapedColumnFqn = (tableFqn + ".c1").replace("\"", "\"\""); - assertTrue(exportedCsv.contains(csvEscapedTableFqn)); // Should contain table FQN - assertTrue(exportedCsv.contains(csvEscapedColumnFqn)); // Should contain column FQN + assertResponse( + () -> createEntity(create, ADMIN_AUTH_HEADERS), + BAD_REQUEST, + "Test definition '" + + testDefinition.getName() + + "' is disabled and cannot be used to create test cases"); - // Verify CSV format uses FQN instead of EntityLink - assertFalse(exportedCsv.contains("<#E::table::")); // Should not contain EntityLink format + // Re-enable the test definition for cleanup using JSON Patch + TestDefinition disabled = + Entity.getEntity(Entity.TEST_DEFINITION, testDefinition.getId(), "", null); + originalJson = JsonUtils.pojoToJson(disabled); + disabled.setEnabled(true); + updatedJson = JsonUtils.pojoToJson(disabled); - // Modify the CSV and import it back - String modifiedCsv = - exportedCsv - .replace(tableTest.getDisplayName(), "Modified Table Test") - .replace(columnTest.getDescription(), "Modified column test description"); - - CsvImportResult result = importCsv(tableFqn, modifiedCsv, false); - assertEquals(org.openmetadata.schema.type.ApiStatus.SUCCESS, result.getStatus()); - assertEquals(3, result.getNumberOfRowsPassed() + result.getNumberOfRowsFailed()); - - // Verify the changes were applied - TestCase updatedTableTest = getEntity(tableTest.getId(), ADMIN_AUTH_HEADERS); - assertEquals("Modified Table Test", updatedTableTest.getDisplayName()); - - TestCase updatedColumnTest = getEntity(columnTest.getId(), ADMIN_AUTH_HEADERS); - assertEquals("Modified column test description", updatedColumnTest.getDescription()); + patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + TestUtils.patch(target, patch, TestDefinition.class, ADMIN_AUTH_HEADERS); } @Test - @Tag("ImportExport") - void testImportExport_TestSuiteLevel(TestInfo test) throws IOException { - // Create resource test instances - TestSuiteResourceTest testSuiteResourceTest = new TestSuiteResourceTest(); - TableResourceTest tableResourceTest = new TableResourceTest(); + void test_existingTestCasesContinueToWork(TestInfo test) throws Exception { + // Create a test case with an enabled definition (use table-level test) + CreateTestCase create = createRequest(getEntityName(test)); + create.setEntityLink(TABLE_LINK); + create.setTestDefinition(TEST_DEFINITION5.getFullyQualifiedName()); + create.setParameterValues( + List.of(new TestCaseParameterValue().withName("value").withValue("100"))); + TestCase testCase = createEntity(create, ADMIN_AUTH_HEADERS); + assertNotNull(testCase); - // Create a logical test suite - CreateTestSuite createTestSuite = - testSuiteResourceTest - .createRequest(getEntityName(test)) - .withDescription("Logical test suite for import/export test"); - TestSuite testSuite = testSuiteResourceTest.createEntity(createTestSuite, ADMIN_AUTH_HEADERS); + // Disable the test definition using JSON Patch + String originalJson = JsonUtils.pojoToJson(TEST_DEFINITION5); + TEST_DEFINITION5.setEnabled(false); + String updatedJson = JsonUtils.pojoToJson(TEST_DEFINITION5); - // Create tables and test cases - Column c1 = new Column().withName("c1").withDataType(BIGINT); - CreateTable createTable1 = - tableResourceTest.createRequest(test, 1).withColumns(listOf(c1)).withTableConstraints(null); - Table table1 = tableResourceTest.createEntity(createTable1, ADMIN_AUTH_HEADERS); + ObjectMapper mapper = new ObjectMapper(); + JsonNode patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); - CreateTable createTable2 = - tableResourceTest.createRequest(test, 2).withColumns(listOf(c1)).withTableConstraints(null); - Table table2 = tableResourceTest.createEntity(createTable2, ADMIN_AUTH_HEADERS); + WebTarget target = + getResource("dataQuality/testDefinitions").path(TEST_DEFINITION5.getId().toString()); + TestUtils.patch(target, patch, TestDefinition.class, ADMIN_AUTH_HEADERS); - // Get test definition - TestDefinition tableRowCountDef = - Entity.getEntityByName( - TEST_DEFINITION, "tableRowCountToBeBetween", "", Include.NON_DELETED); + // Existing test case should still be retrievable + TestCase retrieved = getEntity(testCase.getId(), ADMIN_AUTH_HEADERS); + assertNotNull(retrieved); + assertEquals(testCase.getId(), retrieved.getId()); - // Create test cases in the logical test suite - CreateTestCase createTest1 = - createRequest(getEntityName(test, 1)) - .withEntityLink(String.format("<#E::table::%s>", table1.getFullyQualifiedName())) - .withTestDefinition(tableRowCountDef.getFullyQualifiedName()) - .withParameterValues( - listOf( - new TestCaseParameterValue().withName("minValue").withValue("10"), - new TestCaseParameterValue().withName("maxValue").withValue("100"))); + // Should be able to update the existing test case + String oldDescription = testCase.getDescription(); + String updatedDescription = "Updated description for existing test case"; + String json = JsonUtils.pojoToJson(testCase); + testCase.setDescription(updatedDescription); + ChangeDescription change = getChangeDescription(testCase, MINOR_UPDATE); + fieldUpdated(change, "description", oldDescription, updatedDescription); + patchEntityAndCheck(testCase, json, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change); - CreateTestCase createTest2 = - createRequest(getEntityName(test, 2)) - .withEntityLink(String.format("<#E::table::%s>", table2.getFullyQualifiedName())) - .withTestDefinition(tableRowCountDef.getFullyQualifiedName()) - .withParameterValues( - listOf( - new TestCaseParameterValue().withName("minValue").withValue("5"), - new TestCaseParameterValue().withName("maxValue").withValue("50"))); + // Re-enable the test definition for cleanup using JSON Patch + TestDefinition disabled = + Entity.getEntity(Entity.TEST_DEFINITION, TEST_DEFINITION5.getId(), "", null); + originalJson = JsonUtils.pojoToJson(disabled); + disabled.setEnabled(true); + updatedJson = JsonUtils.pojoToJson(disabled); - TestCase test1 = createEntity(createTest1, ADMIN_AUTH_HEADERS); - TestCase test2 = createEntity(createTest2, ADMIN_AUTH_HEADERS); - - // Add test cases to logical test suite using the existing endpoint - addTestCasesToLogicalTestSuiteViaAPI(testSuite.getId(), listOf(test1.getId(), test2.getId())); - - // Export test cases from the test suite - String exportedCsv = exportCsv(testSuite.getFullyQualifiedName()); - assertNotNull(exportedCsv); - assertTrue(exportedCsv.contains(test1.getName())); - assertTrue(exportedCsv.contains(test2.getName())); - - // Import modified CSV - String modifiedCsv = exportedCsv.replace("10", "20").replace("100", "200"); - CsvImportResult result = importCsv(testSuite.getFullyQualifiedName(), modifiedCsv, false); - LOG.info("Import result status: {}", result.getStatus()); - LOG.info("Import result CSV:\n{}", result.getImportResultsCsv()); - assertEquals(org.openmetadata.schema.type.ApiStatus.SUCCESS, result.getStatus()); - - // Verify parameter values were updated - TestCase updatedTest1 = getEntity(test1.getId(), ADMIN_AUTH_HEADERS); - assertEquals( - "20", - updatedTest1.getParameterValues().stream() - .filter(p -> p.getName().equals("minValue")) - .findFirst() - .get() - .getValue()); + patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + TestUtils.patch(target, patch, TestDefinition.class, ADMIN_AUTH_HEADERS); } @Test - @Tag("ImportExport") - void testImportExport_PlatformWide(TestInfo test) throws IOException { - // Create multiple tables and test cases - TableResourceTest tableResourceTest = new TableResourceTest(); - Column c1 = new Column().withName("c1").withDataType(BIGINT); - CreateTable createTable1 = - tableResourceTest.createRequest(test, 1).withColumns(listOf(c1)).withTableConstraints(null); - Table table1 = tableResourceTest.createEntity(createTable1, ADMIN_AUTH_HEADERS); + void test_cannotCreateNewTestCaseWhenDefinitionDisabled(TestInfo test) throws Exception { + // Create a test case with an enabled definition (use table-level test) + CreateTestCase create1 = createRequest(getEntityName(test) + "_1"); + create1.setEntityLink(TABLE_LINK); + create1.setTestDefinition(TEST_DEFINITION4.getFullyQualifiedName()); + create1.setParameterValues( + List.of( + new TestCaseParameterValue().withName("minValue").withValue("10"), + new TestCaseParameterValue().withName("maxValue").withValue("100"))); + TestCase testCase1 = createEntity(create1, ADMIN_AUTH_HEADERS); + assertNotNull(testCase1); - CreateTable createTable2 = - tableResourceTest.createRequest(test, 2).withColumns(listOf(c1)).withTableConstraints(null); - Table table2 = tableResourceTest.createEntity(createTable2, ADMIN_AUTH_HEADERS); + // Disable the test definition using JSON Patch + String originalJson = JsonUtils.pojoToJson(TEST_DEFINITION4); + TEST_DEFINITION4.setEnabled(false); + String updatedJson = JsonUtils.pojoToJson(TEST_DEFINITION4); - // Get test definition - TestDefinition tableRowCountDef = - Entity.getEntityByName( - TEST_DEFINITION, "tableRowCountToBeBetween", "", Include.NON_DELETED); + ObjectMapper mapper = new ObjectMapper(); + JsonNode patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); - // Create test cases - CreateTestCase createTest1 = - createRequest(getEntityName(test, 1)) - .withEntityLink(String.format("<#E::table::%s>", table1.getFullyQualifiedName())) - .withTestDefinition(tableRowCountDef.getFullyQualifiedName()) - .withParameterValues( - listOf( - new TestCaseParameterValue().withName("minValue").withValue("10"), - new TestCaseParameterValue().withName("maxValue").withValue("100"))); + WebTarget target = + getResource("dataQuality/testDefinitions").path(TEST_DEFINITION4.getId().toString()); + TestUtils.patch(target, patch, TestDefinition.class, ADMIN_AUTH_HEADERS); - CreateTestCase createTest2 = - createRequest(getEntityName(test, 2)) - .withEntityLink(String.format("<#E::table::%s>", table2.getFullyQualifiedName())) - .withTestDefinition(tableRowCountDef.getFullyQualifiedName()) - .withParameterValues( - listOf( - new TestCaseParameterValue().withName("minValue").withValue("5"), - new TestCaseParameterValue().withName("maxValue").withValue("50"))); + // Try to create another test case with the disabled definition - should fail + CreateTestCase create2 = createRequest(getEntityName(test) + "_2"); + create2.setEntityLink(TABLE_LINK); + create2.setTestDefinition(TEST_DEFINITION4.getFullyQualifiedName()); + create2.setParameterValues( + List.of( + new TestCaseParameterValue().withName("minValue").withValue("10"), + new TestCaseParameterValue().withName("maxValue").withValue("100"))); - TestCase test1 = createEntity(createTest1, ADMIN_AUTH_HEADERS); - TestCase test2 = createEntity(createTest2, ADMIN_AUTH_HEADERS); + assertResponse( + () -> createEntity(create2, ADMIN_AUTH_HEADERS), + BAD_REQUEST, + "Test definition '" + + TEST_DEFINITION4.getName() + + "' is disabled and cannot be used to create test cases"); - // Export all test cases platform-wide using "*" - String exportedCsv = exportCsv("*"); - assertNotNull(exportedCsv); + // Re-enable the test definition for cleanup using JSON Patch + TestDefinition disabled = + Entity.getEntity(Entity.TEST_DEFINITION, TEST_DEFINITION4.getId(), "", null); + originalJson = JsonUtils.pojoToJson(disabled); + disabled.setEnabled(true); + updatedJson = JsonUtils.pojoToJson(disabled); - // Verify exported CSV contains test cases from multiple tables - assertTrue(exportedCsv.contains(test1.getName())); - assertTrue(exportedCsv.contains(test2.getName())); - - // Count number of test cases in export (excluding header) - long testCaseCount = exportedCsv.lines().count() - 1; - assertTrue(testCaseCount >= 2, "Should export at least 2 test cases"); - } - - @Test - @Tag("ImportExport") - void testImportCsv_CreateAndUpdate(TestInfo test) throws IOException { - TableResourceTest tableResourceTest = new TableResourceTest(); - Column c1 = new Column().withName("c1").withDataType(BIGINT); - CreateTable createTable = - tableResourceTest.createRequest(test).withColumns(listOf(c1)).withTableConstraints(null); - Table table = tableResourceTest.createEntity(createTable, ADMIN_AUTH_HEADERS); - - // Get test definition - TestDefinition tableRowCountDef = - Entity.getEntityByName( - TEST_DEFINITION, "tableRowCountToBeBetween", "", Include.NON_DELETED); - - // Escape quotes in FQN for CSV format (double the quotes and wrap in quotes) - String entityFQN = "\"" + table.getFullyQualifiedName().replace("\"", "\"\"") + "\""; - - // Test 1: Create a new test case via CSV import with tags - // Properly escape parameter values for CSV - double the quotes and wrap in quotes - String paramValues1 = - "\"{\"\"name\"\":\"\"minValue\"\",\"\"value\"\":10};{\"\"name\"\":\"\"maxValue\"\",\"\"value\"\":100}\""; - String createCsv = - "name*,displayName,description,testDefinition*,entityFQN*,testSuite,parameterValues,computePassedFailedRowCount,useDynamicAssertion,inspectionQuery,tags,glossaryTerms\n" - + String.format( - "%s,New Test Case,Initial description,%s,%s,,%s,false,false,,PII.Sensitive,", - getEntityName(test), - tableRowCountDef.getFullyQualifiedName(), - entityFQN, - paramValues1); - - CsvImportResult createResult = importCsv(table.getFullyQualifiedName(), createCsv, false); - LOG.info("Create import status: {}", createResult.getStatus()); - LOG.info("Create import result:\n{}", createResult.getImportResultsCsv()); - assertEquals(org.openmetadata.schema.type.ApiStatus.SUCCESS, createResult.getStatus()); - // Note: NumberOfRowsPassed includes the header row, so 1 data row = 2 total - assertEquals(2, createResult.getNumberOfRowsPassed()); - - // Verify test case was created with tags - String testCaseFqn = - FullyQualifiedName.add( - table.getFullyQualifiedName(), EntityInterfaceUtil.quoteName(getEntityName(test))); - TestCase created = getEntityByName(testCaseFqn, "tags", ADMIN_AUTH_HEADERS); - assertNotNull(created); - assertEquals("New Test Case", created.getDisplayName()); - assertEquals("Initial description", created.getDescription()); - assertEquals(2, created.getParameterValues().size()); - assertNotNull(created.getTags()); - assertEquals(1, created.getTags().size()); - assertEquals("PII.Sensitive", created.getTags().get(0).getTagFQN()); - - // Test 2: Update the existing test case via CSV import with different tags - String paramValues2 = - "\"{\"\"name\"\":\"\"minValue\"\",\"\"value\"\":5};{\"\"name\"\":\"\"maxValue\"\",\"\"value\"\":50}\""; - String updateCsv = - "name*,displayName,description,testDefinition*,entityFQN*,testSuite,parameterValues,computePassedFailedRowCount,useDynamicAssertion,inspectionQuery,tags,glossaryTerms\n" - + String.format( - "%s,Updated Test Case,Updated description,%s,%s,,%s,true,false,SELECT * FROM table,PersonalData.Personal,", - getEntityName(test), - tableRowCountDef.getFullyQualifiedName(), - entityFQN, - paramValues2); - - CsvImportResult updateResult = importCsv(table.getFullyQualifiedName(), updateCsv, false); - LOG.info("Update import status: {}", updateResult.getStatus()); - LOG.info("Update import result:\n{}", updateResult.getImportResultsCsv()); - assertEquals(org.openmetadata.schema.type.ApiStatus.SUCCESS, updateResult.getStatus()); - // Note: NumberOfRowsPassed includes the header row, so 1 data row = 2 total - assertEquals(2, updateResult.getNumberOfRowsPassed()); - - // Verify test case was updated with new tags - TestCase updated = getEntityByName(testCaseFqn, "tags", ADMIN_AUTH_HEADERS); - assertNotNull(updated); - assertEquals("Updated Test Case", updated.getDisplayName()); - assertEquals("Updated description", updated.getDescription()); - assertEquals(2, updated.getParameterValues().size()); - assertEquals("5", updated.getParameterValues().get(0).getValue()); - assertEquals(true, updated.getComputePassedFailedRowCount()); - assertEquals("SELECT * FROM table", updated.getInspectionQuery()); - assertNotNull(updated.getTags()); - assertEquals(1, updated.getTags().size()); - assertEquals("PersonalData.Personal", updated.getTags().get(0).getTagFQN()); - } - - @Test - @Tag("ImportExport") - void testImportCsv_DryRun(TestInfo test) throws IOException { - TableResourceTest tableResourceTest = new TableResourceTest(); - Column c1 = new Column().withName("c1").withDataType(BIGINT); - CreateTable createTable = - tableResourceTest.createRequest(test).withColumns(listOf(c1)).withTableConstraints(null); - Table table = tableResourceTest.createEntity(createTable, ADMIN_AUTH_HEADERS); - - // Get test definition - TestDefinition tableRowCountDef = - Entity.getEntityByName( - TEST_DEFINITION, "tableRowCountToBeBetween", "", Include.NON_DELETED); - - // Escape quotes in FQN for CSV format (double the quotes and wrap in quotes) - String entityFQN = "\"" + table.getFullyQualifiedName().replace("\"", "\"\"") + "\""; - // Headers with * for required fields (name, testDefinition, entityFQN are required) - String csv = - "name*,displayName,description,testDefinition*,entityFQN*,testSuite,parameterValues,computePassedFailedRowCount,useDynamicAssertion,inspectionQuery,tags,glossaryTerms\n" - + String.format( - "%s,Dry run test,Test description,%s,%s,,,false,false,,,", - getEntityName(test), tableRowCountDef.getFullyQualifiedName(), entityFQN); - - // Dry run should not create the entity - CsvImportResult dryRunResult = importCsv(table.getFullyQualifiedName(), csv, true); - if (dryRunResult.getStatus() != org.openmetadata.schema.type.ApiStatus.SUCCESS) { - LOG.info("Dry run import status: {}", dryRunResult.getStatus()); - LOG.info("Dry run abort reason: {}", dryRunResult.getAbortReason()); - LOG.info("Dry run import result:\n{}", dryRunResult.getImportResultsCsv()); - } - assertEquals(org.openmetadata.schema.type.ApiStatus.SUCCESS, dryRunResult.getStatus()); - assertTrue(dryRunResult.getDryRun()); - - // Verify entity was not created - use FQN which includes entityFQN + test case name - String testCaseFqn = - FullyQualifiedName.add( - table.getFullyQualifiedName(), EntityInterfaceUtil.quoteName(getEntityName(test))); - assertThrows( - HttpResponseException.class, () -> getEntityByName(testCaseFqn, ADMIN_AUTH_HEADERS)); - - // Actual import should create the entity - CsvImportResult actualResult = importCsv(table.getFullyQualifiedName(), csv, false); - LOG.info("Actual import status: {}", actualResult.getStatus()); - LOG.info("Actual import result:\n{}", actualResult.getImportResultsCsv()); - assertEquals(org.openmetadata.schema.type.ApiStatus.SUCCESS, actualResult.getStatus()); - assertFalse(actualResult.getDryRun()); - - // Verify entity was created - TestCase created = getEntityByName(testCaseFqn, ADMIN_AUTH_HEADERS); - assertNotNull(created); - assertEquals("Dry run test", created.getDisplayName()); - } - - private void addTestCasesToLogicalTestSuiteViaAPI(UUID testSuiteId, List testCaseIds) - throws HttpResponseException { - CreateLogicalTestCases createLogicalTestCases = - new CreateLogicalTestCases().withTestSuiteId(testSuiteId).withTestCaseIds(testCaseIds); - WebTarget target = getCollection().path("/logicalTestCases"); - org.openmetadata.service.util.TestUtils.put( - target, createLogicalTestCases, TestSuite.class, OK, ADMIN_AUTH_HEADERS); + patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + TestUtils.patch(target, patch, TestDefinition.class, ADMIN_AUTH_HEADERS); } // ======================================== diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestDefinitionResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestDefinitionResourceTest.java index 43e90e2d99c..bf718d21574 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestDefinitionResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestDefinitionResourceTest.java @@ -1,13 +1,20 @@ package org.openmetadata.service.resources.dqtests; import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; +import static jakarta.ws.rs.core.Response.Status.FORBIDDEN; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.openmetadata.service.exception.CatalogExceptionMessage.permissionNotAllowed; import static org.openmetadata.service.util.TestUtils.ADMIN_AUTH_HEADERS; +import static org.openmetadata.service.util.TestUtils.TEST_AUTH_HEADERS; +import static org.openmetadata.service.util.TestUtils.TEST_USER_NAME; import static org.openmetadata.service.util.TestUtils.assertListNotNull; import static org.openmetadata.service.util.TestUtils.assertListNull; import static org.openmetadata.service.util.TestUtils.assertResponse; import static org.openmetadata.service.util.TestUtils.assertResponseContains; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.flipkart.zjsonpatch.JsonDiff; import java.io.IOException; import java.util.List; import java.util.Map; @@ -22,7 +29,9 @@ import org.openmetadata.schema.tests.TestCaseParameter; import org.openmetadata.schema.tests.TestDefinition; import org.openmetadata.schema.tests.TestPlatform; import org.openmetadata.schema.type.ColumnDataType; +import org.openmetadata.schema.type.MetadataOperation; import org.openmetadata.schema.type.TestDefinitionEntityType; +import org.openmetadata.schema.utils.JsonUtils; import org.openmetadata.schema.utils.ResultList; import org.openmetadata.service.Entity; import org.openmetadata.service.resources.EntityResourceTest; @@ -222,4 +231,274 @@ public class TestDefinitionResourceTest public void assertFieldChange(String fieldName, Object expected, Object actual) { assertCommonFieldChange(fieldName, expected, actual); } + + @Test + void test_enableDisableTestDefinition(TestInfo test) throws Exception { + // Create a test definition + TestDefinition testDef = createEntity(createRequest(test), ADMIN_AUTH_HEADERS); + assertEquals(true, testDef.getEnabled(), "Test definition should be enabled by default"); + + // Disable it using JSON Patch + String originalJson = JsonUtils.pojoToJson(testDef); + testDef.setEnabled(false); + String updatedJson = JsonUtils.pojoToJson(testDef); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + + TestDefinition disabled = patchEntity(testDef.getId(), patch, ADMIN_AUTH_HEADERS); + assertEquals(false, disabled.getEnabled(), "Test definition should be disabled"); + + // Verify disabled state persists + TestDefinition retrieved = getEntity(testDef.getId(), ADMIN_AUTH_HEADERS); + assertEquals(false, retrieved.getEnabled(), "Test definition should remain disabled"); + + // Enable it back using JSON Patch + originalJson = JsonUtils.pojoToJson(retrieved); + retrieved.setEnabled(true); + updatedJson = JsonUtils.pojoToJson(retrieved); + + patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + + TestDefinition enabled = patchEntity(retrieved.getId(), patch, ADMIN_AUTH_HEADERS); + assertEquals(true, enabled.getEnabled(), "Test definition should be enabled"); + + // Verify enabled state persists + retrieved = getEntity(testDef.getId(), ADMIN_AUTH_HEADERS); + assertEquals(true, retrieved.getEnabled(), "Test definition should remain enabled"); + } + + @Test + void test_defaultEnabledTrue(TestInfo test) throws HttpResponseException { + // Create test definition without specifying enabled field + TestDefinition testDef = createEntity(createRequest(test), ADMIN_AUTH_HEADERS); + assertEquals(true, testDef.getEnabled(), "Test definition should default to enabled=true"); + + // Verify via GET as well + TestDefinition retrieved = getEntity(testDef.getId(), ADMIN_AUTH_HEADERS); + assertEquals(true, retrieved.getEnabled(), "Retrieved test definition should be enabled"); + } + + @Test + void test_systemTestDefinitionCanBeDisabled(TestInfo test) throws Exception { + // Get a system test definition + TestDefinition systemDef = getEntityByName("columnValuesToBeNotNull", "", ADMIN_AUTH_HEADERS); + assertEquals( + true, systemDef.getEnabled(), "System test definition should be enabled by default"); + + // Disable system test definition using JSON Patch + String originalJson = JsonUtils.pojoToJson(systemDef); + systemDef.setEnabled(false); + String updatedJson = JsonUtils.pojoToJson(systemDef); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + + TestDefinition disabled = patchEntity(systemDef.getId(), patch, ADMIN_AUTH_HEADERS); + assertEquals(false, disabled.getEnabled(), "System test definition should be disableable"); + + // Re-enable for cleanup using JSON Patch + originalJson = JsonUtils.pojoToJson(disabled); + disabled.setEnabled(true); + updatedJson = JsonUtils.pojoToJson(disabled); + + patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + patchEntity(disabled.getId(), patch, ADMIN_AUTH_HEADERS); + } + + @Test + void test_listTestDefinitions_filterByEnabled(TestInfo test) throws Exception { + // Create two test definitions + TestDefinition enabledDef = createEntity(createRequest(test, 1), ADMIN_AUTH_HEADERS); + TestDefinition toDisableDef = createEntity(createRequest(test, 2), ADMIN_AUTH_HEADERS); + + // Disable the second one using JSON Patch + String originalJson = JsonUtils.pojoToJson(toDisableDef); + toDisableDef.setEnabled(false); + String updatedJson = JsonUtils.pojoToJson(toDisableDef); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + + TestDefinition disabledDef = patchEntity(toDisableDef.getId(), patch, ADMIN_AUTH_HEADERS); + assertEquals(false, disabledDef.getEnabled(), "Test definition should be disabled"); + + // Test 1: enabled=true should return only enabled test definitions + Map enabledTrueParams = Map.of("limit", "1000", "enabled", "true"); + ResultList enabledList = listEntities(enabledTrueParams, ADMIN_AUTH_HEADERS); + boolean hasEnabled = + enabledList.getData().stream().anyMatch(td -> td.getId().equals(enabledDef.getId())); + boolean hasDisabled = + enabledList.getData().stream().anyMatch(td -> td.getId().equals(disabledDef.getId())); + Assertions.assertTrue(hasEnabled, "enabled=true list should include enabled test definition"); + Assertions.assertFalse( + hasDisabled, "enabled=true list should NOT include disabled test definition"); + + // Verify all enabled definitions in the enabled list are actually enabled + boolean allEnabled = + enabledList.getData().stream() + .allMatch(td -> td.getEnabled() == null || Boolean.TRUE.equals(td.getEnabled())); + Assertions.assertTrue( + allEnabled, "All test definitions in enabled=true list should be enabled"); + + // Test 2: enabled=false should return only disabled test definitions + Map enabledFalseParams = Map.of("limit", "1000", "enabled", "false"); + ResultList disabledList = listEntities(enabledFalseParams, ADMIN_AUTH_HEADERS); + hasEnabled = + disabledList.getData().stream().anyMatch(td -> td.getId().equals(enabledDef.getId())); + hasDisabled = + disabledList.getData().stream().anyMatch(td -> td.getId().equals(disabledDef.getId())); + Assertions.assertFalse( + hasEnabled, "enabled=false list should NOT include enabled test definition"); + Assertions.assertTrue( + hasDisabled, "enabled=false list should include disabled test definition"); + + // Verify all disabled definitions in the disabled list are actually disabled + boolean allDisabled = + disabledList.getData().stream().allMatch(td -> Boolean.FALSE.equals(td.getEnabled())); + Assertions.assertTrue( + allDisabled, "All test definitions in enabled=false list should be disabled"); + + // Test 3: Default behavior (no enabled param) should return all test definitions + Map defaultParams = Map.of("limit", "1000"); + ResultList defaultList = listEntities(defaultParams, ADMIN_AUTH_HEADERS); + hasEnabled = + defaultList.getData().stream().anyMatch(td -> td.getId().equals(enabledDef.getId())); + hasDisabled = + defaultList.getData().stream().anyMatch(td -> td.getId().equals(disabledDef.getId())); + Assertions.assertTrue(hasEnabled, "Default list should include enabled test definition"); + Assertions.assertTrue(hasDisabled, "Default list should include disabled test definition"); + + // Re-enable for cleanup using JSON Patch + originalJson = JsonUtils.pojoToJson(disabledDef); + disabledDef.setEnabled(true); + updatedJson = JsonUtils.pojoToJson(disabledDef); + + patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + patchEntity(disabledDef.getId(), patch, ADMIN_AUTH_HEADERS); + } + + @Test + void test_createTestDefinition_asNonAdmin_403() { + // Non-admin user without CREATE permission should not be able to create test definition + CreateTestDefinition create = createRequest("unauthorizedTestDef"); + assertResponse( + () -> createEntity(create, TEST_AUTH_HEADERS), + FORBIDDEN, + permissionNotAllowed(TEST_USER_NAME, List.of(MetadataOperation.CREATE))); + } + + @Test + void test_patchTestDefinition_asNonAdmin_403(TestInfo test) throws Exception { + // Create a test definition as admin + TestDefinition testDef = createEntity(createRequest(test), ADMIN_AUTH_HEADERS); + + // Try to patch (enable/disable) as non-admin user + String originalJson = JsonUtils.pojoToJson(testDef); + testDef.setEnabled(false); + String updatedJson = JsonUtils.pojoToJson(testDef); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + + // Non-admin user without EDIT_ALL permission should not be able to patch + assertResponse( + () -> patchEntity(testDef.getId(), patch, TEST_AUTH_HEADERS), + FORBIDDEN, + permissionNotAllowed(TEST_USER_NAME, List.of(MetadataOperation.EDIT_ALL))); + } + + @Test + void test_deleteTestDefinition_asNonAdmin_403(TestInfo test) throws HttpResponseException { + // Create a test definition as admin + TestDefinition testDef = createEntity(createRequest(test), ADMIN_AUTH_HEADERS); + + // Non-admin user without DELETE permission should not be able to delete + assertResponse( + () -> deleteEntity(testDef.getId(), TEST_AUTH_HEADERS), + FORBIDDEN, + permissionNotAllowed(TEST_USER_NAME, List.of(MetadataOperation.DELETE))); + } + + @Test + void test_systemTestDefinitionCannotModifyEntityType(TestInfo test) throws Exception { + // Get a system test definition + TestDefinition systemDef = getEntityByName("columnValuesToBeNotNull", "", ADMIN_AUTH_HEADERS); + assertEquals( + org.openmetadata.schema.type.ProviderType.SYSTEM, + systemDef.getProvider(), + "Should be a system test definition"); + + // Try to modify entity type using JSON Patch - should fail + String originalJson = JsonUtils.pojoToJson(systemDef); + systemDef.setEntityType(TestDefinitionEntityType.TABLE); + String updatedJson = JsonUtils.pojoToJson(systemDef); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + + assertResponse( + () -> patchEntity(systemDef.getId(), patch, ADMIN_AUTH_HEADERS), + BAD_REQUEST, + "System test definitions cannot have their entity type modified"); + } + + @Test + void test_systemTestDefinitionCannotBeDeleted(TestInfo test) throws HttpResponseException { + // Get a system test definition + TestDefinition systemDef = getEntityByName("columnValuesToBeNotNull", "", ADMIN_AUTH_HEADERS); + assertEquals( + org.openmetadata.schema.type.ProviderType.SYSTEM, + systemDef.getProvider(), + "Should be a system test definition"); + + // Try to delete the system test definition - should fail + assertResponse( + () -> deleteEntity(systemDef.getId(), ADMIN_AUTH_HEADERS), + BAD_REQUEST, + "System entity [columnValuesToBeNotNull] of type testDefinition can not be deleted."); + } + + @Test + void test_userTestDefinitionCanBeModified(TestInfo test) throws Exception { + // Create a user test definition + TestDefinition userDef = createEntity(createRequest(test), ADMIN_AUTH_HEADERS); + assertEquals( + org.openmetadata.schema.type.ProviderType.USER, + userDef.getProvider(), + "Should be a user test definition"); + + // Modify entity type using JSON Patch - should succeed + String originalJson = JsonUtils.pojoToJson(userDef); + userDef.setEntityType(TestDefinitionEntityType.TABLE); + String updatedJson = JsonUtils.pojoToJson(userDef); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode patch = JsonDiff.asJson(mapper.readTree(originalJson), mapper.readTree(updatedJson)); + + TestDefinition modified = patchEntity(userDef.getId(), patch, ADMIN_AUTH_HEADERS); + assertEquals( + TestDefinitionEntityType.TABLE, + modified.getEntityType(), + "User test definition entity type should be modifiable"); + } + + @Test + void test_userTestDefinitionCanBeDeleted(TestInfo test) throws HttpResponseException { + // Create a user test definition + TestDefinition userDef = createEntity(createRequest(test), ADMIN_AUTH_HEADERS); + assertEquals( + org.openmetadata.schema.type.ProviderType.USER, + userDef.getProvider(), + "Should be a user test definition"); + + // Delete the user test definition - should succeed + deleteEntity(userDef.getId(), ADMIN_AUTH_HEADERS); + + // Verify it's deleted + assertResponseContains( + () -> getEntity(userDef.getId(), ADMIN_AUTH_HEADERS), + jakarta.ws.rs.core.Response.Status.NOT_FOUND, + "not found"); + } } diff --git a/openmetadata-spec/src/main/resources/json/schema/api/tests/createTestDefinition.json b/openmetadata-spec/src/main/resources/json/schema/api/tests/createTestDefinition.json index ca2f6a5cb15..1b6856f7903 100644 --- a/openmetadata-spec/src/main/resources/json/schema/api/tests/createTestDefinition.json +++ b/openmetadata-spec/src/main/resources/json/schema/api/tests/createTestDefinition.json @@ -47,6 +47,9 @@ "$ref": "../../tests/testDefinition.json#/definitions/testCaseParameterDefinition" } }, + "dataQualityDimension": { + "$ref": "../../tests/testDefinition.json#/definitions/dataQualityDimensions" + }, "supportedServices": { "description": "List of services that this test definition supports. When empty, it implies all services are supported.", "type": "array", @@ -61,6 +64,10 @@ "items": { "type": "string" } + }, + "sqlExpression": { + "description": "SQL expression template for custom SQL-based test definitions. Supports substitution variables: {table} and {column} for runtime entity references, and {{paramName}} for user-defined parameters.", + "$ref": "../../type/basic.json#/definitions/sqlQuery" } }, "required": ["name", "description","entityType", "testPlatforms"], diff --git a/openmetadata-spec/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json b/openmetadata-spec/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json index f5af3e51fd6..a61cafab4fe 100644 --- a/openmetadata-spec/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json +++ b/openmetadata-spec/src/main/resources/json/schema/entity/policies/accessControl/resourceDescriptor.json @@ -65,7 +65,9 @@ "DeleteScim", "ViewScim", "Impersonate", - "AuditLogs" + "AuditLogs", + "ViewTestDefinitionLibrary", + "EditTestDefinitionLibrary" ] } }, diff --git a/openmetadata-spec/src/main/resources/json/schema/tests/testDefinition.json b/openmetadata-spec/src/main/resources/json/schema/tests/testDefinition.json index ca7f459a274..9ebf7531ddc 100644 --- a/openmetadata-spec/src/main/resources/json/schema/tests/testDefinition.json +++ b/openmetadata-spec/src/main/resources/json/schema/tests/testDefinition.json @@ -217,6 +217,11 @@ "type": "boolean", "default": false }, + "enabled": { + "description": "When `true` indicates the test definition is available for creating test cases. System test definitions can only be disabled by users with appropriate permissions.", + "type": "boolean", + "default": true + }, "supportedServices": { "description": "List of services that this test definition supports. When empty, it implies all services are supported.", "type": "array", @@ -224,6 +229,10 @@ "type": "string" }, "default": [] + }, + "sqlExpression": { + "description": "SQL expression template for custom SQL-based test definitions. Supports substitution variables: {table} and {column} for runtime entity references, and {{paramName}} for user-defined parameters. This field is only applicable for test definitions with testPlatforms set to 'OpenMetadata' and is used to execute custom SQL queries for data quality validation.", + "$ref": "../type/basic.json#/definitions/sqlQuery" } }, "required": ["name", "description", "testPlatforms"], diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/RulesLibrary.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/RulesLibrary.spec.ts new file mode 100644 index 00000000000..38816848d4e --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/RulesLibrary.spec.ts @@ -0,0 +1,520 @@ +/* + * Copyright 2024 Collate. + * Licensed 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. + */ +import test, { expect } from '@playwright/test'; +import { redirectToHomePage, uuid } from '../../../utils/common'; + + +const TEST_DEFINITION_NAME = `aCustomTestDefinition${uuid()}`; +const TEST_DEFINITION_DISPLAY_NAME = `Custom Test Definition ${uuid()}`; +const UPDATE_TEST_DEFINITION_DISPLAY_NAME = `Updated Custom Test Definition ${uuid()}`; +const TEST_DEFINITION_DESCRIPTION = + 'A This is a custom test definition for E2E testing'; + +test.use({ storageState: 'playwright/.auth/admin.json' }); + +test.describe('Rules Library', () => { + test.beforeEach(async ({ page }) => { + await redirectToHomePage(page); + }); + + test('should navigate to Rules Library page', async ({ page }) => { + // Navigate directly to Rules Library + await page.goto('/rules-library'); + + // Wait for page to load + await page.waitForSelector('[data-testid="test-definition-table"]', { + state: 'visible', + timeout: 30000, + }); + + // Verify URL + await expect(page).toHaveURL(/.*\/rules-library/); + }); + + test('should display test definitions table with columns', async ({ + page, + }) => { + // Navigate to Rules Library and wait for response + const responsePromise = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'GET' + ); + + await page.goto('/rules-library'); + await responsePromise; + + // Verify table is displayed + await expect(page.getByTestId('test-definition-table')).toBeVisible(); + }); + + test('should display system test definitions', async ({ page }) => { + // Wait for API response before navigation + const responsePromise = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'GET' + ); + + await page.goto('/rules-library'); + await responsePromise; + + // Verify at least one test definition is displayed + const testDefinitionRows = page.locator( + '[data-testid="test-definition-table"] tbody tr' + ); + + await expect(testDefinitionRows).not.toHaveCount(0); + }); + + test('should create, edit, and delete a test definition', async ({ page }) => { + + await test.step('Create a new test definition', async () => { + // Navigate to Rules Library + await page.goto('/rules-library'); + + // Click add button + await page.getByTestId('add-test-definition-button').click(); + + // Wait for drawer to open + await page.waitForSelector('.ant-drawer', { state: 'visible' }); + + // Verify drawer title + await expect(page.locator('.ant-drawer-title')).toContainText( + 'Add Test Definition' + ); + + // Fill in form fields + await page.locator('#name').fill(TEST_DEFINITION_NAME); + await page.locator('#displayName').fill(TEST_DEFINITION_DISPLAY_NAME); + await page.locator('#description').fill(TEST_DEFINITION_DESCRIPTION); + + // Select entity type + await page.locator('#entityType').click(); + await page + .locator('.ant-select-item-option-content:has-text("TABLE")') + .first() + .click(); + + // Select test platform + await page.locator('#testPlatforms').click(); + await page + .locator('.ant-select-item-option-content:has-text("dbt")') + .first() + .click(); + + // Wait for POST response when creating test definition + const testDefinitionResponse = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'POST' + ); + + // Click save + await page.getByTestId('save-test-definition').click(); + + // Wait for API response + const responseData = await testDefinitionResponse; + + expect(responseData.status()).toBe(201); + + // Wait for success toast + await expect(page.getByText(/created successfully/i)).toBeVisible(); + + // Verify test definition appears in table + await expect(page.getByTestId(TEST_DEFINITION_NAME)).toBeVisible(); + }); + + await test.step('Edit Test Definition', async () => { + + // Wait for table to load + await page.waitForSelector('[data-testid="test-definition-table"]', { + state: 'visible', + }); + + // Find and click edit button on first row + const firstEditButton = page.getByTestId(`edit-test-definition-${TEST_DEFINITION_NAME}`).first(); + await firstEditButton.click(); + + // Wait for drawer to open + await page.waitForSelector('.ant-drawer', { state: 'visible' }); + + // Verify drawer title + await expect(page.locator('.ant-drawer-title')).toContainText( + 'Edit Test Definition' + ); + + // Verify name field is disabled in edit mode + const nameInput = page.locator('#name'); + + await expect(nameInput).toBeDisabled(); + + // Update display name + const displayNameInput = page.getByLabel('Display Name'); + await displayNameInput.clear(); + await displayNameInput.fill(UPDATE_TEST_DEFINITION_DISPLAY_NAME); + + // Wait for POST response when creating test definition + const testDefinitionResponse = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'PATCH' + ); + + // Click save + await page.getByTestId('save-test-definition').click(); + // Wait for API response + const responseData = await testDefinitionResponse; + + expect(responseData.status()).toBe(200); + + // Wait for success toast + await expect(page.getByText(/updated successfully/i)).toBeVisible(); + }); + + await test.step('should enable/disable test definition', async () => { + + // Wait for table to load + await page.waitForSelector('[data-testid="test-definition-table"]', { + state: 'visible', + }); + + // Find first enabled switch + const firstSwitch = page.getByTestId(`enable-switch-${TEST_DEFINITION_NAME}`) + + // Wait for API call + const testDefinitionResponse = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'PATCH' + ); + // Toggle the switch + await firstSwitch.click(); + + // Wait for API response + const responseData = await testDefinitionResponse; + + expect(responseData.status()).toBe(200); + + // Wait for success toast + await expect(page.getByText(/updated successfully/i)).toBeVisible(); + + // Verify switch state changed + await expect(firstSwitch).toHaveAttribute( + 'aria-checked', + String("false") + ); + }); + + await test.step('should delete a test definition', async () => { + + // Wait for table to load + await page.waitForSelector('[data-testid="test-definition-table"]', { + state: 'visible', + }); + + // Find and click delete button + const deleteButton = page.getByTestId(`delete-test-definition-${TEST_DEFINITION_NAME}`); + await deleteButton.click(); + + // Wait for confirmation modal + await page.waitForSelector('.ant-modal', { state: 'visible' }); + + // Verify modal content + await expect(page.getByText(`Delete ${UPDATE_TEST_DEFINITION_DISPLAY_NAME}`)).toBeVisible(); + + await page.getByTestId("confirmation-text-input").fill("DELETE"); + + // Wait for API call + const deleteTestDefinitionResponse = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'DELETE' + ); + + // Click confirm delete + await page.getByTestId("confirm-button").click(); + + const response = await deleteTestDefinitionResponse; + expect(response.status()).toBe(200); + + // Wait for success toast + await expect(page.getByText(/deleted successfully/i)).toBeVisible(); + + // Verify test definition is removed from table + await expect(page.getByText(TEST_DEFINITION_NAME)).not.toBeVisible(); + }); + }); + + + + test('should validate required fields in create form', async ({ page }) => { + // Navigate to Rules Library + await page.goto('/rules-library'); + + // Click add button + await page.getByTestId('add-test-definition-button').click(); + + // Wait for drawer to open + await page.waitForSelector('.ant-drawer', { state: 'visible' }); + + // Click save without filling required fields + await page.getByTestId('save-test-definition').click(); + + // Verify validation errors appear for required fields + await expect( + page.locator('.ant-form-item-explain-error').first() + ).toBeVisible(); + }); + + test('should cancel form and close drawer', async ({ page }) => { + // Navigate to Rules Library + await page.goto('/rules-library'); + + // Click add button + await page.getByTestId('add-test-definition-button').click(); + + // Wait for drawer to open + await page.waitForSelector('.ant-drawer', { state: 'visible' }); + + // Fill in some fields + await page.locator('#name').fill('testName'); + + // Click cancel + await page.getByRole('button', { name: /Cancel/i }).click(); + + // Verify drawer is closed + await expect(page.locator('.ant-drawer')).not.toBeVisible(); + }); + + test('should display pagination when test definitions exceed page size', async ({ + page, + }) => { + // Wait for API response promise before navigation + const responsePromise = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'GET' + ); + + await page.goto('/rules-library'); + const response = await responsePromise; + const data = await response.json(); + + // Check if pagination component exists (NextPrevious component) + const hasMultiplePages = data.paging && data.paging.total > 15; + + if (!hasMultiplePages) { + // Skip test if there aren't enough test definitions for pagination + return; + } + + // Pagination uses NextPrevious component, not ant-pagination + await expect( + page + .getByTestId('next') + ).toBeVisible(); + await expect( + page + .getByTestId('previous') + ).toBeVisible(); + }); + + test('should search and filter test definitions', async ({ page }) => { + // Navigate to Rules Library + await page.goto('/rules-library'); + + // Wait for table to load + await page.waitForSelector('[data-testid="test-definition-table"]', { + state: 'visible', + }); + + // Get initial row count + const initialRows = await page + .locator('[data-testid="test-definition-table"] tbody tr') + .count(); + + // Use table search/filter if available + const searchInput = page.getByPlaceholder(/Search/i); + if (await searchInput.isVisible()) { + await searchInput.fill('column'); + + // Wait for filtered results + await page.waitForTimeout(500); + + // Verify filtered results + const filteredRows = await page + .locator('[data-testid="test-definition-table"] tbody tr') + .count(); + + // Filtered results should be less than or equal to initial results + expect(filteredRows).toBeLessThanOrEqual(initialRows); + } + }); + + test('should display test platform badges correctly', async ({ page }) => { + // Navigate to Rules Library + await page.goto('/rules-library'); + + // Wait for table to load + await page.waitForSelector('[data-testid="test-definition-table"]', { + state: 'visible', + }); + + // Verify test platform tags/badges are displayed + const platformTags = page.locator('.ant-tag, .ant-badge'); + const tagCount = await platformTags.count(); + + expect(tagCount).toBeGreaterThan(0); + }); + + test('should not show edit and delete buttons for system test definitions', async ({ + page, + }) => { + // Wait for API response before navigation + const responsePromise = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'GET' + ); + + await page.goto('/rules-library'); + const response = await responsePromise; + const data = await response.json(); + + // Find a system test definition (e.g., columnValueLengthsToBeBetween) + const systemTestDef = data.data.find( + (def: { provider: string; name: string }) => + def.provider === 'system' + ); + + // Verify edit button does not exist for system test definition + const editButton = page.getByTestId( + `edit-test-definition-${systemTestDef.name}` + ); + + await expect(editButton).toBeDisabled(); + + // Verify delete button does not exist for system test definition + const deleteButton = page.getByTestId( + `delete-test-definition-${systemTestDef.name}` + ); + + await expect(deleteButton).toBeDisabled(); + + // Verify enabled switch still exists and is functional + const row = page.locator(`[data-row-key="${systemTestDef.id}"]`); + const enabledSwitch = row.getByRole('switch'); + + await expect(enabledSwitch).toBeVisible(); + + }); + + test('should allow enabling/disabling system test definitions', async ({ + page, + }) => { + // Wait for API response before navigation + const responsePromise = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'GET' + ); + + await page.goto('/rules-library'); + const response = await responsePromise; + const data = await response.json(); + + // Wait for table to load + await page.waitForSelector('[data-testid="test-definition-table"]', { + state: 'visible', + }); + + // Find a system test definition + const systemTestDef = data.data.findLast( + (def: { provider: string }) => def.provider === 'system' + ); + + + const enabledSwitch = page.getByTestId(`enable-switch-${systemTestDef.name}`); + + // Wait for API call and verify success + const patchResponse = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'PATCH' + ); + // Toggle the switch + await enabledSwitch.click(); + + const disableResponse = await patchResponse; + + expect(disableResponse.status()).toBe(200); + + // Verify switch state changed + await expect(enabledSwitch).toHaveAttribute( + 'aria-checked', + String("false") + ); + + const patchResponse2 = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'PATCH' + ); + + // Toggle back to original state + await enabledSwitch.click(); + const enableResponse = await patchResponse2; + + expect(enableResponse.status()).toBe(200); + + }); + + test('should display correct provider type for test definitions', async ({ + page, + }) => { + // Wait for API response before navigation + const responsePromise = page.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') && + response.request().method() === 'GET' + ); + + await page.goto('/rules-library'); + const response = await responsePromise; + const data = await response.json(); + + // Verify we have both system and user test definitions (if user ones exist) + const systemDefs = data.data.filter( + (def: { provider: string }) => def.provider === 'system' + ); + const userDefs = data.data.filter( + (def: { provider: string }) => def.provider === 'user' + ); + + // Should have system test definitions + expect(systemDefs.length).toBeGreaterThan(0); + + // Verify system definitions have provider = 'system' + systemDefs.forEach((def: { provider: string }) => { + expect(def.provider).toBe('system'); + }); + + // If user definitions exist, verify they have provider = 'user' + if (userDefs.length > 0) { + userDefs.forEach((def: { provider: string }) => { + expect(def.provider).toBe('user'); + }); + } + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestDefinitionPermissions.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestDefinitionPermissions.spec.ts new file mode 100644 index 00000000000..9985691ef17 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/TestDefinitionPermissions.spec.ts @@ -0,0 +1,476 @@ +/* + * Copyright 2024 Collate. + * Licensed 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. + */ +import { test as base, expect, Page } from '@playwright/test'; +import { PolicyClass } from '../../../support/access-control/PoliciesClass'; +import { RolesClass } from '../../../support/access-control/RolesClass'; +import { UserClass } from '../../../support/user/UserClass'; +import { performAdminLogin } from '../../../utils/admin'; +import { getApiContext, redirectToHomePage, uuid } from '../../../utils/common'; + + +const actionNotAllowed = async (page: Page) => { + // Verify "Add Test Definition" button is NOT visible (no Create permission) + const addButton = page.getByTestId('add-test-definition-button'); + + await expect(addButton).not.toBeVisible(); + + // Verify edit buttons are NOT visible for user test definitions + const editButtons = page.getByTestId(/edit-test-definition-/).first(); + await expect(editButtons).toBeDisabled(); + + // Verify delete buttons are NOT visible + const deleteButtons = page.getByTestId(/delete-test-definition-/).first(); + await expect(deleteButtons).toBeDisabled(); + + // Verify enabled/disabled switches are NOT interactive (no EditAll permission) + const firstSwitch = page.getByRole('switch').first(); + await expect(firstSwitch).toBeDisabled(); +} + +// Define permission policies for different roles +const TEST_DEFINITION_VIEW_ONLY_RULES = [ + { + name: `test-definition-view-only-${uuid()}`, + resources: ['testDefinition'], + operations: ['ViewAll', 'ViewBasic'], + effect: 'allow', + }, +]; + +const TEST_DEFINITION_DATA_CONSUMER_RULES = [ + { + name: `test-definition-data-consumer-${uuid()}`, + resources: ['testDefinition'], + operations: ['ViewAll', 'ViewBasic'], + effect: 'allow', + }, +]; + +const TEST_DEFINITION_DATA_STEWARD_RULES = [ + { + name: `test-definition-data-steward-${uuid()}`, + resources: ['testDefinition'], + operations: ['ViewAll', 'ViewBasic', 'EditAll'], + effect: 'allow', + }, +]; + +// Create policy and role instances +const dataConsumerPolicy = new PolicyClass(); +const dataConsumerRole = new RolesClass(); +const dataConsumerUser = new UserClass(); + +const dataStewardPolicy = new PolicyClass(); +const dataStewardRole = new RolesClass(); +const dataStewardUser = new UserClass(); + +const viewOnlyPolicy = new PolicyClass(); +const viewOnlyRole = new RolesClass(); +const viewOnlyUser = new UserClass(); + +const test = base.extend<{ + adminPage: Page; + dataConsumerPage: Page; + dataStewardPage: Page; + viewOnlyPage: Page; +}>({ + adminPage: async ({ browser }, use) => { + const { page } = await performAdminLogin(browser); + await use(page); + await page.close(); + }, + dataConsumerPage: async ({ browser }, use) => { + const page = await browser.newPage(); + await dataConsumerUser.login(page); + await use(page); + await page.close(); + }, + dataStewardPage: async ({ browser }, use) => { + const page = await browser.newPage(); + await dataStewardUser.login(page); + await use(page); + await page.close(); + }, + viewOnlyPage: async ({ browser }, use) => { + const page = await browser.newPage(); + await viewOnlyUser.login(page); + await use(page); + await page.close(); + }, +}); + +test.beforeAll(async ({ browser }) => { + const { apiContext, afterAction } = await performAdminLogin(browser); + + // Create view-only user with policy and role + await viewOnlyUser.create(apiContext, false); + const viewOnlyPolicyResponse = await viewOnlyPolicy.create( + apiContext, + TEST_DEFINITION_VIEW_ONLY_RULES + ); + const viewOnlyRoleResponse = await viewOnlyRole.create(apiContext, [ + viewOnlyPolicyResponse.fullyQualifiedName, + ]); + await viewOnlyUser.patch({ + apiContext, + patchData: [ + { + op: 'add', + path: '/roles/0', + value: { + id: viewOnlyRoleResponse.id, + type: 'role', + name: viewOnlyRoleResponse.name, + }, + }, + ], + }); + + // Create data consumer user with policy and role + await dataConsumerUser.create(apiContext, false); + const dataConsumerPolicyResponse = await dataConsumerPolicy.create( + apiContext, + TEST_DEFINITION_DATA_CONSUMER_RULES + ); + const dataConsumerRoleResponse = await dataConsumerRole.create(apiContext, [ + dataConsumerPolicyResponse.fullyQualifiedName, + ]); + await dataConsumerUser.patch({ + apiContext, + patchData: [ + { + op: 'add', + path: '/roles/0', + value: { + id: dataConsumerRoleResponse.id, + type: 'role', + name: dataConsumerRoleResponse.name, + }, + }, + ], + }); + + // Create data steward user with policy and role + await dataStewardUser.create(apiContext, false); + const dataStewardPolicyResponse = await dataStewardPolicy.create( + apiContext, + TEST_DEFINITION_DATA_STEWARD_RULES + ); + const dataStewardRoleResponse = await dataStewardRole.create(apiContext, [ + dataStewardPolicyResponse.fullyQualifiedName, + ]); + await dataStewardUser.patch({ + apiContext, + patchData: [ + { + op: 'add', + path: '/roles/0', + value: { + id: dataStewardRoleResponse.id, + type: 'role', + name: dataStewardRoleResponse.name, + }, + }, + ], + }); + + await afterAction(); +}); + +test.afterAll(async ({ browser }) => { + const { apiContext, afterAction } = await performAdminLogin(browser); + + // Cleanup users, roles, and policies + await viewOnlyUser.delete(apiContext); + await viewOnlyRole.delete(apiContext); + await viewOnlyPolicy.delete(apiContext); + + await dataConsumerUser.delete(apiContext); + await dataConsumerRole.delete(apiContext); + await dataConsumerPolicy.delete(apiContext); + + await dataStewardUser.delete(apiContext); + await dataStewardRole.delete(apiContext); + await dataStewardPolicy.delete(apiContext); + + await afterAction(); +}); + +test.describe('Test Definition Permissions - View Only User', () => { + test('should allow viewing test definitions but not create, edit, or delete', async ({ + viewOnlyPage, + }) => { + await redirectToHomePage(viewOnlyPage); + + // Navigate to Rules Library + await viewOnlyPage.goto('/rules-library'); + + // Wait for table to load + await viewOnlyPage.waitForSelector( + '[data-testid="test-definition-table"]', + { + state: 'visible', + } + ); + + // Verify user can view the table + await expect( + viewOnlyPage.getByTestId('test-definition-table') + ).toBeVisible(); + + await actionNotAllowed(viewOnlyPage); + }); +}); + +test.describe('Test Definition Permissions - Data Consumer', () => { + test('should allow viewing test definitions but not create, edit, or delete', async ({ + dataConsumerPage, + }) => { + await redirectToHomePage(dataConsumerPage); + + // Navigate to Rules Library + await dataConsumerPage.goto('/rules-library'); + + // Wait for table to load + await dataConsumerPage.waitForSelector( + '[data-testid="test-definition-table"]', + { + state: 'visible', + } + ); + + // Verify user can view the table + await expect( + dataConsumerPage.getByTestId('test-definition-table') + ).toBeVisible(); + + await actionNotAllowed(dataConsumerPage); + }); +}); + +test.describe('Test Definition Permissions - Data Steward', () => { + test('should allow viewing and editing but not creating or deleting test definitions', async ({ + dataStewardPage, + }) => { + await redirectToHomePage(dataStewardPage); + + // Navigate to Rules Library + await dataStewardPage.goto('/rules-library'); + + // Wait for table to load + await dataStewardPage.waitForSelector( + '[data-testid="test-definition-table"]', + { + state: 'visible', + } + ); + + // Verify user can view the table + await expect( + dataStewardPage.getByTestId('test-definition-table') + ).toBeVisible(); + + // Data Steward should NOT have Create permission + const addButton = dataStewardPage.getByTestId('add-test-definition-button'); + + await expect(addButton).not.toBeVisible(); + + // Data Steward should be able to toggle enabled/disabled switches (EditAll permission) + const firstSwitch = dataStewardPage.getByRole('switch').first(); + + await expect(firstSwitch).toBeEnabled(); + + // Wait for API call + const response = dataStewardPage.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions/') && + response.request().method() === 'PATCH' + ); + + // Try to toggle the switch + await firstSwitch.click(); + await response; + + // Verify switch state changed + await expect(firstSwitch).toHaveAttribute( + 'aria-checked', + String("false") + ); + + const response2 = dataStewardPage.waitForResponse( + (response) => + response.url().includes('/api/v1/dataQuality/testDefinitions/') && + response.request().method() === 'PATCH' + ); + // Toggle back to original state + await firstSwitch.click(); + await response2; + + await expect(firstSwitch).toHaveAttribute('aria-checked', String("true")); + + + // Data Steward should NOT see delete buttons (no Delete permission) + const deleteButtons = dataStewardPage.getByTestId( + /delete-test-definition-/ + ); + await expect(deleteButtons.first()).toBeDisabled(); + }); + + test('should not be able to edit system test definitions', async ({ + dataStewardPage, + }) => { + await redirectToHomePage(dataStewardPage); + // Wait for API response to get test definitions + const response = dataStewardPage.waitForResponse((response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') + ); + + // Navigate to Rules Library + await dataStewardPage.goto('/rules-library'); + + const responseResolved = await response; + const data = await responseResolved.json(); + + // Find a system test definition + const systemTestDef = data.data.find( + (def: { provider: string }) => def.provider === 'system' + ); + + + // Verify edit button does not exist for system test definition + const editButton = dataStewardPage.getByTestId( + `edit-test-definition-${systemTestDef.name}` + ); + + await expect(editButton).toBeDisabled(); + + // Verify enabled switch exists and can be toggled + const row = dataStewardPage.locator( + `[data-row-key="${systemTestDef.id}"]` + ); + const enabledSwitch = row.getByRole('switch'); + + await expect(enabledSwitch).toBeVisible(); + await expect(enabledSwitch).toBeEnabled(); + + }); +}); + +test.describe('Test Definition Permissions - API Level Validation', () => { + test('should prevent unauthorized users from creating test definitions via API', async ({ + dataConsumerPage, + }) => { + await redirectToHomePage(dataConsumerPage); + const { apiContext } = await getApiContext(dataConsumerPage); + + // Try to create a test definition via API (should fail) + const createResponse = await apiContext.post( + '/api/v1/dataQuality/testDefinitions', + { + data: { + name: 'unauthorizedTest', + description: 'Should not be created', + entityType: 'COLUMN', + testPlatforms: ['OpenMetadata'], + }, + } + ); + + // Verify the request failed with 403 Forbidden + expect(createResponse.status()).toBe(403); + }); + + test('should prevent unauthorized users from deleting test definitions via API', async ({ + dataStewardPage, + adminPage + }) => { + await redirectToHomePage(dataStewardPage); + await redirectToHomePage(adminPage); + const { apiContext } = await getApiContext(dataStewardPage); + const { apiContext: adminApiContext } = await getApiContext(adminPage); + + const createResponse = await adminApiContext.post( + '/api/v1/dataQuality/testDefinitions', + { + data: { + name: `unauthorizedTest${uuid()}`, + description: `unauthorizedTest`, + entityType: 'COLUMN', + testPlatforms: ['OpenMetadata'], + }, + } + ); + + const data = await createResponse.json(); + + // Try to delete the test definition (should fail - no Delete permission) + const deleteResponse = await apiContext.delete( + `/api/v1/dataQuality/testDefinitions/${data.id}` + ); + + // Verify the request failed with 403 Forbidden + expect(deleteResponse.status()).toBe(403); + + }); + + test('should prevent all users from modifying system test definition entity type via API', async ({ + adminPage, + }) => { + await redirectToHomePage(adminPage); + const { apiContext } = await getApiContext(adminPage); + + const response = adminPage.waitForResponse((response) => + response.url().includes('/api/v1/dataQuality/testDefinitions') + ); + + // Navigate to Rules Library + await adminPage.goto('/rules-library'); + + const responseResolved = await response; + const data = await responseResolved.json(); + + // Find a system test definition with COLUMN entity type + const systemTestDef = data.data.find( + (def: { provider: string; entityType: string }) => + def.provider === 'system' && def.entityType === 'COLUMN' + ); + + + // Try to patch the test definition to change entity type (should fail even for admin) + const patchResponse = await apiContext.patch( + `/api/v1/dataQuality/testDefinitions/${systemTestDef.id}`, + { + data: [ + { + op: 'replace', + path: '/entityType', + value: 'TABLE', + }, + ], + headers: { + 'Content-Type': 'application/json-patch+json', + }, + } + ); + + // Verify the request failed with 400 Bad Request + expect(patchResponse.status()).toBe(400); + + const errorBody = await patchResponse.json(); + + expect(errorBody.message).toContain( + 'System test definitions cannot have their entity type modified' + ); + + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AuthenticatedAppRouter.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AuthenticatedAppRouter.tsx index ab62c598768..53841c648d0 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AuthenticatedAppRouter.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AuthenticatedAppRouter.tsx @@ -242,6 +242,10 @@ const IncidentManagerPage = withSuspenseFallback( React.lazy(() => import('../../pages/IncidentManager/IncidentManagerPage')) ); +const RulesLibraryPage = withSuspenseFallback( + React.lazy(() => import('../../pages/RulesLibrary/RulesLibraryPage')) +); + const IncidentManagerDetailPage = withSuspenseFallback( React.lazy( () => @@ -586,6 +590,18 @@ const AuthenticatedAppRouter: FunctionComponent = () => { } path={ROUTES.INCIDENT_MANAGER} /> + + + + } + path={ROUTES.RULES_LIBRARY} + /> {[ ROUTES.TEST_CASE_DETAILS, diff --git a/openmetadata-ui/src/main/resources/ui/src/components/RulesLibrary/TestDefinitionForm/TestDefinitionForm.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/RulesLibrary/TestDefinitionForm/TestDefinitionForm.component.tsx new file mode 100644 index 00000000000..93cea44709b --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/components/RulesLibrary/TestDefinitionForm/TestDefinitionForm.component.tsx @@ -0,0 +1,437 @@ +/* + * Copyright 2024 Collate. + * Licensed 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. + */ + +import { + MinusCircleOutlined, + PlusOutlined, + QuestionCircleOutlined, +} from '@ant-design/icons'; +import { + Button, + Card, + Drawer, + Form, + Input, + Select, + Space, + Switch, + Tooltip, + Typography, +} from 'antd'; +import { AxiosError } from 'axios'; +import { compare } from 'fast-json-patch'; +import React, { useMemo, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { ReactComponent as CloseIcon } from '../../../assets/svg/close.svg'; +import { CSMode } from '../../../enums/codemirror.enum'; +import { CreateTestDefinition } from '../../../generated/api/tests/createTestDefinition'; +import { + DataQualityDimensions, + DataType, + EntityType, + TestDataType, + TestDefinition, + TestPlatform, +} from '../../../generated/tests/testDefinition'; +import { + createTestDefinition, + patchTestDefinition, +} from '../../../rest/testAPI'; +import { createScrollToErrorHandler } from '../../../utils/formUtils'; +import { showSuccessToast } from '../../../utils/ToastUtils'; +import AlertBar from '../../AlertBar/AlertBar'; +import FormItemLabel from '../../common/Form/FormItemLabel'; +import CodeEditor from '../../Database/SchemaEditor/CodeEditor'; + +interface TestDefinitionFormProps { + initialValues?: TestDefinition; + onSuccess: () => void; + onCancel: () => void; +} + +const TestDefinitionForm: React.FC = ({ + initialValues, + onSuccess, + onCancel, +}) => { + const { t } = useTranslation(); + const [form] = Form.useForm(); + const [isSubmitting, setIsSubmitting] = useState(false); + const [errorMessage, setErrorMessage] = useState(''); + const isEditMode = Boolean(initialValues); + const scrollToError = useMemo(() => createScrollToErrorHandler(), []); + + const handleSubmit = async (values: TestDefinition) => { + setIsSubmitting(true); + setErrorMessage(''); + try { + if (isEditMode && initialValues) { + const updatedValues = { + ...initialValues, + ...values, + }; + const patch = compare(initialValues, updatedValues); + if (patch.length > 0) { + await patchTestDefinition(initialValues?.id ?? '', patch); + } + showSuccessToast( + t('server.entity-updated-success', { + entity: t('label.test-definition'), + }) + ); + } else { + const payload: CreateTestDefinition = { + name: values.name, + displayName: values.displayName, + description: values.description, + sqlExpression: values.sqlExpression, + entityType: values.entityType ?? EntityType.Table, + testPlatforms: values.testPlatforms, + dataQualityDimension: values.dataQualityDimension, + supportedDataTypes: values.supportedDataTypes, + parameterDefinition: values.parameterDefinition, + }; + await createTestDefinition(payload); + showSuccessToast( + t('server.entity-created-success', { + entity: t('label.test-definition'), + }) + ); + } + + onSuccess(); + } catch (error) { + const errorMsg = + (error as AxiosError<{ message: string }>)?.response?.data?.message || + (isEditMode + ? t('server.update-entity-error', { + entity: t('label.test-definition'), + }) + : t('server.create-entity-error', { + entity: t('label.test-definition'), + })); + setErrorMessage(errorMsg); + } finally { + setIsSubmitting(false); + } + }; + + return ( + } + type="link" + onClick={onCancel} + /> + } + footer={ + + + + + } + title={ + isEditMode + ? t('label.edit-entity', { entity: t('label.test-definition') }) + : t('label.add-entity', { entity: t('label.test-definition') }) + } + width={720} + onClose={onCancel}> + {errorMessage && ( +
+ +
+ )} +
+ + + + + + + + + + + + + + + + + } + /> + + + + ({ + label: platform, + value: platform, + }))} + placeholder={t('label.select-field', { + field: t('label.test-platform-plural'), + })} + /> + + + + ({ + label: dataType, + value: dataType, + }))} + placeholder={t('label.select-field', { + field: t('label.supported-data-type-plural'), + })} + /> + + + + }> + + {(fields, { add, remove }) => ( + <> + {fields.map(({ key, name, ...restField }) => ( + remove(name)} />} + key={key} + size="small" + style={{ marginTop: 16 }} + title={`${t('label.parameter')} ${name + 1}`}> + + + + + + + + + + + + + +