Skip to content

Commit be93742

Browse files
committed
josh - chaneg the return to null instead of unknown
Signed-off-by: Jialiang Liang <[email protected]>
1 parent e0fd3ef commit be93742

File tree

4 files changed

+82
-48
lines changed

4 files changed

+82
-48
lines changed

src/plugins/dashboard/public/application/embeddable/grid/dashboard_grid.test.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -364,11 +364,11 @@ test('synchronizeNow does nothing when feature flag is disabled', async () => {
364364
expect(startLoadingSpy).not.toHaveBeenCalled();
365365
});
366366

367-
test('synchronizeNow does nothing when metadata contains unknown values', async () => {
367+
test('synchronizeNow does nothing when metadata contains invalid values', async () => {
368368
const { props, options } = prepare({ isDirectQuerySyncEnabled: true });
369369

370370
(extractIndexInfoFromDashboard as jest.Mock).mockResolvedValue({
371-
parts: { datasource: 'ds', database: 'db', index: 'unknown' },
371+
parts: { datasource: 'ds', database: 'db', index: null },
372372
mapping: { lastRefreshTime: 123456, refreshInterval: 30000 },
373373
mdsId: '',
374374
});
@@ -390,11 +390,12 @@ test('synchronizeNow does nothing when metadata contains unknown values', async
390390
expect(startLoadingSpy).not.toHaveBeenCalled();
391391
});
392392

393+
// New tests for recent changes
393394
test('synchronizeNow exits early when feature flag is disabled and datasource is invalid', async () => {
394395
const { props, options } = prepare({ isDirectQuerySyncEnabled: false });
395396

396397
(extractIndexInfoFromDashboard as jest.Mock).mockResolvedValue({
397-
parts: { datasource: 'ds', database: 'db', index: 'unknown' },
398+
parts: { datasource: 'ds', database: 'db', index: null },
398399
mapping: { lastRefreshTime: 123456, refreshInterval: 30000 },
399400
mdsId: '',
400401
});
@@ -420,7 +421,7 @@ test('synchronizeNow exits early when feature flag is enabled but datasource is
420421
const { props, options } = prepare({ isDirectQuerySyncEnabled: true });
421422

422423
(extractIndexInfoFromDashboard as jest.Mock).mockResolvedValue({
423-
parts: { datasource: 'ds', database: '', index: 'idx' },
424+
parts: { datasource: 'ds', database: null, index: 'idx' },
424425
mapping: { lastRefreshTime: 123456, refreshInterval: 30000 },
425426
mdsId: '',
426427
});
@@ -468,7 +469,7 @@ test('areDataSourceParamsValid returns false for invalid datasource params', asy
468469
const { props, options } = prepare({ isDirectQuerySyncEnabled: true });
469470

470471
(extractIndexInfoFromDashboard as jest.Mock).mockResolvedValue({
471-
parts: { datasource: 'ds', database: 'unknown', index: 'idx' },
472+
parts: { datasource: 'ds', database: null, index: 'idx' },
472473
mapping: { lastRefreshTime: 123456, refreshInterval: 30000 },
473474
mdsId: '',
474475
});

src/plugins/dashboard/public/application/embeddable/grid/dashboard_grid.tsx

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,9 @@ class DashboardGridUi extends React.Component<DashboardGridProps, State> {
175175
// item.
176176
private gridItems = {} as { [key: string]: HTMLDivElement | null };
177177

178-
private extractedDatasource?: string;
179-
private extractedDatabase?: string;
180-
private extractedIndex?: string;
178+
private extractedDatasource?: string | null;
179+
private extractedDatabase?: string | null;
180+
private extractedIndex?: string | null;
181181

182182
constructor(props: DashboardGridProps) {
183183
super(props);
@@ -303,18 +303,11 @@ class DashboardGridUi extends React.Component<DashboardGridProps, State> {
303303

304304
/**
305305
* Validates if the extracted datasource, database, and index are present and valid.
306-
* Returns true if all values are non-empty and not 'unknown', false otherwise.
306+
* Returns true if all values are non-null, false otherwise.
307307
*/
308308
private areDataSourceParamsValid(): boolean {
309309
const { extractedDatasource, extractedDatabase, extractedIndex } = this;
310-
return (
311-
!!extractedDatasource &&
312-
!!extractedDatabase &&
313-
!!extractedIndex &&
314-
extractedDatasource !== 'unknown' &&
315-
extractedDatabase !== 'unknown' &&
316-
extractedIndex !== 'unknown'
317-
);
310+
return !!extractedDatasource && !!extractedDatabase && !!extractedIndex;
318311
}
319312

320313
/**
@@ -352,9 +345,7 @@ class DashboardGridUi extends React.Component<DashboardGridProps, State> {
352345
* and triggers the sync process if direct query sync is enabled.
353346
*/
354347
private synchronizeNow = () => {
355-
if (!this.isDirectQuerySyncEnabled() || !this.areDataSourceParamsValid()) {
356-
return;
357-
}
348+
if (!this.isDirectQuerySyncEnabled() || !this.areDataSourceParamsValid()) return;
358349

359350
const { extractedDatasource, extractedDatabase, extractedIndex } = this;
360351

src/plugins/dashboard/public/application/utils/direct_query_sync/direct_query_sync.test.ts

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,30 +104,39 @@ describe('resolveConcreteIndex', () => {
104104
});
105105

106106
describe('extractIndexParts', () => {
107-
it('correctly extracts parts from a full index name', () => {
108-
const result = extractIndexParts('flint_datasource1_database1_my_index');
107+
it('correctly extracts parts from a mapping name', () => {
108+
const result = extractIndexParts('datasource1.database1.my_index');
109109
expect(result).toEqual({
110110
datasource: 'datasource1',
111111
database: 'database1',
112112
index: 'my_index',
113113
});
114114
});
115115

116-
it('handles missing parts with unknown values', () => {
117-
const result = extractIndexParts('flint_datasource1');
116+
it('handles missing parts with null values', () => {
117+
const result = extractIndexParts('datasource1');
118118
expect(result).toEqual({
119119
datasource: 'datasource1',
120-
database: 'unknown',
121-
index: 'unknown',
120+
database: null,
121+
index: null,
122122
});
123123
});
124124

125-
it('handles empty index name', () => {
125+
it('handles empty mapping name with null values', () => {
126126
const result = extractIndexParts('');
127127
expect(result).toEqual({
128-
datasource: 'unknown',
129-
database: 'unknown',
130-
index: 'unknown',
128+
datasource: null,
129+
database: null,
130+
index: null,
131+
});
132+
});
133+
134+
it('returns null values when mappingName is undefined', () => {
135+
const result = extractIndexParts(undefined);
136+
expect(result).toEqual({
137+
datasource: null,
138+
database: null,
139+
index: null,
131140
});
132141
});
133142
});
@@ -142,6 +151,39 @@ describe('generateRefreshQuery', () => {
142151
const result = generateRefreshQuery(info);
143152
expect(result).toBe('REFRESH MATERIALIZED VIEW `datasource1`.`database1`.`my_index`');
144153
});
154+
155+
it('throws an error if datasource is null', () => {
156+
const info = {
157+
datasource: null,
158+
database: 'database1',
159+
index: 'my_index',
160+
};
161+
expect(() => generateRefreshQuery(info)).toThrow(
162+
'Cannot generate refresh query: missing required datasource, database, or index'
163+
);
164+
});
165+
166+
it('throws an error if database is null', () => {
167+
const info = {
168+
datasource: 'datasource1',
169+
database: null,
170+
index: 'my_index',
171+
};
172+
expect(() => generateRefreshQuery(info)).toThrow(
173+
'Cannot generate refresh query: missing required datasource, database, or index'
174+
);
175+
});
176+
177+
it('throws an error if index is null', () => {
178+
const info = {
179+
datasource: 'datasource1',
180+
database: 'database1',
181+
index: null,
182+
};
183+
expect(() => generateRefreshQuery(info)).toThrow(
184+
'Cannot generate refresh query: missing required datasource, database, or index'
185+
);
186+
});
145187
});
146188

147189
describe('fetchIndexMapping', () => {

src/plugins/dashboard/public/application/utils/direct_query_sync/direct_query_sync.ts

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ import { HttpStart, SavedObjectsClientContract } from 'src/core/public';
77
import { i18n } from '@osd/i18n';
88

99
interface IndexExtractionResult {
10-
datasource: string;
11-
database: string;
12-
index: string;
10+
datasource: string | null;
11+
database: string | null;
12+
index: string | null;
1313
}
1414

1515
export const DIRECT_QUERY_BASE = '/api/directquery';
@@ -67,31 +67,31 @@ export async function resolveConcreteIndex(
6767
}
6868
}
6969

70-
export function extractIndexParts(
71-
fullIndexName: string,
72-
mappingName?: string
73-
): IndexExtractionResult {
74-
// Prefer parsing the mapping name if provided
70+
export function extractIndexParts(mappingName?: string): IndexExtractionResult {
71+
// Use mapping name if provided; otherwise, return null values
7572
if (mappingName) {
7673
const parts = mappingName.split('.');
7774
return {
78-
datasource: parts[0] || 'unknown',
79-
database: parts[1] || 'unknown',
80-
index: parts.slice(2).join('.') || 'unknown',
75+
datasource: parts[0] || null,
76+
database: parts[1] || null,
77+
index: parts.slice(2).join('.') || null,
8178
};
8279
}
8380

84-
// Fallback to original regex-based parsing
85-
const trimmed = fullIndexName.replace(/^flint_/, '');
86-
const parts = trimmed.split('_');
8781
return {
88-
datasource: parts[0] || 'unknown',
89-
database: parts[1] || 'unknown',
90-
index: parts.slice(2).join('_') || 'unknown',
82+
datasource: null,
83+
database: null,
84+
index: null,
9185
};
9286
}
9387

9488
export function generateRefreshQuery(info: IndexExtractionResult): string {
89+
// Ensure all required fields are non-null before constructing the query
90+
if (!info.datasource || !info.database || !info.index) {
91+
throw new Error(
92+
'Cannot generate refresh query: missing required datasource, database, or index'
93+
);
94+
}
9595
return `REFRESH MATERIALIZED VIEW \`${info.datasource}\`.\`${info.database}\`.\`${info.index}\``;
9696
}
9797

@@ -176,7 +176,7 @@ export async function extractIndexInfoFromDashboard(
176176
const mappingName = val.mappings?._meta?.name;
177177
return {
178178
mapping: val.mappings._meta.properties!,
179-
parts: extractIndexParts(concreteTitle, mappingName),
179+
parts: extractIndexParts(mappingName),
180180
mdsId: selectedMdsId,
181181
};
182182
}

0 commit comments

Comments
 (0)