Created src/ Folder Structure

[allenbakki]

Restructures the project under src/ (routes, controllers, db, auth, services, utils) and applies static review fixes. Includes a small refactor: new crudService.js used by the id() controller, with explicit 400 for missing ID and a getLocationHeader() helper. Route handlers live in src/routes/, auth in src/auth/ (renamed from config). Unused queryObjects and shebang removed from crudService; CLAUDE.md updated. Talend API demo verified; no breaking API changes.
What changed
Layout: app entry src/index.js; route handlers in src/routes/; controllers in src/controllers/; auth in src/auth/; DB in src/db/; service layer in src/services/crudService.js.
Home route: services/index.js → services/home.js → src/routes/home.js.
config → auth rename; route handlers moved from services to routes.
crudService: getIdById, getLocationHeader; removed unused queryObjects and shebang; JDoc updated.

In general, the issue is fixed by adding rate limiting middleware to the route (or router) that performs file system access. In an Express-based service, this is typically done using a well-known library such as express-rate-limit, configuring sensible limits (e.g., maximum number of requests per IP per time window), and then applying it via router.use(limiter) or router.get('/', limiter, handler).

For this specific file, the minimal, non-breaking fix is to: (1) import express-rate-limit, (2) configure a limiter instance (for example, 100 requests per 15 minutes per IP, aligning with the background example), and (3) apply that limiter middleware to the existing GET / route. This does not change the behavior of the route under normal traffic conditions, but it will start rejecting excessive requests with HTTP 429 responses. All changes will be made in src/services/index.js, adding the import and limiter configuration near the top of the file and then inserting the limiter as middleware on the router.get('/', ...) definition.

Concretely:

• Add import rateLimit from 'express-rate-limit' alongside the existing imports.
• Define a const limiter = rateLimit({ windowMs: 15 * 60 * 1000, max: 100 }) after the imports and before using the router.
• Change router.get('/', (req, res, next) => { ... }) to router.get('/', limiter, (req, res, next) => { ... }).

[github-advanced-security]

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

[thehabes]

Static Review Comments

Branch: feat-reshma-folder-structuring
Review Date: 2026-02-24
Reviewer: Claude Code assisted by Bryan - @thehabes

Bryan and Claude make mistakes. Verify all issues and suggestions. Avoid unnecessary scope creep.

Category	Issues Found
🔴 Critical	2
🟠 Major	2
🟡 Minor	1
🔵 Suggestions	2

---

Critical Issues 🔴

Issue 1: src/config/ is a complete orphan copy of src/auth/ — 193-line dead file with auth logic

File: src/config/index.js (new, 193 lines), src/config/__tests__/, src/config/__mocks__/
Category: Dead Code / Confusing Duplication

Problem:
The first commit created src/config/ as a copy of auth/. The fifth commit (61454db) renamed config/ → auth/ (by moving the original auth/ into src/auth/). But the src/config/ directory was never deleted — it still exists with identical copies of index.js, __tests__/token.test.txt, and __mocks__/index.txt.

src/config/index.js is 193 lines of authentication code (JWT validation, token generation, bot checking, READONLY middleware) that is never imported by anything. No file in the codebase references config/:

$ grep -r "from.*config/" src/ --include="*.js"
# (no results)

All 14 route files correctly import from ../auth/index.js.

Impact: This is a full duplicate of the auth module sitting in the tree. It will confuse future developers, could accidentally be imported, and bloats the repository. Since it contains auth logic (JWT secrets handling, token validation), having an unmonitored copy is also a minor security hygiene concern.

Suggested Fix:
Delete the entire src/config/ directory:

rm -rf src/config/

How to Verify:
After deletion, run grep -r "config/" src/ --include="*.js" to confirm nothing imports from it. Then run npm run runtest -- __tests__/routes_mounted.test.js to confirm no breakage.

---

Issue 2: src/services/ contains 10 duplicate route handler files plus 19 duplicate test files

File: src/services/ directory (entire contents except crudService.js)
Category: Dead Code / Confusing Duplication

Problem:
The sixth commit (ae0871c) moved route handlers from services/ to routes/. But the original src/services/ copies were never deleted. Every file below is byte-identical to its src/routes/ counterpart:

