Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

GEN 1931 - Fix entity link accepted chars #18391

Merged
merged 16 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
protected final boolean supportsEmptyDescription;

// Special characters supported in the entity name
protected String supportedNameCharacters = "_'-.&()" + RANDOM_STRING_GENERATOR.generate(1);
protected String supportedNameCharacters = "_'-.&()[]" + RANDOM_STRING_GENERATOR.generate(1);

protected final boolean supportsCustomExtension;

Expand All @@ -252,7 +252,7 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
public static final String DATA_CONSUMER_ROLE_NAME = "DataConsumer";

public static final String ENTITY_LINK_MATCH_ERROR =
"[entityLink must match \"(?U)^<#E::\\w+::[\\w'\\- .&/:+\"\\\\()$#%]+>$\"]";
"[entityLink must match \"(?U)^<#E::\\w+::(?:[^:<>|]|:[^:<>|])+(?:::(?:[^:<>|]|:[^:<>|])+)*>$\"]";

// Random unicode string generator to test entity name accepts all the unicode characters
protected static final RandomStringGenerator RANDOM_STRING_GENERATOR =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.openmetadata.common.utils.CommonUtil.listOf;
import static org.openmetadata.schema.type.ColumnDataType.BIGINT;
Expand Down Expand Up @@ -3188,6 +3189,52 @@ void test_listTestCaseFromSearch(TestInfo testInfo) throws HttpResponseException
});
}

@Test
void test_testCaseInvalidEntityLinkTest(TestInfo testInfo) throws IOException {
// Invalid entity link as not parsable by antlr parser
String entityLink = "<#E::table::special!@#$%^&*()_+[]{}|;:\\'\",./?>";
CreateTestCase create = createRequest(testInfo);
create
.withEntityLink(entityLink)
.withTestSuite(TEST_SUITE1.getFullyQualifiedName())
.withTestDefinition(TEST_DEFINITION3.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("100").withName("missingCountValue")));

assertThrows(
HttpResponseException.class,
() -> createAndCheckEntity(create, ADMIN_AUTH_HEADERS),
"entityLink must match \"(?U)^<#E::\\w+::[\\w'\\- .&/:+\"\\\\()$#%]+>$\"");

entityLink = "<#E::table::user<name>::column>";
create.setEntityLink(entityLink);
assertThrows(
HttpResponseException.class,
() -> createAndCheckEntity(create, ADMIN_AUTH_HEADERS),
"entityLink must match \"(?U)^<#E::\\w+::[\\w'\\- .&/:+\"\\\\()$#%]+>$\"");

entityLink = "<#E::table::user>name::column>";
create.setEntityLink(entityLink);
assertThrows(
HttpResponseException.class,
() -> createAndCheckEntity(create, ADMIN_AUTH_HEADERS),
"entityLink must match \"(?U)^<#E::\\w+::[\\w'\\- .&/:+\"\\\\()$#%]+>$\"");

entityLink = "<#E::table::foo<>bar::baz>\");";
create.setEntityLink(entityLink);
assertThrows(
HttpResponseException.class,
() -> createAndCheckEntity(create, ADMIN_AUTH_HEADERS),
"entityLink must match \"(?U)^<#E::\\w+::[\\w'\\- .&/:+\"\\\\()$#%]+>$\"");

entityLink = "<#E::table::::baz>";
create.setEntityLink(entityLink);
assertThrows(
HttpResponseException.class,
() -> createAndCheckEntity(create, ADMIN_AUTH_HEADERS),
"entityLink must match \"(?U)^<#E::\\w+::[\\w'\\- .&/:+\"\\\\()$#%]+>$\"");
}

