Fixed issue: Quality tests can not be created if the column names have spaces or special characters #7623 (#7698)

* Fixed issue: Quality tests can not be created if the column names have spaces or special characters #7623

* Added logic to not decode fqn param from path

* Removed commented out code

* Allowed only "_" in spacial character in test case name

Co-authored-by: Teddy Crepineau <teddy.crepineau@gmail.com>
This commit is contained in:
Shailesh Parmar 2022-09-26 18:52:25 +05:30 committed by GitHub
parent e49ad000ee
commit b81686b426
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 89 additions and 30 deletions

View File

@ -22,6 +22,7 @@ import javax.validation.constraints.Min;
import javax.ws.rs.Consumes; import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE; import javax.ws.rs.DELETE;
import javax.ws.rs.DefaultValue; import javax.ws.rs.DefaultValue;
import javax.ws.rs.Encoded;
import javax.ws.rs.GET; import javax.ws.rs.GET;
import javax.ws.rs.PATCH; import javax.ws.rs.PATCH;
import javax.ws.rs.POST; import javax.ws.rs.POST;
@ -451,7 +452,8 @@ public class TestCaseResource extends EntityResource<TestCase, TestCaseRepositor
public Response addTestCaseResult( public Response addTestCaseResult(
@Context UriInfo uriInfo, @Context UriInfo uriInfo,
@Context SecurityContext securityContext, @Context SecurityContext securityContext,
@Parameter(description = "fqn of the testCase", schema = @Schema(type = "string")) @PathParam("fqn") String fqn, @Encoded @Parameter(description = "fqn of the testCase", schema = @Schema(type = "string")) @PathParam("fqn")
String fqn,
@Valid TestCaseResult testCaseResult) @Valid TestCaseResult testCaseResult)
throws IOException { throws IOException {
authorizer.authorizeAdmin(securityContext, true); authorizer.authorizeAdmin(securityContext, true);

View File

@ -41,6 +41,7 @@ import {
} from '../../utils/CommonUtils'; } from '../../utils/CommonUtils';
import { getTestSuitePath } from '../../utils/RouterUtils'; import { getTestSuitePath } from '../../utils/RouterUtils';
import { serviceTypeLogo } from '../../utils/ServiceUtils'; import { serviceTypeLogo } from '../../utils/ServiceUtils';
import { getDecodedFqn } from '../../utils/StringsUtils';
import { showErrorToast } from '../../utils/ToastUtils'; import { showErrorToast } from '../../utils/ToastUtils';
import SuccessScreen from '../common/success-screen/SuccessScreen'; import SuccessScreen from '../common/success-screen/SuccessScreen';
import TitleBreadcrumb from '../common/title-breadcrumb/title-breadcrumb.component'; import TitleBreadcrumb from '../common/title-breadcrumb/title-breadcrumb.component';
@ -103,7 +104,7 @@ const AddDataQualityTestV1: React.FC<AddDataQualityTestProps> = ({ table }) => {
if (isColumnFqn) { if (isColumnFqn) {
const colVal = [ const colVal = [
{ {
name: getPartialNameFromTableFQN(entityTypeFQN, [ name: getPartialNameFromTableFQN(getDecodedFqn(entityTypeFQN), [
FqnPart.NestedColumn, FqnPart.NestedColumn,
]), ]),
url: getTableTabPath(entityTypeFQN, 'profiler'), url: getTableTabPath(entityTypeFQN, 'profiler'),

View File

@ -37,7 +37,11 @@ import {
TestDefinition, TestDefinition,
TestPlatform, TestPlatform,
} from '../../../generated/tests/testDefinition'; } from '../../../generated/tests/testDefinition';
import { getNameFromFQN } from '../../../utils/CommonUtils'; import {
getNameFromFQN,
replaceAllSpacialCharWith_,
} from '../../../utils/CommonUtils';
import { getDecodedFqn, getEncodedFqn } from '../../../utils/StringsUtils';
import { generateEntityLink } from '../../../utils/TableUtils'; import { generateEntityLink } from '../../../utils/TableUtils';
import { showErrorToast } from '../../../utils/ToastUtils'; import { showErrorToast } from '../../../utils/ToastUtils';
import RichTextEditor from '../../common/rich-text-editor/RichTextEditor'; import RichTextEditor from '../../common/rich-text-editor/RichTextEditor';
@ -51,6 +55,7 @@ const TestCaseForm: React.FC<TestCaseFormProps> = ({
table, table,
}) => { }) => {
const { entityTypeFQN, dashboardType } = useParams<Record<string, string>>(); const { entityTypeFQN, dashboardType } = useParams<Record<string, string>>();
const decodedEntityFQN = getDecodedFqn(entityTypeFQN);
const isColumnFqn = dashboardType === ProfilerDashboardType.COLUMN; const isColumnFqn = dashboardType === ProfilerDashboardType.COLUMN;
const [form] = Form.useForm(); const [form] = Form.useForm();
const markdownRef = useRef<EditorContentRef>(); const markdownRef = useRef<EditorContentRef>();
@ -72,7 +77,7 @@ const TestCaseForm: React.FC<TestCaseFormProps> = ({
testPlatform: TestPlatform.OpenMetadata, testPlatform: TestPlatform.OpenMetadata,
supportedDataType: isColumnFqn supportedDataType: isColumnFqn
? table.columns.find( ? table.columns.find(
(column) => column.fullyQualifiedName === entityTypeFQN (column) => column.fullyQualifiedName === decodedEntityFQN
)?.dataType )?.dataType
: undefined, : undefined,
}); });
@ -87,7 +92,7 @@ const TestCaseForm: React.FC<TestCaseFormProps> = ({
const { data } = await getListTestCase({ const { data } = await getListTestCase({
fields: 'testDefinition', fields: 'testDefinition',
limit: API_RES_MAX_SIZE, limit: API_RES_MAX_SIZE,
entityLink: generateEntityLink(entityTypeFQN, isColumnFqn), entityLink: generateEntityLink(decodedEntityFQN, isColumnFqn),
}); });
setTestCases(data); setTestCases(data);
@ -158,7 +163,10 @@ const TestCaseForm: React.FC<TestCaseFormProps> = ({
return { return {
name: value.testName, name: value.testName,
entityLink: generateEntityLink(entityTypeFQN, isColumnFqn), entityLink: generateEntityLink(
getEncodedFqn(decodedEntityFQN, true),
isColumnFqn
),
parameterValues: parameterValues as TestCaseParameterValue[], parameterValues: parameterValues as TestCaseParameterValue[],
testDefinition: { testDefinition: {
id: value.testTypeId, id: value.testTypeId,
@ -201,13 +209,16 @@ const TestCaseForm: React.FC<TestCaseFormProps> = ({
); );
setSelectedTestType(value.testTypeId); setSelectedTestType(value.testTypeId);
const testCount = testCases.filter((test) => const testCount = testCases.filter((test) =>
test.name.includes(`${getNameFromFQN(entityTypeFQN)}_${testType?.name}`) test.name.includes(
`${getNameFromFQN(decodedEntityFQN)}_${testType?.name}`
)
); );
// generating dynamic unique name based on entity_testCase_number // generating dynamic unique name based on entity_testCase_number
form.setFieldsValue({ const name = `${getNameFromFQN(decodedEntityFQN)}_${testType?.name}${
testName: `${getNameFromFQN(entityTypeFQN)}_${testType?.name}${
testCount.length ? `_${testCount.length}` : '' testCount.length ? `_${testCount.length}` : ''
}`, }`;
form.setFieldsValue({
testName: replaceAllSpacialCharWith_(name),
}); });
} }
}; };
@ -220,7 +231,9 @@ const TestCaseForm: React.FC<TestCaseFormProps> = ({
fetchAllTestCases(); fetchAllTestCases();
} }
form.setFieldsValue({ form.setFieldsValue({
testName: initialValue?.name ?? getNameFromFQN(entityTypeFQN), testName: replaceAllSpacialCharWith_(
initialValue?.name ?? getNameFromFQN(decodedEntityFQN)
),
testTypeId: initialValue?.testDefinition?.id, testTypeId: initialValue?.testDefinition?.id,
params: initialValue?.parameterValues?.length params: initialValue?.parameterValues?.length
? getParamsValue() ? getParamsValue()
@ -243,6 +256,10 @@ const TestCaseForm: React.FC<TestCaseFormProps> = ({
required: true, required: true,
message: 'Name is required!', message: 'Name is required!',
}, },
{
pattern: /^[A-Za-z0-9_]*$/g,
message: 'Spacial character is not allowed!',
},
{ {
validator: (_, value) => { validator: (_, value) => {
if (testCases.some((test) => test.name === value)) { if (testCases.some((test) => test.name === value)) {

View File

@ -49,6 +49,7 @@ import {
getProfilerDashboardWithFqnPath, getProfilerDashboardWithFqnPath,
} from '../../utils/RouterUtils'; } from '../../utils/RouterUtils';
import { serviceTypeLogo } from '../../utils/ServiceUtils'; import { serviceTypeLogo } from '../../utils/ServiceUtils';
import { getDecodedFqn } from '../../utils/StringsUtils';
import { import {
generateEntityLink, generateEntityLink,
getTagsWithoutTier, getTagsWithoutTier,
@ -88,6 +89,7 @@ const ProfilerDashboard: React.FC<ProfilerDashboardProps> = ({
dashboardType: ProfilerDashboardType; dashboardType: ProfilerDashboardType;
tab: ProfilerDashboardTab; tab: ProfilerDashboardTab;
}>(); }>();
const decodedEntityFQN = getDecodedFqn(entityTypeFQN);
const isColumnView = dashboardType === ProfilerDashboardType.COLUMN; const isColumnView = dashboardType === ProfilerDashboardType.COLUMN;
const [follower, setFollower] = useState<EntityReference[]>([]); const [follower, setFollower] = useState<EntityReference[]>([]);
const [isFollowing, setIsFollowing] = useState<boolean>(false); const [isFollowing, setIsFollowing] = useState<boolean>(false);
@ -155,7 +157,7 @@ const ProfilerDashboard: React.FC<ProfilerDashboardProps> = ({
const breadcrumb = useMemo(() => { const breadcrumb = useMemo(() => {
const serviceName = getEntityName(table.service); const serviceName = getEntityName(table.service);
const fqn = table.fullyQualifiedName || ''; const fqn = table.fullyQualifiedName || '';
const columnName = getPartialNameFromTableFQN(entityTypeFQN, [ const columnName = getPartialNameFromTableFQN(decodedEntityFQN, [
FqnPart.NestedColumn, FqnPart.NestedColumn,
]); ]);
@ -337,7 +339,7 @@ const ProfilerDashboard: React.FC<ProfilerDashboardProps> = ({
tablePermissions.ViewBasic || tablePermissions.ViewBasic ||
tablePermissions.ViewTests) tablePermissions.ViewTests)
) { ) {
fetchTestCases(generateEntityLink(entityTypeFQN, true)); fetchTestCases(generateEntityLink(decodedEntityFQN, true));
} else if ( } else if (
ProfilerDashboardTab.PROFILER === value && ProfilerDashboardTab.PROFILER === value &&
(tablePermissions.ViewAll || (tablePermissions.ViewAll ||
@ -392,7 +394,7 @@ const ProfilerDashboard: React.FC<ProfilerDashboardProps> = ({
useEffect(() => { useEffect(() => {
if (table) { if (table) {
if (isColumnView) { if (isColumnView) {
const columnName = getNameFromFQN(entityTypeFQN); const columnName = getNameFromFQN(decodedEntityFQN);
const selectedColumn = table.columns.find( const selectedColumn = table.columns.find(
(col) => col.name === columnName (col) => col.name === columnName
); );

View File

@ -26,6 +26,7 @@ import { TestCase, TestCaseResult } from '../../../generated/tests/testCase';
import { useAuth } from '../../../hooks/authHooks'; import { useAuth } from '../../../hooks/authHooks';
import { getEntityName, getNameFromFQN } from '../../../utils/CommonUtils'; import { getEntityName, getNameFromFQN } from '../../../utils/CommonUtils';
import { getTestSuitePath } from '../../../utils/RouterUtils'; import { getTestSuitePath } from '../../../utils/RouterUtils';
import { getDecodedFqn } from '../../../utils/StringsUtils';
import SVGIcons, { Icons } from '../../../utils/SvgUtils'; import SVGIcons, { Icons } from '../../../utils/SvgUtils';
import { import {
getEntityFqnFromEntityLink, getEntityFqnFromEntityLink,
@ -126,15 +127,16 @@ const DataQualityTab: React.FC<DataQualityTabProps> = ({
if (isColumn) { if (isColumn) {
const name = getNameFromFQN( const name = getNameFromFQN(
getEntityFqnFromEntityLink(entityLink, isColumn) getDecodedFqn(
getEntityFqnFromEntityLink(entityLink, isColumn),
true
)
); );
return name; return name;
} }
return isColumn return '--';
? getNameFromFQN(getEntityFqnFromEntityLink(entityLink, isColumn))
: '--';
}, },
}, },
{ {

View File

@ -39,6 +39,7 @@ import {
TestCaseResult, TestCaseResult,
TestCaseStatus, TestCaseStatus,
} from '../../../generated/tests/testCase'; } from '../../../generated/tests/testCase';
import { getEncodedFqn } from '../../../utils/StringsUtils';
import { showErrorToast } from '../../../utils/ToastUtils'; import { showErrorToast } from '../../../utils/ToastUtils';
import ErrorPlaceHolder from '../../common/error-with-placeholder/ErrorPlaceHolder'; import ErrorPlaceHolder from '../../common/error-with-placeholder/ErrorPlaceHolder';
import RichTextEditorPreviewer from '../../common/rich-text-editor/RichTextEditorPreviewer'; import RichTextEditorPreviewer from '../../common/rich-text-editor/RichTextEditorPreviewer';
@ -130,7 +131,7 @@ const TestSummary: React.FC<TestSummaryProps> = ({ data }) => {
.unix(); .unix();
const endTs = moment().unix(); const endTs = moment().unix();
const { data: chartData } = await getListTestCaseResults( const { data: chartData } = await getListTestCaseResults(
data.fullyQualifiedName || '', getEncodedFqn(data.fullyQualifiedName || ''),
{ {
startTs, startTs,
endTs, endTs,

View File

@ -33,6 +33,7 @@ import {
getAddDataQualityTableTestPath, getAddDataQualityTableTestPath,
getProfilerDashboardWithFqnPath, getProfilerDashboardWithFqnPath,
} from '../../../utils/RouterUtils'; } from '../../../utils/RouterUtils';
import { getEncodedFqn } from '../../../utils/StringsUtils';
import SVGIcons, { Icons } from '../../../utils/SvgUtils'; import SVGIcons, { Icons } from '../../../utils/SvgUtils';
import Ellipses from '../../common/Ellipses/Ellipses'; import Ellipses from '../../common/Ellipses/Ellipses';
import Searchbar from '../../common/searchbar/Searchbar'; import Searchbar from '../../common/searchbar/Searchbar';
@ -165,7 +166,9 @@ const ColumnProfileTable: FC<ColumnProfileTableProps> = ({
key: 'dataQualityTest', key: 'dataQualityTest',
render: (_, record) => { render: (_, record) => {
const summary = const summary =
columnTestSummary?.[record.fullyQualifiedName || '']?.results; columnTestSummary?.[
getEncodedFqn(record.fullyQualifiedName || '', true)
]?.results;
const currentResult = summary const currentResult = summary
? Object.entries(summary).map(([key, value]) => ({ ? Object.entries(summary).map(([key, value]) => ({
value, value,
@ -246,7 +249,9 @@ const ColumnProfileTable: FC<ColumnProfileTableProps> = ({
setData( setData(
columns.map((col) => ({ columns.map((col) => ({
...col, ...col,
testCount: colResult?.[col.fullyQualifiedName || '']?.count, testCount:
colResult?.[getEncodedFqn(col.fullyQualifiedName || '', true)]
?.count,
})) }))
); );
setColumnTestSummary(colResult); setColumnTestSummary(colResult);

View File

@ -43,6 +43,7 @@ import {
getTableFQNFromColumnFQN, getTableFQNFromColumnFQN,
} from '../../utils/CommonUtils'; } from '../../utils/CommonUtils';
import { DEFAULT_ENTITY_PERMISSION } from '../../utils/PermissionsUtils'; import { DEFAULT_ENTITY_PERMISSION } from '../../utils/PermissionsUtils';
import { getDecodedFqn } from '../../utils/StringsUtils';
import { generateEntityLink } from '../../utils/TableUtils'; import { generateEntityLink } from '../../utils/TableUtils';
import { showErrorToast } from '../../utils/ToastUtils'; import { showErrorToast } from '../../utils/ToastUtils';
@ -52,6 +53,7 @@ const ProfilerDashboardPage = () => {
dashboardType: ProfilerDashboardType; dashboardType: ProfilerDashboardType;
tab: ProfilerDashboardTab; tab: ProfilerDashboardTab;
}>(); }>();
const decodedEntityFQN = getDecodedFqn(entityTypeFQN);
const isColumnView = dashboardType === ProfilerDashboardType.COLUMN; const isColumnView = dashboardType === ProfilerDashboardType.COLUMN;
const [table, setTable] = useState<Table>({} as Table); const [table, setTable] = useState<Table>({} as Table);
const [profilerData, setProfilerData] = useState<ColumnProfile[]>([]); const [profilerData, setProfilerData] = useState<ColumnProfile[]>([]);
@ -117,14 +119,14 @@ const ProfilerDashboardPage = () => {
}; };
const handleTestCaseUpdate = () => { const handleTestCaseUpdate = () => {
fetchTestCases(generateEntityLink(entityTypeFQN, isColumnView)); fetchTestCases(generateEntityLink(decodedEntityFQN, isColumnView));
}; };
const fetchTableEntity = async () => { const fetchTableEntity = async () => {
try { try {
const fqn = isColumnView const fqn = isColumnView
? getTableFQNFromColumnFQN(entityTypeFQN) ? getTableFQNFromColumnFQN(decodedEntityFQN)
: entityTypeFQN; : decodedEntityFQN;
const field = `tags, usageSummary, owner, followers${ const field = `tags, usageSummary, owner, followers${
isColumnView ? ', profile' : '' isColumnView ? ', profile' : ''
}`; }`;
@ -159,7 +161,7 @@ const ProfilerDashboardPage = () => {
tab === ProfilerDashboardTab.DATA_QUALITY && tab === ProfilerDashboardTab.DATA_QUALITY &&
(permission.ViewAll || permission.ViewBasic || permission.ViewTests) (permission.ViewAll || permission.ViewBasic || permission.ViewTests)
) { ) {
fetchTestCases(generateEntityLink(entityTypeFQN)); fetchTestCases(generateEntityLink(decodedEntityFQN));
} else if ( } else if (
permission.ViewAll || permission.ViewAll ||
permission.ViewBasic || permission.ViewBasic ||
@ -172,13 +174,13 @@ const ProfilerDashboardPage = () => {
}; };
useEffect(() => { useEffect(() => {
if (entityTypeFQN) { if (decodedEntityFQN) {
fetchTableEntity(); fetchTableEntity();
} else { } else {
setIsLoading(false); setIsLoading(false);
setError(true); setError(true);
} }
}, [entityTypeFQN]); }, [decodedEntityFQN]);
useEffect(() => { useEffect(() => {
if (!isEmpty(table)) { if (!isEmpty(table)) {
@ -201,7 +203,9 @@ const ProfilerDashboardPage = () => {
<ErrorPlaceHolder> <ErrorPlaceHolder>
<p className="tw-text-center"> <p className="tw-text-center">
No data found{' '} No data found{' '}
{entityTypeFQN ? `for column ${getNameFromFQN(entityTypeFQN)}` : ''} {decodedEntityFQN
? `for column ${getNameFromFQN(decodedEntityFQN)}`
: ''}
</p> </p>
</ErrorPlaceHolder> </ErrorPlaceHolder>
); );

View File

@ -711,6 +711,10 @@ export const replaceSpaceWith_ = (text: string) => {
return text.replace(/\s/g, '_'); return text.replace(/\s/g, '_');
}; };
export const replaceAllSpacialCharWith_ = (text: string) => {
return text.replaceAll(/[&/\\#, +()$~%.'":*?<>{}]/g, '_');
};
export const getFeedCounts = ( export const getFeedCounts = (
entityType: string, entityType: string,
entityFQN: string, entityFQN: string,

View File

@ -128,8 +128,29 @@ export const getErrorText = (
* @param fqn - Value to be encoded * @param fqn - Value to be encoded
* @returns - Encoded text string as a valid component of a Uniform Resource Identifier (URI). * @returns - Encoded text string as a valid component of a Uniform Resource Identifier (URI).
*/ */
export const getEncodedFqn = (fqn: string) => { export const getEncodedFqn = (fqn: string, spaceAsPlus = false) => {
return encodeURIComponent(fqn); let uri = encodeURIComponent(fqn);
if (spaceAsPlus) {
uri = uri.replaceAll('%20', '+');
}
return uri;
};
/**
*
* @param fqn - Value to be encoded
* @returns - Decode text string as a valid component of a Uniform Resource Identifier (URI).
*/
export const getDecodedFqn = (fqn: string, plusAsSpace = false) => {
let uri = decodeURIComponent(fqn);
if (plusAsSpace) {
uri = uri.replaceAll('+', ' ');
}
return uri;
}; };
/** /**