Skip to content

Commit ef0b1ac

Browse files
Copilothotlong
andcommitted
Address code review feedback
- Update terminology: ipWhitelist/ipBlacklist → allowedIps/blockedIps - Add validation: minSize <= maxSize - Add validation: virusScanProvider requires virusScan enabled - Add validation: transition action requires targetStorageClass - Add comprehensive tests for all validation rules - Remove incorrectly placed integration/object-storage.mdx - All 2238 tests passing Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 34375ec commit ef0b1ac

8 files changed

Lines changed: 141 additions & 16 deletions

File tree

content/docs/references/system/object-storage.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ const result = AccessControlConfigSchema.parse(data);
3535
| **maxAge** | `number` | optional | CORS preflight cache duration in seconds |
3636
| **corsEnabled** | `boolean` | optional | Enable CORS configuration |
3737
| **publicAccess** | `object` | optional | Public access control |
38-
| **ipWhitelist** | `string[]` | optional | Allowed IP addresses/CIDR blocks |
39-
| **ipBlacklist** | `string[]` | optional | Blocked IP addresses/CIDR blocks |
38+
| **allowedIps** | `string[]` | optional | Allowed IP addresses/CIDR blocks |
39+
| **blockedIps** | `string[]` | optional | Blocked IP addresses/CIDR blocks |
4040

4141
---
4242