private void putInspectionQuery(TestCase testCase, String sql) throws IOException {
TestCase putResponse = putInspectionQuery(testCase.getId(), sql, ADMIN_AUTH_HEADERS);
assertEquals(sql, putResponse.getInspectionQuery());
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ void post_feedWithInvalidAbout_4xx() {
// Create thread without addressed to entity in the request
CreateThread create = create().withFrom(USER.getName()).withAbout("<>"); // Invalid EntityLink

String failureReason = "[about must match \"(?U)^<#E::\\w+::[\\w'\\- .&/:+\"\\\\()$#%]+>$\"]";
String failureReason =
"[about must match \"(?U)^<#E::\\w+::(?:[^:<>|]|:[^:<>|])+(?:::(?:[^:<>|]|:[^:<>|])+)*>$\"]";
assertResponseContains(
() -> createThread(create, USER_AUTH_HEADERS), BAD_REQUEST, failureReason);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ void post_suggestionWithInvalidAbout_4xx() {
CreateSuggestion create = create().withEntityLink("<>"); // Invalid EntityLink

String failureReason =
"[entityLink must match \"(?U)^<#E::\\w+::[\\w'\\- .&/:+\"\\\\()$#%]+>$\"]";
"[entityLink must match \"(?U)^<#E::\\w+::(?:[^:<>|]|:[^:<>|])+(?:::(?:[^:<>|]|:[^:<>|])+)*>$\"]";
assertResponseContains(
() -> createSuggestion(create, USER_AUTH_HEADERS), BAD_REQUEST, failureReason);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

import static org.junit.jupiter.api.Assertions.*;

import java.util.HashMap;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.openmetadata.schema.entity.data.GlossaryTerm;
import org.openmetadata.schema.entity.data.Table;
import org.openmetadata.service.resources.feeds.MessageParser;

class EntityUtilTest {
@Test
Expand All @@ -15,4 +18,167 @@ void test_isDescriptionRequired() {
EntityUtil.isDescriptionRequired(
GlossaryTerm.class)); // GlossaryTerm entity requires description
}

@Test
void test_entityLinkParser() {

// Valid entity links
Map<String, String> expected = new HashMap<>();
expected.put("entityLink", "<#E::table::users>");
expected.put("arrayFieldName", null);
expected.put("arrayFieldValue", null);
expected.put("fieldName", null);
expected.put("entityFQN", "users");
expected.put("entityType", "table");
expected.put("linkType", "ENTITY");
expected.put("fullyQualifiedFieldType", "table");
expected.put("fullyQualifiedFieldValue", "users");
verifyEntityLinkParser(expected);

expected.clear();
expected.put("entityLink", "<#E::table::users.foo.\"bar.baz\">");
expected.put("arrayFieldName", null);
expected.put("arrayFieldValue", null);
expected.put("fieldName", null);
expected.put("entityFQN", "users.foo.\"bar.baz\"");
expected.put("entityType", "table");
expected.put("linkType", "ENTITY");
expected.put("fullyQualifiedFieldType", "table");
expected.put("fullyQualifiedFieldValue", "users.foo.\"bar.baz\"");
verifyEntityLinkParser(expected);

expected.clear();
expected.put("entityLink", "<#E::db::customers>");
expected.put("arrayFieldName", null);
expected.put("arrayFieldValue", null);
expected.put("fieldName", null);
expected.put("entityFQN", "customers");
expected.put("entityType", "db");
expected.put("linkType", "ENTITY");
expected.put("fullyQualifiedFieldType", "db");
expected.put("fullyQualifiedFieldValue", "customers");
verifyEntityLinkParser(expected);

expected.clear();
expected.put("entityLink", "<#E::table::users::column::id>");
expected.put("arrayFieldName", "id");
expected.put("arrayFieldValue", null);
expected.put("fieldName", "column");
expected.put("entityFQN", "users");
expected.put("entityType", "table");
expected.put("linkType", "ENTITY_ARRAY_FIELD");
expected.put("fullyQualifiedFieldType", "table.column.member");
expected.put("fullyQualifiedFieldValue", "users.id");
verifyEntityLinkParser(expected);

expected.clear();
expected.put("entityLink", "<#E::table::orders::column::status::type::enum>");
expected.put("arrayFieldName", "status");
expected.put("arrayFieldValue", "type::enum");
expected.put("fieldName", "column");
expected.put("entityFQN", "orders");
expected.put("entityType", "table");
expected.put("linkType", "ENTITY_ARRAY_FIELD");
expected.put("fullyQualifiedFieldType", "table.column.member");
expected.put("fullyQualifiedFieldValue", "orders.status.type::enum");
verifyEntityLinkParser(expected);

expected.clear();
expected.put("entityLink", "<#E::db::schema::table::view::column>");
expected.put("arrayFieldName", "view");
expected.put("arrayFieldValue", "column");
expected.put("fieldName", "table");
expected.put("entityFQN", "schema");
expected.put("entityType", "db");
expected.put("linkType", "ENTITY_ARRAY_FIELD");
expected.put("fullyQualifiedFieldType", "db.table.member");
expected.put("fullyQualifiedFieldValue", "schema.view.column");
verifyEntityLinkParser(expected);

expected.clear();
expected.put("entityLink", "<#E::table::foo@bar>");
expected.put("arrayFieldName", null);
expected.put("arrayFieldValue", null);
expected.put("fieldName", null);
expected.put("entityFQN", "foo@bar");
expected.put("entityType", "table");
expected.put("linkType", "ENTITY");
expected.put("fullyQualifiedFieldType", "table");
expected.put("fullyQualifiedFieldValue", "foo@bar");
verifyEntityLinkParser(expected);

expected.clear();
expected.put("entityLink", "<#E::table::foo[bar]>");
expected.put("arrayFieldName", null);
expected.put("arrayFieldValue", null);
expected.put("fieldName", null);
expected.put("entityFQN", "foo[bar]");
expected.put("entityType", "table");
expected.put("linkType", "ENTITY");
expected.put("fullyQualifiedFieldType", "table");
expected.put("fullyQualifiedFieldValue", "foo[bar]");
verifyEntityLinkParser(expected);

expected.clear();
expected.put("entityLink", "<#E::table::special!@#$%^&*()_+[]{};:\\'\",./?>");
expected.put("arrayFieldName", null);
expected.put("arrayFieldValue", null);
expected.put("fieldName", null);
expected.put("entityFQN", "special!@#$%^&*()_+[]{};:\\'\",./?");
expected.put("entityType", "table");
expected.put("linkType", "ENTITY");
expected.put("fullyQualifiedFieldType", "table");
expected.put("fullyQualifiedFieldValue", "special!@#$%^&*()_+[]{};:\\'\",./?");
verifyEntityLinkParser(expected);

// Invalid entity link
expected.clear();
expected.put("entityLink", "<#E::table::special!@#$%^&*()_+[]{}|;:\\'\",./?>");
// EntityLink with `|` character will not be parsed correctly and everything after `|` will be
// ignored
org.opentest4j.AssertionFailedError exception =
assertThrows(
org.opentest4j.AssertionFailedError.class, () -> verifyEntityLinkParser(expected));
assertEquals(
"expected: <<#E::table::special!@#$%^&*()_+[]{}|;:\\'\",./?>> but was: <<#E::table::special!@#$%^&*()_+[]{}>>",
exception.getMessage());

expected.clear();
expected.put("entityLink", "<#E::table::user<name>::column>");
IllegalArgumentException argException =
assertThrows(IllegalArgumentException.class, () -> verifyEntityLinkParser(expected));
assertEquals(
"Entity link was not found in <#E::table::user<name>::column>", argException.getMessage());

expected.clear();
expected.put("entityLink", "<#E::table::user>name::column>");
exception =
assertThrows(
org.opentest4j.AssertionFailedError.class, () -> verifyEntityLinkParser(expected));
assertEquals(
"expected: <<#E::table::user>name::column>> but was: <<#E::table::user>>",
exception.getMessage());

expected.clear();
expected.put("entityLink", "<#E::table::foo<>bar::baz>");
argException =
assertThrows(IllegalArgumentException.class, () -> verifyEntityLinkParser(expected));
assertEquals(
"Entity link was not found in <#E::table::foo<>bar::baz>", argException.getMessage());
}

void verifyEntityLinkParser(Map<String, String> expected) {
MessageParser.EntityLink entityLink =
MessageParser.EntityLink.parse(expected.get("entityLink"));
assertEquals(expected.get("entityLink"), entityLink.getLinkString());
assertEquals(expected.get("arrayFieldName"), entityLink.getArrayFieldName());
assertEquals(expected.get("arrayFieldValue"), entityLink.getArrayFieldValue());
assertEquals(expected.get("entityType"), entityLink.getEntityType());
assertEquals(expected.get("fieldName"), entityLink.getFieldName());
assertEquals(expected.get("entityFQN"), entityLink.getEntityFQN());
assertEquals(expected.get("linkType"), entityLink.getLinkType().toString());
assertEquals(expected.get("fullyQualifiedFieldType"), entityLink.getFullyQualifiedFieldType());
assertEquals(
expected.get("fullyQualifiedFieldValue"), entityLink.getFullyQualifiedFieldValue());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"entityLink": {
"description": "Link to an entity or field within an entity using this format `<#E::{entities}::{entityType}::{field}::{arrayFieldName}::{arrayFieldValue}`.",
"type": "string",
"pattern": "(?U)^<#E::\\w+::[\\w'\\- .&/:+\"\\\\()$#%]+>$"
"pattern": "(?U)^<#E::\\w+::(?:[^:<>|]|:[^:<>|])+(?:::(?:[^:<>|]|:[^:<>|])+)*>$"
},
"entityName": {
"description": "Name that identifies an entity.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
FormItemLayout,
} from '../../../../interface/FormUtils.interface';
import { generateFormFields } from '../../../../utils/formUtils';
import { escapeESReservedCharacters } from '../../../../utils/StringsUtils';
import ScheduleInterval from '../../../Settings/Services/AddIngestion/Steps/ScheduleInterval';
import { WorkflowExtraConfig } from '../../../Settings/Services/AddIngestion/Steps/ScheduleInterval.interface';
import { AddTestCaseList } from '../../AddTestCaseList/AddTestCaseList.component';
Expand Down Expand Up @@ -152,9 +153,9 @@ const AddTestSuitePipeline = ({
]}
valuePropName="selectedTest">
<AddTestCaseList
filters={`testSuite.fullyQualifiedName:${
filters={`testSuite.fullyQualifiedName:${escapeESReservedCharacters(
testSuiteFQN ?? fqn
}`}
)}`}
showButton={false}
/>
</Form.Item>
Expand Down
Loading