Wrong pagination params on cursor

[drochag]

I have the following Schema

    new querySchema({
      type: String,
      garment: String,
      brand: String,
      supplier: String,
      name: RegExp
    })

Haven't touched anything regarding pagination or limit, however after updating to 2.1.3 to include the cache problem, which I was having as well, this happens:

http://localhost:9000/branch-products?sort=-createdAt&page=1&limit=10
Mongoose: branchproducts.find({}, { skip: 0, limit: 10, sort: { createdAt: -1 }, fields: {} })

http://localhost:9000/branch-products?sort=-createdAt&page=2&limit=10
Mongoose: branchproducts.find({}, { skip: 30, limit: 10, sort: { createdAt: -1 }, fields: {} })

http://localhost:9000/branch-products?sort=-createdAt&page=3&limit=10
Mongoose: branchproducts.find({}, { skip: 60, limit: 10, sort: { createdAt: -1 }, fields: {} })

http://localhost:9000/branch-products?sort=-createdAt&page=4&limit=10
Mongoose: branchproducts.find({}, { skip: 90, limit: 10, sort: { createdAt: -1 }, fields: {} })

As if the limit was 30. Any ideas?

[chemitaxis]

Hi @danmmx

I have tested and this is the result I have:

request(app)
      .get('/tests?sort=-createdAt&page=12&limit=20')
      .query(new querymen.Schema({
        name: String
      }))
      .expect(200)
      .end((err, res) => {
        if (err) throw err
        t.equal(res.body.length, 0, 'should return empty array')
      })

tests.find({}, { skip: 220, limit: 20, sort: { createdAt: -1 }, fields: {} })

Limit default value is 30... Can you please check this? Thanks.

[drochag]

Yes, I understand the limit default is 30, however, sending a limit of 10 pagination should be skipping _this.param('limit').value() * (value - 1), right? And for a limit of 10 on page 2, skip should be 10 * 1 but instead it gets 30 as if limit was 30 (same for page 3, 4, etc...).

Here's how the req.query looks before passing through querymen:

URL: http://localhost:9000/branch-products?sort=-createdAt&page=1&limit=10
req.query { sort: '-createdAt', page: '1', limit: '10' }
<-- querymen middleware -->
cursor { skip: 0, limit: 10, sort: { createdAt: -1 } }
Mongoose: branchproducts.find({}, { skip: 0, limit: 10, sort: { createdAt: -1 }, fields: {} })

URL: http://localhost:9000/branch-products?sort=-createdAt&page=2&limit=10
req.query { sort: '-createdAt', page: '2', limit: '10' }
<-- querymen middleware -->
cursor { skip: 30, limit: 10, sort: { createdAt: -1 } }
Mongoose: branchproducts.find({}, { skip: 30, limit: 10, sort: { createdAt: -1 }, fields: {} })

URL:http://localhost:9000/branch-products?sort=-createdAt&page=3&limit=10
req.query { sort: '-createdAt', page: '3', limit: '10' }
<-- querymen middleware -->
cursor { skip: 60, limit: 10, sort: { createdAt: -1 } }
Mongoose: branchproducts.find({}, { skip: 60, limit: 10, sort: { createdAt: -1 }, fields: {} })

It looks like the limit gets ignored at some point when the skip parse happens.

@chemitaxis ^

[drochag]

Further info (debugging querymen/dist/querymen-schema.js) :

URL: /branch-products?sort=-createdAt&page=1&limit=10
req.query { sort: '-createdAt', page: '1', limit: '10' }
querymen-schema.js:269 - get |  this.params[name].value(): 30 |  name: limit // here limit is 30 ¿?
querymen-schema.js:138 - page#parse |  _this.param('limit').value(): 30 |  page value: 1 // here limit is 30 ¿?
querymen-schema.js:269 - get |  this.params[name].value(): 30 |  name: limit // here limit is 30 ¿?
querymen-schema.js:147 - limit#parse |  limit value: 10 // here limit is 10 - this is ok
cursor { skip: 0, limit: 10, sort: { createdAt: -1 } }
Mongoose: branchproducts.find({}, { skip: 0, limit: 10, sort: { createdAt: -1 }, fields: {} })

