Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Every REST API request with a date query param returns an empty result #12660

Closed
benwick opened this issue Nov 19, 2018 · 17 comments
Closed

Every REST API request with a date query param returns an empty result #12660

benwick opened this issue Nov 19, 2018 · 17 comments

Comments

@benwick
Copy link

benwick commented Nov 19, 2018

Rocket API Version: 0.71.1

Expected behavior
When I use an API endpoint with the query parameter information, I want to search for certain date fields. For example: Find all direct messages that were updated since the first of november. Or find all users that were created since the beginning of the year.

Actual behavior
All of the API endpoints will return an empty result, when I search for these fields. Even if they shouldn't.

Reproduction steps
Go on a webserver with PHP support and installed Composer of your liking.

  1. mkdir rocket-test && cd rocket-test && composer require "fab1en/rocket-chat-rest-client"
  2. touch index.php
  3. Insert into index.php and replace SERVER, ADMIN, PASSWORD & SOME_REAL_ID with your data:
<?php

require __DIR__ . '/vendor/autoload.php';

use Httpful\Request;

define('REST_API_ROOT', '/api/v1/');
define('ROCKET_CHAT_INSTANCE', 'SERVER');

$admin = new \RocketChat\User('ADMIN', 'PASSWORD');
echo $admin->version(); echo "\n";