packages/spec/json-schema/system/AccessControlConfig.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,14 @@
8484
"additionalProperties": false,
8585
"description": "Public access control"
8686
},
87-
"ipWhitelist": {
87+
"allowedIps": {
8888
"type": "array",
8989
"items": {
9090
"type": "string"
9191
},
9292
"description": "Allowed IP addresses/CIDR blocks"
9393
},
94-
"ipBlacklist": {
94+
"blockedIps": {
9595
"type": "array",
9696
"items": {
9797
"type": "string"

packages/spec/json-schema/system/BucketConfig.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,14 @@
161161
"additionalProperties": false,
162162
"description": "Public access control"
163163
},
164-
"ipWhitelist": {
164+
"allowedIps": {
165165
"type": "array",
166166
"items": {
167167
"type": "string"
168168
},
169169
"description": "Allowed IP addresses/CIDR blocks"
170170
},
171-
"ipBlacklist": {
171+
"blockedIps": {
172172
"type": "array",
173173
"items": {
174174
"type": "string"

packages/spec/json-schema/system/ObjectStorageConfig.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,14 +248,14 @@
248248
"additionalProperties": false,
249249
"description": "Public access control"
250250
},
251-
"ipWhitelist": {
251+
"allowedIps": {
252252
"type": "array",
253253
"items": {
254254
"type": "string"
255255
},
256256
"description": "Allowed IP addresses/CIDR blocks"
257257
},
258-
"ipBlacklist": {
258+
"blockedIps": {
259259
"type": "array",
260260
"items": {
261261
"type": "string"

packages/spec/src/data/field.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,5 +1331,59 @@ describe('Field Factory Helpers', () => {
13311331
maxVersions: 0,
13321332
})).toThrow();
13331333
});
1334+
1335+
it('should reject minSize greater than maxSize', () => {
1336+
expect(() => FileAttachmentConfigSchema.parse({
1337+
minSize: 1000,
1338+
maxSize: 500,
1339+
})).toThrow();
1340+
1341+
// Verify the specific error message
1342+
try {
1343+
FileAttachmentConfigSchema.parse({
1344+
minSize: 1000,
1345+
maxSize: 500,
1346+
});
1347+
} catch (error: any) {
1348+
expect(error.issues[0].message).toContain('minSize must be less than or equal to maxSize');
1349+
}
1350+
});
1351+
1352+
it('should accept valid minSize and maxSize', () => {
1353+
const config = FileAttachmentConfigSchema.parse({
1354+
minSize: 500,
1355+
maxSize: 1000,
1356+
});
1357+
1358+
expect(config.minSize).toBe(500);
1359+
expect(config.maxSize).toBe(1000);
1360+
});
1361+
1362+
it('should reject virusScanProvider without virusScan enabled', () => {
1363+
expect(() => FileAttachmentConfigSchema.parse({
1364+
virusScan: false,
1365+
virusScanProvider: 'clamav',
1366+
})).toThrow();
1367+
1368+
// Verify the specific error message
1369+
try {
1370+
FileAttachmentConfigSchema.parse({
1371+
virusScan: false,
1372+
virusScanProvider: 'clamav',
1373+
});
1374+
} catch (error: any) {
1375+
expect(error.issues[0].message).toContain('virusScanProvider requires virusScan to be enabled');
1376+
}
1377+
});
1378+
1379+
it('should accept virusScanProvider when virusScan is enabled', () => {
1380+
const config = FileAttachmentConfigSchema.parse({
1381+
virusScan: true,
1382+
virusScanProvider: 'clamav',
1383+
});
1384+
1385+
expect(config.virusScan).toBe(true);
1386+
expect(config.virusScanProvider).toBe('clamav');
1387+
});
13341388
});
13351389
});

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ export const VectorConfigSchema = z.object({
160160
* Configuration for file and attachment field types
161161
*
162162
* Provides comprehensive file upload capabilities with:
163-
* - File type restrictions (whitelist/blacklist)
163+
* - File type restrictions (allowed/blocked)
164164
* - File size limits (min/max)
165165
* - Virus scanning integration
166166
* - Storage provider integration
@@ -241,6 +241,22 @@ export const FileAttachmentConfigSchema = z.object({
241241
/** Access Control */
242242
publicRead: z.boolean().default(false).describe('Allow public read access to uploaded files'),
243243
presignedUrlExpiry: z.number().min(60).max(604800).default(3600).describe('Presigned URL expiration in seconds (default: 1 hour)'),
244+
}).refine((data) => {
245+
// Validate minSize is less than or equal to maxSize
246+
if (data.minSize !== undefined && data.maxSize !== undefined && data.minSize > data.maxSize) {
247+
return false;
248+
}
249+
return true;
250+
}, {
251+
message: 'minSize must be less than or equal to maxSize',
252+
}).refine((data) => {
253+
// Validate virusScanProvider requires virusScan to be enabled
254+
if (data.virusScanProvider !== undefined && data.virusScan !== true) {
255+
return false;
256+
}
257+
return true;
258+
}, {
259+
message: 'virusScanProvider requires virusScan to be enabled',
244260
});
245261

246262
/**

packages/spec/src/system/object-storage.test.ts

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -265,14 +265,14 @@ describe('AccessControlConfigSchema', () => {
265265
expect(config.publicAccess?.allowPublicWrite).toBe(false);
266266
});
267267

268-
it('should accept IP whitelist/blacklist', () => {
268+
it('should accept IP allow/block lists', () => {
269269
const config = AccessControlConfigSchema.parse({
270-
ipWhitelist: ['192.168.1.0/24', '10.0.0.1'],
271-
ipBlacklist: ['1.2.3.4'],
270+
allowedIps: ['192.168.1.0/24', '10.0.0.1'],
271+
blockedIps: ['1.2.3.4'],
272272
});
273273

274-
expect(config.ipWhitelist).toHaveLength(2);
275-
expect(config.ipBlacklist).toHaveLength(1);
274+
expect(config.allowedIps).toHaveLength(2);
275+
expect(config.blockedIps).toHaveLength(1);
276276
});
277277
});
278278

@@ -342,6 +342,53 @@ describe('LifecyclePolicyRuleSchema', () => {
342342
daysAfterCreation: 30,
343343
})).toThrow();
344344
});
345+
346+
it('should require targetStorageClass when action is transition', () => {
347+
expect(() => LifecyclePolicyRuleSchema.parse({
348+
id: 'move_to_glacier',
349+
enabled: true,
350+
action: 'transition',
351+
daysAfterCreation: 30,
352+
// missing targetStorageClass
353+
})).toThrow();
354+
355+
// Verify the specific error message
356+
try {
357+
LifecyclePolicyRuleSchema.parse({
358+
id: 'move_to_glacier',
359+
enabled: true,
360+
action: 'transition',
361+
daysAfterCreation: 30,
362+
});
363+
} catch (error: any) {
364+
expect(error.issues[0].message).toContain('targetStorageClass is required');
365+
}
366+
});
367+
368+
it('should accept transition rule with targetStorageClass', () => {
369+
const rule = LifecyclePolicyRuleSchema.parse({
370+
id: 'move_to_glacier',
371+
enabled: true,
372+
action: 'transition',
373+
daysAfterCreation: 30,
374+
targetStorageClass: 'glacier',
375+
});
376+
377+
expect(rule.action).toBe('transition');
378+
expect(rule.targetStorageClass).toBe('glacier');
379+
});
380+
381+
it('should allow delete action without targetStorageClass', () => {
382+
const rule = LifecyclePolicyRuleSchema.parse({
383+
id: 'delete_old',
384+
enabled: true,
385+
action: 'delete',
386+
daysAfterCreation: 365,
387+
});
388+
389+
expect(rule.action).toBe('delete');
390+
expect(rule.targetStorageClass).toBeUndefined();
391+
});
345392
});
346393

347394
describe('LifecyclePolicyConfigSchema', () => {

packages/spec/src/system/object-storage.zod.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,8 @@ export const AccessControlConfigSchema = z.object({
214214
allowPublicWrite: z.boolean().default(false).describe('Allow public write access'),
215215
allowPublicList: z.boolean().default(false).describe('Allow public bucket listing'),
216216
}).optional().describe('Public access control'),
217-
ipWhitelist: z.array(z.string()).optional().describe('Allowed IP addresses/CIDR blocks'),
218-
ipBlacklist: z.array(z.string()).optional().describe('Blocked IP addresses/CIDR blocks'),
217+
allowedIps: z.array(z.string()).optional().describe('Allowed IP addresses/CIDR blocks'),
218+
blockedIps: z.array(z.string()).optional().describe('Blocked IP addresses/CIDR blocks'),
219219
});
220220

221221
export type AccessControlConfig = z.infer<typeof AccessControlConfigSchema>;
@@ -253,6 +253,14 @@ export const LifecyclePolicyRuleSchema = z.object({
253253
daysAfterCreation: z.number().min(0).optional().describe('Days after object creation'),
254254
daysAfterModification: z.number().min(0).optional().describe('Days after last modification'),
255255
targetStorageClass: StorageClassSchema.optional().describe('Target storage class for transition action'),
256+
}).refine((data) => {
257+
// Validate that transition action has targetStorageClass
258+
if (data.action === 'transition' && !data.targetStorageClass) {
259+
return false;
260+
}
261+
return true;
262+
}, {
263+
message: 'targetStorageClass is required when action is "transition"',
256264
});
257265

258266
export type LifecyclePolicyRule = z.infer<typeof LifecyclePolicyRuleSchema>;

0 commit comments

Comments
 (0)