Conversation
Introduce cloneObject in utils.js that uses globalThis.structuredClone when available and falls back to JSON.parse(JSON.stringify(...)). Replace direct JSON cloning in configureRerumOptions for configuredObject and received_options with cloneObject, and export cloneObject. This centralizes cloning logic and allows better deep-clone behavior when structuredClone is supported.
Introduce MAX_QUERY_LIMIT and MAX_QUERY_SKIP and add clampNonNegativeInt and getPagination to safely parse and clamp limit/skip query params (with sensible defaults and env overrides). Replace ad-hoc deep copies (JSON.parse(JSON.stringify(...))) with utils.cloneObject in idNegotiation and getAllVersions to avoid issues with prototype loss and improve clarity. Export getPagination for reuse.
Replace repeated JSON.parse(JSON.stringify(...)) deep-clones with utils.cloneObject across controllers (crud, patchSet, patchUnset, patchUpdate, putUpdate, gog) and add/get getPagination to standardize parsing of limit/skip in search, gog and crud. Adjust imports accordingly to centralize cloning and pagination logic, improving readability and consistency.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Replace numerous JSON.parse(JSON.stringify(...)) and utils.cloneObject calls with native structuredClone across controllers and utils for safer, more reliable deep copies. Remove the now-unused cloneObject helper from utils.js and update configureRerumOptions to use structuredClone. Also tweak clampNonNegativeInt to treat non-positive values (<= 0) as fallback, switch history HEAD handling to res.status(200).end(), and add safeBody usage in delete to avoid re-parsing request body.
Replace many end-to-end route tests with it.todo placeholders to temporarily stub numerous route test cases (files under routes/__tests__ and __tests__/routes_mounted.test.js). Update routes_mounted.test.js to safely access Express router stack via app._router?.stack ?? [] for compatibility. Resolve a merge/conflict in controllers/delete.js by removing logic that attempted to read an id from the request body and enforce that DELETE requires the id in the URL (returns a 400 if missing).
Replace placeholder todos with active integration tests that probe mounted routes via supertest. Removed the unused api_routes import and the routeExists helper; tests now perform actual HTTP requests against app for top-level paths (/v1, /client/register, /v1/id/:id, /v1/since/:id, /v1/history/:id) and for /v1/api/* endpoints using appropriate methods and content types, asserting endpoints are mounted (not 404) or return expected 404 for unknown IDs. This validates routing mounts across multiple HTTP verbs instead of relying on stack inspection.
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
2 participants
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.
fixes #251
This pull request refactors several controller files to improve code maintainability and consistency. The main changes involve replacing manual object cloning with a utility function and standardizing pagination logic across multiple endpoints. These updates help reduce code duplication, make the codebase easier to maintain, and ensure more robust handling of request parameters.
Key changes include:
Object Cloning Consistency:
JSON.parse(JSON.stringify(...))used for deep cloning objects with the centralizedutils.cloneObjectfunction in CRUD, patch, and update controller files. This change improves code clarity and avoids potential issues with JSON serialization. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18]Pagination Handling Standardization:
getPaginationutility function to extractlimitandskipparameters with default values, replacing repeated manual parsing logic in CRUD, search, history, and Gallery of Glosses controller endpoints. This ensures consistent pagination behavior and reduces code duplication. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]Let me know if you have questions about how to use the new utility functions or why these refactors were made!