Refactor API to conform with new specifications#10
Merged
Conversation
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Copilot stopped work on behalf of
hotlong due to an error
January 11, 2026 08:58
Contributor
|
@copilot 继续 |
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Contributor
Author
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor API to conform with new specifications
Refactor API to conform with new specifications
Jan 11, 2026
hotlong
approved these changes
Jan 11, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request successfully implements a comprehensive API refactoring to conform with new specifications documented in docs/api/README.md. The refactoring introduces REST-style API endpoints, enhanced error handling with standardized error codes, AI context support for explainability, and expanded metadata API capabilities.
Changes:
- Implements complete REST API adapter with CRUD operations mapped to HTTP methods
- Adds standardized error codes and HTTP status code mapping across all adapters
- Introduces AI context support for request tracking and debugging
- Enhances metadata API with field-level and actions endpoints
- Updates example application to demonstrate all API styles
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/server/src/types.ts | Adds ErrorCode enum, AIContext and ErrorDetails interfaces, extends ObjectQLRequest and ObjectQLResponse |
| packages/server/src/server.ts | Implements AI context logging, enhanced error handling, improved findOne with string ID support, standardized delete responses |
| packages/server/src/adapters/rest.ts | NEW: Complete REST adapter supporting GET/POST/PUT/DELETE with query parameter parsing and error mapping |
| packages/server/src/adapters/node.ts | Enhances JSON-RPC handler with AI context support and proper HTTP status codes |
| packages/server/src/metadata.ts | Adds field-level metadata endpoint and actions listing endpoint with enhanced error responses |
| packages/server/src/index.ts | Exports REST adapter for public use |
| packages/server/test/rest.test.ts | NEW: 7 REST API tests covering CRUD operations and error cases |
| examples/starters/express-api/src/index.ts | Integrates REST handler and updates console output with all endpoint examples |
| API_REFACTORING_SUMMARY.md | NEW: Comprehensive documentation of refactoring changes with usage examples |
Comment on lines
+24
to
+30
| // Try to parse JSON values | ||
| try { | ||
| params[decodedKey] = JSON.parse(decodedValue); | ||
| } catch { | ||
| params[decodedKey] = decodedValue; | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential security vulnerability: The parseQueryParams function uses JSON.parse on user-supplied query parameter values without proper error handling or validation. A malicious user could potentially supply crafted JSON that could cause issues. While there's a try-catch that falls back to the raw string, there's no validation of what the parsed JSON contains (e.g., it could be an object with prototype pollution attempts).
Comment on lines
+111
to
156
| private handleError(error: any): ObjectQLResponse { | ||
| console.error('[ObjectQL Server] Error:', error); | ||
|
|
||
| // Handle validation errors | ||
| if (error.name === 'ValidationError' || error.code === 'VALIDATION_ERROR') { | ||
| return this.errorResponse( | ||
| ErrorCode.VALIDATION_ERROR, | ||
| 'Validation failed', | ||
| { fields: error.fields || error.details } | ||
| ); | ||
| } | ||
|
|
||
| // Handle permission errors | ||
| if (error.name === 'PermissionError' || error.code === 'FORBIDDEN') { | ||
| return this.errorResponse( | ||
| ErrorCode.FORBIDDEN, | ||
| error.message || 'You do not have permission to access this resource', | ||
| error.details | ||
| ); | ||
| } | ||
|
|
||
| // Handle not found errors | ||
| if (error.name === 'NotFoundError' || error.code === 'NOT_FOUND') { | ||
| return this.errorResponse( | ||
| ErrorCode.NOT_FOUND, | ||
| error.message || 'Resource not found' | ||
| ); | ||
| } | ||
|
|
||
| // Handle conflict errors (e.g., unique constraint violations) | ||
| if (error.name === 'ConflictError' || error.code === 'CONFLICT') { | ||
| return this.errorResponse( | ||
| ErrorCode.CONFLICT, | ||
| error.message || 'Resource conflict', | ||
| error.details | ||
| ); | ||
| } | ||
|
|
||
| // Handle database errors | ||
| if (error.name === 'DatabaseError' || error.code?.startsWith('DB_')) { | ||
| return this.errorResponse( | ||
| ErrorCode.DATABASE_ERROR, | ||
| 'Database operation failed', | ||
| { originalError: error.message } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Error handling relies on string matching of error names and codes (e.g., error.name === 'ValidationError' or error.code === 'VALIDATION_ERROR'). This is fragile and could break if error sources change their error naming conventions. Consider using instanceof checks for error classes or a more robust error type system.
Comment on lines
+64
to
+160
| describe('REST API Adapter', () => { | ||
| let app: ObjectQL; | ||
| let server: any; | ||
| let handler: any; | ||
|
|
||
| beforeAll(async () => { | ||
| app = new ObjectQL({ | ||
| datasources: { | ||
| default: new MockDriver() | ||
| } | ||
| }); | ||
|
|
||
| // Manual schema registration | ||
| app.metadata.register('object', { | ||
| type: 'object', | ||
| id: 'user', | ||
| content: { | ||
| name: 'user', | ||
| fields: { | ||
| name: { type: 'text' }, | ||
| email: { type: 'email' } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| // Create handler and server once for all tests | ||
| handler = createRESTHandler(app); | ||
| server = createServer(handler); | ||
| }); | ||
|
|
||
| it('should handle GET /api/data/:object - List records', async () => { | ||
| const response = await request(server) | ||
| .get('/api/data/user') | ||
| .set('Accept', 'application/json'); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body.data).toHaveLength(2); | ||
| expect(response.body.data[0].name).toBe('Alice'); | ||
| }); | ||
|
|
||
| it('should handle GET /api/data/:object/:id - Get single record', async () => { | ||
| const response = await request(server) | ||
| .get('/api/data/user/1') | ||
| .set('Accept', 'application/json'); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body.data.name).toBe('Alice'); | ||
| }); | ||
|
|
||
| it('should handle POST /api/data/:object - Create record', async () => { | ||
| const response = await request(server) | ||
| .post('/api/data/user') | ||
| .send({ name: 'Charlie', email: 'charlie@example.com' }) | ||
| .set('Accept', 'application/json'); | ||
|
|
||
| expect(response.status).toBe(201); | ||
| expect(response.body.data.name).toBe('Charlie'); | ||
| expect(response.body.data._id).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should handle PUT /api/data/:object/:id - Update record', async () => { | ||
| const response = await request(server) | ||
| .put('/api/data/user/1') | ||
| .send({ name: 'Alice Updated' }) | ||
| .set('Accept', 'application/json'); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| }); | ||
|
|
||
| it('should handle DELETE /api/data/:object/:id - Delete record', async () => { | ||
| const response = await request(server) | ||
| .delete('/api/data/user/1') | ||
| .set('Accept', 'application/json'); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body.data.deleted).toBe(true); | ||
| }); | ||
|
|
||
| it('should return 404 for non-existent object', async () => { | ||
| const response = await request(server) | ||
| .get('/api/data/nonexistent') | ||
| .set('Accept', 'application/json'); | ||
|
|
||
| expect(response.status).toBe(404); | ||
| expect(response.body.error.code).toBe('NOT_FOUND'); | ||
| }); | ||
|
|
||
| it('should return 400 for update without ID', async () => { | ||
| const response = await request(server) | ||
| .put('/api/data/user') | ||
| .send({ name: 'Test' }) | ||
| .set('Accept', 'application/json'); | ||
|
|
||
| expect(response.status).toBe(400); | ||
| expect(response.body.error.code).toBe('INVALID_REQUEST'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test suite shares state through a single MockDriver instance that is modified across tests (e.g., the POST test creates a user with _id '3', the PUT test updates user '1', and the DELETE test deletes user '1'). This creates interdependencies between tests where later tests depend on the state created/modified by earlier tests. Tests should be independent and idempotent. Consider resetting the driver state in beforeEach or using separate data for each test.
Comment on lines
+103
to
+181
| // GET /api/metadata/objects/:name/fields/:field - Get field metadata | ||
| const fieldMatch = url.match(/^\/api\/metadata\/objects\/([^\/]+)\/fields\/([^\/\?]+)$/); | ||
| if (method === 'GET' && fieldMatch) { | ||
| const [, objectName, fieldName] = fieldMatch; | ||
| const metadata = app.getObject(objectName); | ||
|
|
||
| if (!metadata) { | ||
| res.statusCode = 404; | ||
| res.end(JSON.stringify({ | ||
| error: { | ||
| code: ErrorCode.NOT_FOUND, | ||
| message: `Object '${objectName}' not found` | ||
| } | ||
| })); | ||
| return; | ||
| } | ||
|
|
||
| const field = metadata.fields?.[fieldName]; | ||
| if (!field) { | ||
| res.statusCode = 404; | ||
| res.end(JSON.stringify({ | ||
| error: { | ||
| code: ErrorCode.NOT_FOUND, | ||
| message: `Field '${fieldName}' not found in object '${objectName}'` | ||
| } | ||
| })); | ||
| return; | ||
| } | ||
|
|
||
| res.setHeader('Content-Type', 'application/json'); | ||
| res.statusCode = 200; | ||
| res.end(JSON.stringify({ | ||
| name: field.name || fieldName, | ||
| type: field.type, | ||
| label: field.label, | ||
| required: field.required, | ||
| unique: field.unique, | ||
| defaultValue: field.defaultValue, | ||
| options: field.options, | ||
| min: field.min, | ||
| max: field.max, | ||
| min_length: field.min_length, | ||
| max_length: field.max_length, | ||
| regex: field.regex | ||
| })); | ||
| return; | ||
| } | ||
|
|
||
| // GET /api/metadata/objects/:name/actions - List actions | ||
| const actionsMatch = url.match(/^\/api\/metadata\/objects\/([^\/]+)\/actions$/); | ||
| if (method === 'GET' && actionsMatch) { | ||
| const [, objectName] = actionsMatch; | ||
| const metadata = app.getObject(objectName); | ||
|
|
||
| if (!metadata) { | ||
| res.statusCode = 404; | ||
| res.end(JSON.stringify({ | ||
| error: { | ||
| code: ErrorCode.NOT_FOUND, | ||
| message: `Object '${objectName}' not found` | ||
| } | ||
| })); | ||
| return; | ||
| } | ||
|
|
||
| const actions = metadata.actions || {}; | ||
| const formattedActions = Object.entries(actions).map(([key, action]) => ({ | ||
| name: key, | ||
| type: action.type || 'record', | ||
| label: action.label || key, | ||
| params: action.params || {}, | ||
| description: action.description | ||
| })); | ||
|
|
||
| res.setHeader('Content-Type', 'application/json'); | ||
| res.statusCode = 200; | ||
| res.end(JSON.stringify({ actions: formattedActions })); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The new metadata endpoints added in this PR (field-level metadata at lines 104-149 and actions metadata at lines 152-181) lack test coverage. While other endpoints may have tests, these new features should have dedicated tests to ensure they work correctly and to prevent regressions.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
API Refactoring According to New Specification
Successfully completed the full refactoring of the ObjectQL server package to fully implement the new API specification documented in
docs/api/README.md.Implementation Status
POST /api/objectql) - Fully implemented with enhanced error handling/api/data/:object) - Fully implementedChanges Made
1. REST API Implementation
Created a complete REST-style API adapter (
packages/server/src/adapters/rest.ts) supporting:GET /api/data/:object- List records with query parameters (filter, fields, sort, top, skip, expand)GET /api/data/:object/:id- Get single recordPOST /api/data/:object- Create record (returns HTTP 201)PUT/PATCH /api/data/:object/:id- Update recordDELETE /api/data/:object/:id- Delete record2. Enhanced Error Handling
ErrorCodeenum with standardized codes:VALIDATION_ERROR,UNAUTHORIZED,FORBIDDEN,NOT_FOUND,CONFLICT,INTERNAL_ERROR,DATABASE_ERROR,RATE_LIMIT_EXCEEDED3. AI Context Support
ObjectQLRequesttype with optionalai_contextfieldAIContextinterface supporting intent, natural language descriptions, and use case tracking4. Metadata API Enhancements
GET /api/metadata/objects/:name/fields/:field- Get detailed field metadataGET /api/metadata/objects/:name/actions- List available actions5. Updated Example Application
examples/starters/express-apito demonstrate all API styles (JSON-RPC, REST, Metadata)6. Comprehensive Testing
packages/server/test/rest.test.ts)Available API Endpoints
POST /api/objectqlGET/POST/PUT/DELETE /api/data/:objectGET /api/metadata/objects- List all objectsGET /api/metadata/objects/:name- Get object schemaGET /api/metadata/objects/:name/fields/:field- Get field metadataGET /api/metadata/objects/:name/actions- List actionsGET /api/objectql/openapi.jsonTesting & Quality
See
API_REFACTORING_SUMMARY.mdfor detailed usage examples and implementation details.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.