Add and remove highlights, plus some fixes

[franck-eyraud]

Implements

47 #52

Fixes

58 #60

[benel]

Looks great. I will test it.

[benel]

I've got a problem when I'm authenticated on the highlighted page:

I'm not sure about the problem (maybe the credentials are forwarded from the page to the scripts?).

You can test it on /text/IF14/b61bc2c460f4a9f60953756797a81d88 with your personal Hypertopic account.

[benel]

I understand. The bug is because one of the request (the one to Cassandre) got a 401. Maybe the process should continue when one of the server is successful.

[franck-eyraud]

The bug is because one of the request (the one to Cassandre) got a 401. Maybe the process should continue when one of the server is successful.

I'll have a look. Do you think that this problem was introduced by these changes, or already existed ?

[benel]

Do you think that this problem was introduced by these changes, or already existed ?

I'm not sure... Maybe I should handle the errors differently in Hypertopic node library. I use Promise.all:

https://github.com/Hypertopic/lib-node/blob/master/src/index.js#L16

[benel]

Maybe we could replace the faulty line in Hypertopic node lib getView:

 return Promise.all(uris.map(_get))

with:

 return Promise.all(uris.map(_safeGet))

where _safeGet return {rows:[]} on HTTP errors.

[benel]

Hmmm. In fact _get is only used in getView and it doesn't throw any exception. More tests have to be done.

[benel]

In fact, the problem is not the 401 status itself but the body (Unauthorized) that is not valid JSON. I'll try to fix this.

[benel]

OK. Seems to work. I will publish an updated version of the library.

[benel]

The following code:

const hypertopic = require('hypertopic');

var db = hypertopic([
  "http://argos2.hypertopic.org", 
  "http://cassandre.hypertopic.org"
]);

var _log = function _log(x) {
  return console.log(JSON.stringify(x, null, 2));
};

db.getView('/item/?resource=http://cassandre.hypertopic.org/text/IF14/b61bc2c460f4a9f60953756797a81d88')
  .then(_log);

displays (with the new 3.1.5 version of Hypertopic/lib-node):

{
  "http://cassandre.hypertopic.org/text/IF14/b61bc2c460f4a9f60953756797a81d88": {
    "item": [
      {
        "corpus": "b61bc2c460f4a9f60953756797c36361",
        "id": "b61bc2c460f4a9f60953756797c36820"
      }
    ]
  }
}

[benel]

Here is the updated part of the library: https://github.com/Hypertopic/lib-node/commit/d9ca553341d9ef3a9610dbfcf39962173a123923.

[franck-eyraud]

Thank you for this analysis. This probably handles the issue. However, what is strange is that I tested this page without upgrading the Hypertopic library (with both argos and argos.test servers in Lasuli configuration) but I never got this error. Were you getting it when loading the highlights ?

However, if one is not identified in the argos server, then any modification is currently silently ignored (except the 401 unauthorized displayed in the web browser console), and there it would be probably nice to display some message. Do you have some idea where ?

[benel]

Were you getting it when loading the highlights ?

As I wrote earlier, it is on password protected pages in Cassandre. The exception occurs because API URI containing the same UUID are also password protected.

If you want to test it yourself (before and after upgrading Hypertopic/lib-node), use the URI mentioned in the thread (with your own Hypertopic LDAP login and password, and just ask me for a reset if you don't remember it).

However, if one is not identified in the argos server, then any modification is currently silently ignored (except the 401 unauthorized displayed in the web browser console), and there it would be probably nice to display some message. Do you have some idea where ?

There is a difference indeed between:

• exceptions on getView (which could be highly distributed similarly to a P2P network), that should remain silent,
• exceptions on put and delete (often preceded by get) that means that what the user wanted to do coud not be achieved.

For the latter, there should be indeed a feedback to the user (with of course a single notification by unachieved feature and a message as understandable as possible).

[franck-eyraud]

Thanks for the merge!

For the latter, there should be indeed a feedback to the user (with of course a single notification by unachieved feature and a message as understandable as possible).

I tried to do that for now on the two write requests of this merge (create and remove highlight) in this pull request : https://github.com/Hypertopic/LaSuli/pull/63