fix: address PR review feedback on populate robustness, docs, and tests

Agent-Logs-Url: https://github.com/geturbackend/urBackend/sessions/2bf89d6d-c2ca-425b-ae5d-71ce054d8edb

Co-authored-by: yash-pouranik <172860064+yash-pouranik@users.noreply.github.com>

Author: Copilot

apps/public-api/src/__tests__/data.controller.read.test.js

@@ -5,6 +5,7 @@ const mockAnd = jest.fn();
 const mockFindOne = jest.fn();
 const mockQueryLean = jest.fn().mockResolvedValue([]);
 const mockPopulate = jest.fn();
+const mockEnginePopulate = jest.fn();
 
 // mockAnd returns an object with .lean() so features.query.lean() works after .and()
 mockAnd.mockReturnValue({ lean: mockQueryLean });
@@ -14,7 +15,7 @@ const mockQueryEngine = jest.fn((query) => {
         query,
         filter() { return engine; },
         sort() { return engine; },
-        populate() { return engine; },
+        populate() { mockEnginePopulate(...arguments); return engine; },
         paginate() { return engine; },
     };
     return engine;
@@ -38,13 +39,14 @@ jest.mock('@urbackend/common', () => ({
         },
         findOne: (...args) => {
             mockFindOne(...args);
-            return { 
+            const chainable = {
                 populate: (...pArgs) => {
                     mockPopulate(...pArgs);
-                    return { lean: jest.fn().mockResolvedValue({ _id: 'doc_1' }) };
+                    return chainable;
                 },
-                lean: jest.fn().mockResolvedValue({ _id: 'doc_1' }) 
+                lean: jest.fn().mockResolvedValue({ _id: 'doc_1' }),
             };
+            return chainable;
         },
     })),
     QueryEngine: mockQueryEngine,
@@ -89,6 +91,7 @@ function makeRes() {
 describe('data.controller read RLS filters', () => {
     beforeEach(() => {
         jest.clearAllMocks();
+        mockEnginePopulate.mockClear();
     });
 
     test('getAllData applies rlsFilter to find()', async () => {
@@ -117,16 +120,28 @@ describe('data.controller read RLS filters', () => {
         expect(res.json).toHaveBeenCalled();
     });
 
-    test('getAllData passes populate param to QueryEngine', async () => {
+    test('getAllData calls engine.populate() when populate param is provided', async () => {
         const req = makeReq({ query: { populate: 'author,comments' } });
         const res = makeRes();
 
-        // The real QueryEngine (not mocked) would call .populate() on the query
-        // But since we mock QueryEngine, we just check if populate() was part of the chain
-        // In this specific test file, mockQueryEngine is what's used.
         await getAllData(req, res);
-        
+
         expect(mockQueryEngine).toHaveBeenCalled();
+        expect(mockEnginePopulate).toHaveBeenCalled();
+    });
+
+    test('getAllData does not forward populate/expand to Mongo filter', async () => {
+        const req = makeReq({ query: { populate: 'author', expand: 'category', title: 'hello' } });
+        const res = makeRes();
+
+        await getAllData(req, res);
+
+        // mockFind is called with the raw Mongoose query (no args for Model.find())
+        // The real filter exclusion is tested via the QueryEngine directly,
+        // but we confirm find() was invoked and the request did not error out.
+        expect(mockFind).toHaveBeenCalledWith();
+        expect(res.json).toHaveBeenCalled();
+        expect(res.status).not.toHaveBeenCalledWith(500);
     });
 
     test('getSingleDoc calls populate on the query', async () => {
@@ -138,4 +153,16 @@ describe('data.controller read RLS filters', () => {
         expect(mockPopulate).toHaveBeenCalledWith('author');
         expect(res.json).toHaveBeenCalled();
     });
+
+    test('getSingleDoc handles array-format populate param without crashing', async () => {
+        const req = makeReq({ query: { populate: ['author', 'category'] } });
+        const res = makeRes();
+
+        await getSingleDoc(req, res);
+
+        expect(mockPopulate).toHaveBeenCalledWith('author');
+        expect(mockPopulate).toHaveBeenCalledWith('category');
+        expect(res.json).toHaveBeenCalled();
+        expect(res.status).not.toHaveBeenCalledWith(500);
+    });
 });

apps/public-api/src/controllers/data.controller.js

@@ -162,8 +162,9 @@ module.exports.getSingleDoc = async (req, res) => {
     let query = Model.findOne({ $and: [{ _id: id }, baseFilter] });
 
     // Handle population for single doc
-    const populateParam = req.query.populate || req.query.expand;
-    if (populateParam) {
+    const rawPopulateParam = req.query.populate || req.query.expand;
+    if (rawPopulateParam) {
+      const populateParam = Array.isArray(rawPopulateParam) ? rawPopulateParam.join(',') : String(rawPopulateParam);
       const fields = populateParam.split(',').map(f => f.trim()).filter(Boolean);
       fields.forEach(f => {
         query = query.populate(f);

docs/database.md

@@ -56,6 +56,7 @@ curl "https://api.ub.bitbros.in/api/data/posts" ^
 ### Fetch Single Document
 **Endpoint**: `GET /api/data/:collectionName/:id`
 
+```bash
 curl "https://api.ub.bitbros.in/api/data/posts/64fd1234abcd5678ef901234" ^
   -H "x-api-key: pk_live_xxx"
 ```
@@ -69,8 +70,8 @@ You can refine your `GET` requests using the following query parameters:
 | `populate` | `String` | `?populate=author,category` | Expand `Ref` fields into full objects. |
 | `expand` | `String` | `?expand=author` | Alias for `populate`. |
 | `sort` | `String` | `?sort=createdAt:desc` | Sort results by field and order. |
-| `limit` | `Number` | `?limit=25` | Number of documents to return (max 100). |
-| `page` | `Number` | `?page=2` | Pagination offset. |
+| `limit` | `Number` | `?limit=25` | Number of documents to return (default: 100, max: 100). |
+| `page` | `Number` | `?page=2` | 1-indexed page number for pagination. |
 
 ### Relational Data (Population) 🔗
 
@@ -87,6 +88,8 @@ curl "https://api.ub.bitbros.in/api/data/posts?populate=author,category" ^
 > [!TIP]
 > This solves the N+1 query problem, allowing you to fetch complex relational data in a single request.
 
+## 3. Update a Document
+
 **Endpoint**: `PUT /api/data/:collectionName/:id`
 
 urBackend uses `$set` logic, meaning you only need to send the fields you want to change.

packages/common/src/utils/queryEngine.js

@@ -6,7 +6,7 @@ class QueryEngine {
 
     filter() {
         const queryObj = { ...this.queryString };
-        const excludedFields = ['page', 'sort', 'limit', 'fields'];
+        const excludedFields = ['page', 'sort', 'limit', 'fields', 'populate', 'expand'];
         excludedFields.forEach(el => delete queryObj[el]);
 
         const mongoQuery = {};
@@ -53,7 +53,7 @@ class QueryEngine {
 
     paginate() {
         const page = parseInt(this.queryString.page, 10) || 1;
-        const limit = parseInt(this.queryString.limit, 10) || 100;
+        const limit = Math.min(parseInt(this.queryString.limit, 10) || 100, 100);
         const skip = (page - 1) * limit;
 
         this.query = this.query.skip(skip).limit(limit);
@@ -62,13 +62,13 @@ class QueryEngine {
     }
 
     populate() {
-        const populateParam = this.queryString.populate || this.queryString.expand;
-        if (populateParam) {
-            const fields = populateParam.split(',').map(f => f.trim()).filter(Boolean);
-            fields.forEach(f => {
-                this.query = this.query.populate(f);
-            });
-        }
+        const rawParam = this.queryString.populate || this.queryString.expand;
+        if (!rawParam) return this;
+        const populateParam = Array.isArray(rawParam) ? rawParam.join(',') : String(rawParam);
+        const fields = populateParam.split(',').map(f => f.trim()).filter(Boolean);
+        fields.forEach(f => {
+            this.query = this.query.populate(f);
+        });
         return this;
     }
 }