Turistforeningen / node-mongo-querystring

Query builder for accepting URL query parameters into your MongoDB queries. Safe and feature rich. Supports most of MongoDB's query operators such as $eq, $gt, $lt, $ne, $in, $nin, $exists, $regex, geospatial queries such as bbox and near, as well as your own custom query business logic!
MIT License
100 stars 31 forks source link

Unable to filter certain strings and decimal values #23

Closed wsfuller closed 8 years ago

wsfuller commented 8 years ago

I've pulled down the sample code to test out the filtering for a monitor resolution api. The only change I've made to the provided sample code in examples/app.js is the qs variable

app.js

...
var qs = new MongoQS({
  custom: {
    //bbox: 'geojson',
    //near: 'geojson',
    exists: 'hdmi',
    equal: 'resolution'
  }
});
...

Database:

[
  {
    "series": "meh-monitor",
    "hdmi": false,
    "hdmi_version": "",
    "vga": true,
    "resolution": "1366x768"
  },
  {
    "series": "ok-monitor",
    "hdmi": true,
    "hdmi_version": "1.0",
    "vga": true,
    "resolution": "1920x1080"  
   },
  {
    "series": "good-monitor",
    "hdmi": true,
    "hdmi_version": "1.2",
    "vga": false,
    "resolution": "2560x1440"
  }  
]

Working Sample URLs:

Non working sample URLs:

Starefossen commented 8 years ago

@wsfuller and thank you for reporting this in such detail!

Put this configuration in your app.js and both of these non-working URLs should start working:

…
var qs = new MongoQS({
  …
  string: {
    toNumber: false
  }
});
…

This disables string to number parsing which is creating problems for you in both of these cases.

wsfuller commented 8 years ago

@Starefossen thanks for the response, unfortunately this isn't working as expected with the suggested change.

app.js

var qs = new MongoQS({
  custom: {
    exists: 'hdmi',
    equal: 'resolution'
  },
  string: {
    toNumber: false
  }
});

url

localhost:3000/api/places?resolution=2560x1440

data

 {
    "series": "good-monitor",
    "hdmi": true,
    "hdmi_version": "1.2",
    "vga": false,
    "resolution": "2560x1440"
  }

I am running a console.log on the query:

app.js

...
  // Parse the request query parameters
  var query = qs.parse(req.query);
  console.log(query);
...

returns { resolution: 2560 }

So for some reason at the "x" the req.query stops. As well still not able to query ...?hdmi_version: 1.2

Starefossen commented 8 years ago

Could you try updating to at least v2.0.0 as the toNumber option was introduced in that version.

wsfuller commented 8 years ago

Apologies, didn't see the pull request until right after I posted the comment

Starefossen commented 8 years ago

No worries, I hope this solves your problems 😄

wsfuller commented 8 years ago

So I pulled down v3.0.0 and added in string:{ toNumber: false}. After trying localhost:3000/api/places?resolution=2560x1440 was still not returning expected results. So looking at the pull request made 4 days ago I just manually added in your fix on line 192

fix line #192

...
} else if (this.string.toNumber && !isNaN(parseInt(string, 10)) && 
(+string - +string + 1) >= 0) {
...

That solves the resolution issue and localhost:3000/api/places?resolution=2560x1440 is working as expected.

After removing the string property of hdmi_version: 1.2 and making it an integer that is working as expected now as well.

New to databases and JSON and didn't realize that having a decimal integer was acceptable, my thought was having a dot would make it invalid for some reason.

Thanks for your help :+1:

Starefossen commented 8 years ago

That patch from #24 has landed in v3.1.0