URL: /branch-products?sort=-createdAt&page=2&limit=10
req.query { sort: '-createdAt', page: '2', limit: '10' }
querymen-schema.js:269 - get |  this.params[name].value(): 30 |  name: limit // here limit is 30 ¿?
querymen-schema.js:138 - page#parse |  _this.param('limit').value(): 30 |  page value: 2 // here limit is 30 ¿?
querymen-schema.js:269 - get |  this.params[name].value(): 30 |  name: limit // here limit is 30 ¿?
querymen-schema.js:147 - limit#parse |  limit value: 10 // here limit is 10 - this is ok
cursor { skip: 30, limit: 10, sort: { createdAt: -1 } }
Mongoose: branchproducts.find({}, { skip: 30, limit: 10, sort: { createdAt: -1 }, fields: {} })

URL: /branch-products?sort=-createdAt&page=3&limit=10
req.query { sort: '-createdAt', page: '3', limit: '10' }
querymen-schema.js:269 - get |  this.params[name].value(): 30 |  name: limit // here limit is 30 ¿?
querymen-schema.js:138 - page#parse |  _this.param('limit').value(): 30 |  page value: 3 // here limit is 30 ¿?
querymen-schema.js:269 - get |  this.params[name].value(): 30 |  name: limit // here limit is 30 ¿?
querymen-schema.js:147 - limit#parse |  limit value: 10 // here limit is 10 - this is ok
cursor { skip: 60, limit: 10, sort: { createdAt: -1 } }
Mongoose: branchproducts.find({}, { skip: 60, limit: 10, sort: { createdAt: -1 }, fields: {} })

Haven't looked deeply into the querymen-schema.js code, but it clearly looks that the get is somehow getting the default values instead of the current params.

[chemitaxis]

Ok, I will look again... but the test is working 🤔

Do you know how to pass the test? Clone and run test?

And we are using querymen in production without problems...

[chemitaxis]

I have created a project test, and all is working fine... can you check it please? Thanks.

https://github.com/chemitaxis/test_rest

[drochag]

I don't see any test regarding pagination on https://github.com/chemitaxis/test_rest
I'll try to make a PR reproducing the problems, but I've found that the problem happens after using a custom Schema.

import { middleware as query, Schema as querySchema } from 'querymen'

// this works
router.get('/',
  token({ required: true, role: 'branch' }),
  query(),
  index)

// this doesn't
router.get('/',
  token({ required: true, role: 'branch' }),
  query(
    new querySchema({
      branchCollection: String,
      composition: String,
      brand: String,
      code: RegExp
    })
  ),
  index)

[chemitaxis]

Hi @danmmx ok, I can reproduce it right now...

If you create the querySchema, inside the middleware, it fails, if you do that:

const customSchema = new querySchema({
  branchCollection: String,
  composition: String,
  brand: String,
  code: RegExp
})

// this works too
router.get('/',
  token({ customSchema }),
  query(),
  index)

I will check it... until that, you can use previous version... sorry for the inconvenience. PR is welcome if you find an error ;)

[drochag]

@chemitaxis can you explain further this comments? not sure I can understand them, and confirming after that PR this bug was introduced.

[drochag]

@diegohaz can you take a look at this one?
You can see the PR #51 to check the test that is failing, the bug was introduced after #49

however I'm not able to solve #30 without triggering this bug

[chemitaxis]

I will make a new PR tomorrow removing the previous changes... it's worth the solution applyed that the problem in my opinion

[diegohaz]

I think we should revert the last change since it introduced another bug and try to solve #30 in another way, trying to pass the failing tests on #51 and #52. What do you think?

[drochag]

Sounds nice … however both things (with cache problem and pagination problem) are a big issue on my app. Have spent several hours checking the bug, my only guess is it is related to rich-param or somehow caching the previous things.

Haven't been able to find out a solution for one without introducing the 2nd one 😢