src/services/ file	Identical to src/routes/ file
api-routes.js	api-routes.js
db-controller.js	db-controller.js
history.js	history.js
id.js	id.js
index.js	home.js
query.js	query.js
search.js	search.js
since.js	since.js
static.js	static.js

Plus all 19 test files in src/services/__tests__/ are identical duplicates of src/routes/__tests__/.

The only unique file is src/services/crudService.js, which is legitimately imported by src/controllers/crud.js.

Impact:

• 28 duplicate files inflating the PR by ~1,500 added lines
• jest.config.js still has "**/services/*.js" in collectCoverageFrom (line 35), which means Jest is collecting coverage from the orphan src/services/ route handlers instead of the actual src/routes/ files
• The duplicate test files under src/services/__tests__/ will be discovered by Jest's default test patterns and run twice — once from src/routes/__tests__/ and once from src/services/__tests__/

Suggested Fix:

1. Delete everything in src/services/ except crudService.js:

# Remove duplicate route handlers
rm src/services/api-routes.js src/services/db-controller.js src/services/history.js \
   src/services/id.js src/services/index.js src/services/query.js \
   src/services/search.js src/services/since.js src/services/static.js

# Remove duplicate test directory
rm -rf src/services/__tests__/

2. Update jest.config.js coverage collection:

collectCoverageFrom: [
    "**/db-controller.js",
    "**/routes/*.js"
],

How to Verify:
After cleanup, ls src/services/ should only show crudService.js. Run npm run runtest -- __tests__/routes_mounted.test.js to confirm nothing breaks.

---

Major Issues 🟠

Issue 1: jest.config.js coverage collects from orphan services/ instead of active routes/

File: jest.config.js:35
Category: Configuration / Testing

Problem:
The PR changed the coverage glob from "**/routes/*.js" to "**/services/*.js". Since the active route handlers now live in src/routes/, this change means coverage is being collected from the orphan duplicates in src/services/ rather than the actual source files.

Current Code:

collectCoverageFrom: [
    //"**/*.js",
    "**/db-controller.js",
    "**/services/*.js"   // ← Points to orphan duplicates
],

Suggested Fix:

collectCoverageFrom: [
    //"**/*.js",
    "**/db-controller.js",
    "**/routes/*.js"
],

How to Verify:
Run any test file and check the coverage output to confirm it reports on src/routes/ files.

---

Issue 2: PR description does not reflect the actual scope of changes

Category: Process

Problem:
The PR description has been updated since the first review to mention the crudService refactor, which is good. However, it still doesn't mention the orphan files problem. The description says:

"Layout: app entry src/index.js; route handlers in src/routes/; controllers in src/controllers/; auth in src/auth/"

This is accurate for the intended structure, but the PR as-submitted also includes src/config/ (full auth duplicate) and src/services/ (full routes duplicate) which contradict the stated layout.

Anyone merging this PR trusting the description will unknowingly introduce 30+ orphan files.

Suggested Fix:
Either clean up the orphan files (recommended — see Critical Issues above), or update the description to note the leftover directories that need cleanup.

---

Minor Issues 🟡

Issue 1: CLAUDE.md still references old directory names in some places

File: .claude/CLAUDE.md:69
Category: Documentation Accuracy

Problem:
The controller listing still uses unqualified paths:

- `controllers/crud.js`: Core create, query, id operations
- `controllers/update.js`: PUT/PATCH update operations
- `controllers/delete.js`: Delete operations
...

The high-level architecture was updated to use src/ prefixed paths, but the controller bullet list omits the src/ prefix. While minor (the context makes it clear), it's inconsistent with the rest of the updated section.

---

Suggestions 🔵

Suggestion 1: Consider consolidating src/services/ to only contain actual service-layer files

After cleaning up the duplicates, src/services/ would contain only crudService.js. This is a good start for a service layer, but the directory name services/ plus a single file may cause confusion about where new services should go vs. where controllers go.

Consider either:

• Documenting the intended distinction between controllers/ and services/ in CLAUDE.md
• Or keeping crudService.js in controllers/ alongside crud.js since they're tightly coupled

Suggestion 2: The @author in crudService.js could include the original author

File: src/services/crudService.js:7

 * @author thehabes, cubap, RERUM team

This is better than the first version reviewed ("Refactored for separation of concerns"), but since this is extracted code from crud.js which was originally @author Claude Sonnet 4, cubap, thehabes, consider keeping consistency.