if ($admin->login()) {
    $response = Request::get($admin->api . 'im.list.everyone')->send();
    var_dump($response->body);
    // -> non empty result

    $response = Request::get($admin->api . 'im.list.everyone?query={"lm":{"$gte":"2018-11-01T00:00:00.000Z"}}')->send();
    var_dump($response->body);
    // -> empty result

    $response = Request::get($admin->api . 'im.list.everyone?query={"_updatedAt":{"$gte":"2018-11-01T00:00:00.000Z"}}')->send();
    var_dump($response->body);
    // -> empty result

    $response = Request::get($admin->api . 'users.list?query={"createdAt":{"$gte":"2018-01-01T00:00:00.000Z"}}')->send();
    var_dump($response->body);
    // -> empty result

    $response = Request::get($admin->api . 'users.list?query={"utcOffset":{"$gt":1}}')->send();
    var_dump($response->body);
    // -> non empty result

    $response = Request::get($admin->api . 'im.list.everyone?query={"_id":"SOME_REAL_ID"}')->send();
    var_dump($response->body);
    // -> non empty result
  1. Call the index.php

All calls to the API with date fields will stay empty. I know for sure, that some months ago requests of this kind were still working...

@reetp
Copy link

reetp commented Nov 19, 2018

What if you do this without the query? What data is returned?

im.list.everyone

Have you tried it without the query just using say a simple curl query to test ?

@benwick
Copy link
Author

benwick commented Nov 20, 2018

Of course it does function without the query params. I get 50 results back. I have updated the test example. The API is working, the query params are working. Only when you try to query date fields, the query returns an empty result.

@reetp
Copy link

reetp commented Nov 20, 2018

There is no 'of course' when debugging.

Have you checked the rocket logs to see what they say? There will presumably be some error message in there as it makes the call.

@Maqsyo
Copy link

Maqsyo commented Nov 20, 2018

Backend -> View Logs
image
No Error occurs

Log-Level: 2 - Errors, Information and Debug

@reetp
Copy link

reetp commented Nov 20, 2018

OK, so the official answer to this is....

The only SUPPORTED way to talk to Rocket is via the JScript API.

The PHP API is written by a 3rd party developer and you need to contact him for support. The Rocket chat team have no control over that code.

If you can persuade the author to speak to the Rocket chat team then they will be happy to work with him on this.

FYI it looks like the issue is on the PHP side converting the Rocket timestamp back to whatever it is you need.

Going to add a label wontfix and close

@reetp
Copy link

reetp commented Nov 20, 2018

@rocket-cat label add wontfix

@rocket-cat rocket-cat bot added the wontfix label Nov 20, 2018
@reetp
Copy link

reetp commented Nov 20, 2018

@rocket-cat close

@rocket-cat rocket-cat bot closed this as completed Nov 20, 2018
@benwick
Copy link
Author

benwick commented Nov 20, 2018

This is blatantly wrong.

The only SUPPORTED way to talk to Rocket is via the JScript API.

No, the REST API is one of the official documented ways to talk to Rocket. This API is not language specific. It just receives HTTP requests. How the requests are created does not play any role. I have used the PHP wrapper that is mentioned in the documentation.

FYI it looks like the issue is on the PHP side converting the Rocket timestamp back to whatever it is you need.

No, it cleary does not look like that. The REST API can only be queried with json strings. As documented here. So the only two possibilities to query a date field is with an ISO string or a number / timestamp. Both ways do not work.

My usage of PHP was only an example to recreate a fast test case for you. The official Rocket.Chat SDK has exactly the same issues and is broken. Please reopen this task. I have tried everything and it is not working...

@reetp
Copy link

reetp commented Nov 20, 2018

You are not being 'mocked' as per your foro comment. I am merely repeating what I have been told from higher up the food chain as I am no expert on this (so I bothered to go ask). Don't beat up the messenger :-)

Note - you were using PHP code from a third party repo which is not supported here:
https://github.com/fab1en/rocket-chat-rest-client

From a debugging PoV there is no point in using its output if you are trying to illustrate a specific point.

For reference I also note your for post here:
https://forums.rocket.chat/t/rest-api-query-with-date-fields-isnt-working-anymore/2119

One of the devs told you:

I also tried to filter with your query directly in mongodb, and did not return any records, what is the content of your oldTreshold variable?

And you later made this comment:

The only idea I have is that the query parameter for any kind of date query has to be a Date instance. But at that point the _updatedAt query param is a string, because it was read from the get parameters of the API request.

So you really need to demonstrate your issue fully. Show output etc etc etc. Try querying the DB directly and show output. Just saying "non empty result" or "empty result" tells the devs nothing - it could be for a myriad of reasons. Normal debugging procedures apply here.

The best place to continue the discussion is in the foros. If there is an actual issue in Rocket a dev will reopen this.

Thamk you.

These issues refer:
https://github.com/RocketChat/docs/issues/870
#11657

@benwick
Copy link
Author

benwick commented Nov 20, 2018

Why should I query the DB directly? That is not my job... I just try to use the REST API here. With correctly used query params. That should work and it once worked. Everything else is really going very deep and is out of my hands to be true...

@reetp
Copy link

reetp commented Nov 20, 2018

Because that is the sort of thing good devs do when they are debugging stuff and trying to prove their point. Go back to basics and test step by step.

It's either that, or have a lot of patience hoping for an answer, or you can pay for support, sit back and wait for someone else to try and see what is going on and whether it really is an issue or not.

@benwick
Copy link
Author

benwick commented Nov 20, 2018

Okay, so I have queried the mongo shell directly and I can see a problem here:

db.users.find({"createdAt": {"$gt": "2016-01-01T00:00:00.000Z"}}) is returning an empty result.
db.users.find({"createdAt": {"$gt": ISODate("2016-01-01T00:00:00.000Z")}}) is working and returning users.

The REST API is called with the get param query, which is a JSON string. The JSON string is deserialized and all query parameters are read from this object. See here. Every date field query parameter is and stays a string or a number. Of course it can't be of any complex type like ISODate because of the JSON serialization. But the Mongo DB exactly needs that. So my conclusion stays the same: Every REST API request with a date query param returns an empty result.

One possible solution I can fathom is to have a collection of field names which are of the type date. And in parseJsonQuery function all query param fields are to be cross referenced with said collection and if there is a match the ISO string is to be wrapped in a call of ISODate(dateFieldQueryValue) ... so the date field value is coerced to the right type to work with MongoDB.

Or we use the string: {"$gt": "ISODate(\"2013-04-02T10:37:21.529Z\")}" and search for it with a regular expression like here.

Or we could use a regular expression to test every search string for the ISO format. But that is pretty complex...

@reetp
Copy link

reetp commented Nov 20, 2018

Now, that's what I call good work and will get you listened to :-)

I have asked for someone to take a look. Have some patience please....

@Maqsyo
Copy link

Maqsyo commented Nov 21, 2018

So it isn't closed or a wontfix anymore, isn't it?

@Sing-Li
Copy link
Member

Sing-Li commented Nov 21, 2018

@Maqsyo @benwick This is a rather niche use case of the REST-APIs. The impact to the community appears to be quite small (from response of your forum post so far). Allowing ad-hoc global date based query param extension may have performance implications.

Reference: your forum post

@rodrigok @MarcosSpessatto We may need to either :

(1) document clearly that we support only simple string and numeric query-params

OR

(2) actually implement the escape for specific functions that can convert the simple types to date

@benwick
Copy link
Author

benwick commented Nov 21, 2018

Thanks for reopening the task. To be true: Obviously there does not seem to be a wide spread community use case. But I have to say, that it is not such a far fetched use case to have time based requests to the REST API... it seems pretty useful and apt to me.

@MarcosSpessatto
Copy link
Contributor

MarcosSpessatto commented Nov 23, 2018

@benwick I opened a PR that corrects the problem, in fact when the search was done by Date, it didn't work, because JSON does not support Date, so the query was being executed comparing with a string, which resulted in empty results.
I changed JSON by EJSON which supports fields of type Date, and also updated the documentation.
Thanks for letting us know and help us debug. 👍
Thanks @Sing-Li for the tips. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants