RetroShare / RSNewWebUI

30 stars 20 forks source link

File Search : Response from the `turtleSearchRequest` is not correctly formatted #73

Closed zelfroster closed 1 year ago

zelfroster commented 1 year ago

The response from the turtleSearchRequest is not correctly formatted as JSON, so that's why handleDeserialize throws error when parsing in rswebui.js. 1682775057_Apr29_19:00:57

So, The correct format for response should be like this. An object which will have its first item as a retval: <boolean>. And the second item which will be an array of all the result files "results": [{},{}]

{
  "retval": true,
  "results": [{ "fSize": {}, "fHash": "", "fName": "", "fWeight": 0.0, "fSnippet": ""}, { }, { }]
}

The real response might look something like this.

And after formatting, It will look something like this. 1682785511_Apr29_21:55:11

zelfroster commented 1 year ago

@csoler Have a look at it.

rottencandy commented 1 year ago

What format are you getting currently?

Turtle search request returns as a stream so I don't think all the data in results list will be available immediately.

zelfroster commented 1 year ago

What format are you getting currently?

Turtle search request returns as a stream so I don't think all the data in results list will be available immediately.

Yes, I actually didn't know that It was coming as a stream, and Cyril told me about that when we discussed. So, currently, I am able to get the data as chunks of stream as soon as they are fetched from the server.

1683035779_May02_19:26:19

But, Since TurtleSearchRequest doesn't send an end of stream marker, there is no way to know when the stream ends. So, I am thinking of setting a time duration using setTimeout to manually stop the connection.

1683035865_May02_19:27:45 1683035884_May02_19:28:04 1683035897_May02_19:28:17 1683036439_May02_19:37:19

Please suggest some better ways, I could only think of this after doing some research and help from internet.

csoler commented 1 year ago

Setting a timeout is ok. I do not think that there is another way because RS keeps accepting results coming for each specific search request. I'm not sure how JS works, but ideally you should keep a list of search request IDs and fill them with incoming results (suppose for instance that you search twice, then you will receive results for both searches alternatively), and update them on display. When the search is "closed" (from your JS interface) then just ignore the related incoming results. Is that possible in JS?

csoler commented 1 year ago

also, one may issue a search from the Qt UI, so the webUI will also receive results for some unknown searches. I suppose that you can afford to ignore these.

zelfroster commented 1 year ago

(suppose for instance that you search twice, then you will receive results for both searches alternatively), and update them on display. When the search is "closed" (from your JS interface) then just ignore the related incoming results. Is that possible in JS?

Yes, As soon as someone initiates a new search request, the latest search results will only be shown. And If we move to a different page, then we can use onunload method to abort the ongoing search request.

also, one may issue a search from the Qt UI, so the webUI will also receive results for some unknown searches. I suppose that you can afford to ignore these.

I have tested it, and Qt UI results should not be shown in JS request.

csoler commented 1 year ago

Yes, As soon as someone creates a new search request, the latest search results will only be shown. And If we move to a different page, then we can use onunload method to abort the ongoing search request.

That is not what I was saying: when you change pages (in other words, look at the results of a different search) you should keep the previous search active, since more results are likely to come. When you close that page however, you can remove the search id from the list and ignore additional results that will come for this search.

zelfroster commented 1 year ago

Yes, As soon as someone creates a new search request, the latest search results will only be shown. And If we move to a different page, then we can use onunload method to abort the ongoing search request.

That is not what I was saying: when you change pages (in other words, look at the results of a different search) you should keep the previous search active, since more results are likely to come. When you close that page however, you can remove the search id from the list and ignore additional results that will come for this search.

Ohh, So If I understood correctly, You are saying that I should keep the results of all the search requests stored, So that It can be shown just like the Qt UI. Yes, I think It can be done.

Edit: I actually didn't try all the features of search, Now I see a bunch of features in the search UI. And they can be implemented.

One more thing is that there are duplicate search requests and their results are stored in the Qt UI, So, Can we just not create a new one and only update the results of the previous search request if it already exists?

csoler commented 1 year ago

each search request is unique since it is represented internally by its 32bits ID. The search strings may be the same for two search requests, but these requests should be understood to be different since they have different IDs.

zelfroster commented 1 year ago

each search request is unique since it is represented internally by its 32bits ID. The search strings may be the same for two search requests, but these requests should be understood to be different since they have different IDs.

Then, I would have to manually give them an ID. Either way, It can be done.

csoler commented 1 year ago

Normally the ID should be returned by turtleSearchRequest(). It is the only way you can fit further results with that request. [edit] actually it doesn't. I will change the function tonight so that it returns an ID instead of a bool, as other turtleSearch methods do. I'll keep you posted.

zelfroster commented 1 year ago

Normally the ID should be returned by turtleSearchRequest(). It is the only way you can fit further results with that request. [edit] actually it doesn't. I will change the function tonight so that it returns an ID instead of a bool, as other turtleSearch methods do. I'll keep you posted.

Ohh, Thanks for this. 👍🏼