Skip to content

Commit be355e4

Browse files
Copilothotlong
Copilot
and
hotlong
committed
Address code review: Add schema refinements for validation and fix defaults
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent fd4969c commit be355e4

10 files changed

Lines changed: 118 additions & 10 deletions

File tree

‎content/docs/references/data/field.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ const result = AddressSchema.parse(data);
7979
| Property | Type | Required | Description |
8080
| :--- | :--- | :--- | :--- |
8181
| **uniqueness** | `boolean` | optional | Enforce unique values across all records |
82-
| **completeness** | `number` | optional | Minimum ratio of non-null values (0-1, default: 1) |
82+
| **completeness** | `number` | optional | Minimum ratio of non-null values (0-1, default: 0 = no requirement) |
8383
| **accuracy** | `object` | optional | Accuracy validation configuration |
8484

8585
---

‎packages/spec/json-schema/data/DataQualityRules.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
"type": "number",
1414
"minimum": 0,
1515
"maximum": 1,
16-
"default": 1,
17-
"description": "Minimum ratio of non-null values (0-1, default: 1)"
16+
"default": 0,
17+
"description": "Minimum ratio of non-null values (0-1, default: 0 = no requirement)"
1818
},
1919
"accuracy": {
2020
"type": "object",

‎packages/spec/json-schema/data/Field.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -797,8 +797,8 @@
797797
"type": "number",
798798
"minimum": 0,
799799
"maximum": 1,
800-
"default": 1,
801-
"description": "Minimum ratio of non-null values (0-1, default: 1)"
800+
"default": 0,
801+
"description": "Minimum ratio of non-null values (0-1, default: 0 = no requirement)"
802802
},
803803
"accuracy": {
804804
"type": "object",

‎packages/spec/json-schema/data/Object.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -854,8 +854,8 @@
854854
"type": "number",
855855
"minimum": 0,
856856
"maximum": 1,
857-
"default": 1,
858-
"description": "Minimum ratio of non-null values (0-1, default: 1)"
857+
"default": 0,
858+
"description": "Minimum ratio of non-null values (0-1, default: 0 = no requirement)"
859859
},
860860
"accuracy": {
861861
"type": "object",

‎packages/spec/json-schema/ui/FieldWidgetProps.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -817,8 +817,8 @@
817817
"type": "number",
818818
"minimum": 0,
819819
"maximum": 1,
820-
"default": 1,
821-
"description": "Minimum ratio of non-null values (0-1, default: 1)"
820+
"default": 0,
821+
"description": "Minimum ratio of non-null values (0-1, default: 0 = no requirement)"
822822
},
823823
"accuracy": {
824824
"type": "object",

‎packages/spec/src/data/field.zod.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ export const DataQualityRulesSchema = z.object({
280280
uniqueness: z.boolean().default(false).describe('Enforce unique values across all records'),
281281

282282
/** Completeness ratio (0-1) indicating minimum percentage of non-null values */
283-
completeness: z.number().min(0).max(1).default(1).describe('Minimum ratio of non-null values (0-1, default: 1)'),
283+
completeness: z.number().min(0).max(1).default(0).describe('Minimum ratio of non-null values (0-1, default: 0 = no requirement)'),
284284

285285
/** Accuracy validation against authoritative source */
286286
accuracy: z.object({

‎packages/spec/src/data/object.zod.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,14 @@ export const PartitioningConfigSchema = z.object({
151151
strategy: z.enum(['range', 'hash', 'list']).describe('Partitioning strategy: range (date ranges), hash (consistent hashing), list (predefined values)'),
152152
key: z.string().describe('Field name to partition by'),
153153
interval: z.string().optional().describe('Partition interval for range strategy (e.g., "1 month", "1 year")'),
154+
}).refine((data) => {
155+
// If strategy is 'range', interval must be provided
156+
if (data.strategy === 'range' && !data.interval) {
157+
return false;
158+
}
159+
return true;
160+
}, {
161+
message: 'interval is required when strategy is "range"',
154162
});
155163

156164
/**

‎packages/spec/src/system/driver-sql.test.ts‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,36 @@ describe('SSLConfigSchema', () => {
100100
const result = SSLConfigSchema.parse(sslConfig);
101101
expect(result.rejectUnauthorized).toBe(true);
102102
});
103+
104+
it('should reject SSL config with cert but no key', () => {
105+
const sslConfig = {
106+
cert: '/path/to/cert.pem',
107+
// missing key
108+
};
109+
110+
const result = SSLConfigSchema.safeParse(sslConfig);
111+
expect(result.success).toBe(false);
112+
});
113+
114+
it('should reject SSL config with key but no cert', () => {
115+
const sslConfig = {
116+
key: '/path/to/key.pem',
117+
// missing cert
118+
};
119+
120+
const result = SSLConfigSchema.safeParse(sslConfig);
121+
expect(result.success).toBe(false);
122+
});
123+
124+
it('should accept SSL config with both cert and key', () => {
125+
const sslConfig = {
126+
cert: '/path/to/cert.pem',
127+
key: '/path/to/key.pem',
128+
};
129+
130+
const result = SSLConfigSchema.safeParse(sslConfig);
131+
expect(result.success).toBe(true);
132+
});
103133
});
104134

105135
describe('SQLDriverConfigSchema', () => {
@@ -312,4 +342,51 @@ describe('SQLDriverConfigSchema', () => {
312342

313343
expect(() => SQLDriverConfigSchema.parse(config)).not.toThrow();
314344
});
345+
346+
it('should reject SQL driver config with ssl=true but no sslConfig', () => {
347+
const config = {
348+
name: 'test-db',
349+
type: 'sql',
350+
dialect: 'postgresql',
351+
connectionString: 'postgresql://localhost/test',
352+
dataTypeMapping: {
353+
text: 'VARCHAR(255)',
354+
number: 'NUMERIC',
355+
boolean: 'BOOLEAN',
356+
date: 'DATE',
357+
datetime: 'TIMESTAMP',
358+
},
359+
ssl: true,
360+
// missing sslConfig
361+
capabilities: {},
362+
};
363+
364+
const result = SQLDriverConfigSchema.safeParse(config);
365+
expect(result.success).toBe(false);
366+
});
367+
368+
it('should accept SQL driver config with ssl=true and sslConfig provided', () => {
369+
const config = {
370+
name: 'test-db',
371+
type: 'sql',
372+
dialect: 'postgresql',
373+
connectionString: 'postgresql://localhost/test',
374+
dataTypeMapping: {
375+
text: 'VARCHAR(255)',
376+
number: 'NUMERIC',
377+
boolean: 'BOOLEAN',
378+
date: 'DATE',
379+
datetime: 'TIMESTAMP',
380+
},
381+
ssl: true,
382+
sslConfig: {
383+
rejectUnauthorized: true,
384+
ca: '/path/to/ca.pem',
385+
},
386+
capabilities: {},
387+
};
388+
389+
const result = SQLDriverConfigSchema.safeParse(config);
390+
expect(result.success).toBe(true);
391+
});
315392
});

‎packages/spec/src/system/driver-sql.zod.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,13 @@ export const SSLConfigSchema = z.object({
6262
ca: z.string().optional().describe('CA certificate file path or content'),
6363
cert: z.string().optional().describe('Client certificate file path or content'),
6464
key: z.string().optional().describe('Client private key file path or content'),
65+
}).refine((data) => {
66+
// If cert is provided, key must also be provided, and vice versa
67+
const hasCert = data.cert !== undefined;
68+
const hasKey = data.key !== undefined;
69+
return hasCert === hasKey;
70+
}, {
71+
message: 'Client certificate (cert) and private key (key) must be provided together',
6572
});
6673

6774
export type SSLConfig = z.infer<typeof SSLConfigSchema>;
@@ -139,6 +146,14 @@ export const SQLDriverConfigSchema = DriverConfigSchema.extend({
139146
dataTypeMapping: DataTypeMappingSchema.describe('SQL data type mapping configuration'),
140147
ssl: z.boolean().default(false).describe('Enable SSL/TLS connection'),
141148
sslConfig: SSLConfigSchema.optional().describe('SSL/TLS configuration (required when ssl is true)'),
149+
}).refine((data) => {
150+
// If ssl is enabled, sslConfig must be provided
151+
if (data.ssl && !data.sslConfig) {
152+
return false;
153+
}
154+
return true;
155+
}, {
156+
message: 'sslConfig is required when ssl is true',
142157
});
143158

144159
export type SQLDriverConfig = z.infer<typeof SQLDriverConfigSchema>;

‎packages/spec/src/system/driver.zod.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,14 @@ export const DriverCapabilitiesSchema = z.object({
250250
* Whether the driver supports query result caching.
251251
*/
252252
queryCache: z.boolean().default(false).describe('Supports query result caching'),
253+
}).refine((data) => {
254+
// Ensure deprecated geoSpatial and new geospatialQuery are consistent if both are provided
255+
if (data.geoSpatial !== undefined && data.geospatialQuery !== undefined && data.geoSpatial !== data.geospatialQuery) {
256+
return false;
257+
}
258+
return true;
259+
}, {
260+
message: 'Deprecated geoSpatial and geospatialQuery must have the same value if both are provided',
253261
});
254262

255263
/**

0 commit comments

Comments
 (0)