CouchDB
  1. CouchDB
  2. COUCHDB-1852

Last-Event-ID header should be honoured in eventsource _changes feed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4.0
    • Component/s: HTTP Interface
    • Labels:
      None

      Description

      When using the EventSource _changes feed support, the browser API will automatically reconnect and send the last sequence it received in the Last-Event-ID header.

      The server side needs to use the Last-Event-ID instead of the 'since' query string as a starting point for the changes feed.

      You can see the issue by simply creating a database and adding documents and watching what happens to the eventsource _changes feed.

      To see the feed with curl use:
      curl -i -H 'Last-Event-ID: 2' 'http://localhost:5984/testdb/_changes?feed=eventsource'
      the header should also have priority over the since query string:
      curl -i -H 'Last-Event-ID: 2' 'http://localhost:5984/testdb/_changes?feed=eventsource?since=1'

      1. 0001-add-test-for-COUCHDB-1852.patch
        2 kB
        Damjan Georgievski
      2. 0002-Fix-for-COUCHDB-1852.patch
        3 kB
        Damjan Georgievski

        Activity

        Hide
        Damjan Georgievski added a comment - - edited

        Here's a patch fixing this issue. This one works, but I could make it better if I move the line

        ChangesFun = couch_changes:handle_changes

        under the whole case ChangesArgs#changes_args.feed of block

        Show
        Damjan Georgievski added a comment - - edited Here's a patch fixing this issue. This one works, but I could make it better if I move the line ChangesFun = couch_changes:handle_changes under the whole case ChangesArgs#changes_args.feed of block
        Hide
        Damjan Georgievski added a comment -

        Updated the patch to add the header check in parse_changes_query, after all the query parameters are processed. That way the Last-Event-ID header overrides any "since" query params.

        Show
        Damjan Georgievski added a comment - Updated the patch to add the header check in parse_changes_query, after all the query parameters are processed. That way the Last-Event-ID header overrides any "since" query params.
        Hide
        Robert Newson added a comment -

        hm, I still see the original patch.

        Show
        Robert Newson added a comment - hm, I still see the original patch.
        Hide
        Damjan Georgievski added a comment -

        It seems I deleted the wrong one.

        Show
        Damjan Georgievski added a comment - It seems I deleted the wrong one.
        Hide
        Robert Newson added a comment -

        That looks better. A couple of comments but we're almost there;

        1) 'since it's probably the browser makes a reconnect' to 'since it's probably the browser reconnecting'
        2) A test to confirm the header overrides the since value in changes.js
        3) Add a sentence to ./share/doc/src/changes.rst to state that couchdb honors the Last-Event-ID header
        4) Add 'eventsource feeds now supports the Last-Event-ID header' to CHANGES and NEWS.

        Show
        Robert Newson added a comment - That looks better. A couple of comments but we're almost there; 1) 'since it's probably the browser makes a reconnect' to 'since it's probably the browser reconnecting' 2) A test to confirm the header overrides the since value in changes.js 3) Add a sentence to ./share/doc/src/changes.rst to state that couchdb honors the Last-Event-ID header 4) Add 'eventsource feeds now supports the Last-Event-ID header' to CHANGES and NEWS.
        Hide
        Damjan Georgievski added a comment -

        Here's the latest patches. I checked that the test fails for non-patched couch_httpd_db and succeeds on a patched one.

        I don't know if I can make the tests fail with a more reasonable error message?

        Show
        Damjan Georgievski added a comment - Here's the latest patches. I checked that the test fails for non-patched couch_httpd_db and succeeds on a patched one. I don't know if I can make the tests fail with a more reasonable error message?
        Hide
        Damjan Georgievski added a comment -

        New take on the test

        Show
        Damjan Georgievski added a comment - New take on the test
        Hide
        ASF subversion and git services added a comment -

        Commit dfd2199a4c6f67b76018eccfdd00b4fd4190e392 in branch refs/heads/master from Damjan Georgievski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=dfd2199 ]

        Support Last-Event-ID header for eventsource changes feeds

        COUCHDB-1852

        Show
        ASF subversion and git services added a comment - Commit dfd2199a4c6f67b76018eccfdd00b4fd4190e392 in branch refs/heads/master from Damjan Georgievski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=dfd2199 ] Support Last-Event-ID header for eventsource changes feeds COUCHDB-1852
        Hide
        ASF subversion and git services added a comment -

        Commit be1643b43106e30ae17e7aac92680c57be7018de in couchdb-chttpd's branch refs/heads/1843-feature-bigcouch-clustered-eventsource-changes-feed from Damjan Georgievski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb-chttpd.git;h=be1643b ]

        Support Last-Event-ID header for eventsource changes feeds

        COUCHDB-1852

        Show
        ASF subversion and git services added a comment - Commit be1643b43106e30ae17e7aac92680c57be7018de in couchdb-chttpd's branch refs/heads/1843-feature-bigcouch-clustered-eventsource-changes-feed from Damjan Georgievski [ https://git-wip-us.apache.org/repos/asf?p=couchdb-chttpd.git;h=be1643b ] Support Last-Event-ID header for eventsource changes feeds COUCHDB-1852
        Hide
        ASF subversion and git services added a comment -

        Commit 002a908132bfe6a74e4a886fc539a3b7987d4c63 in couchdb-chttpd's branch refs/heads/1843-feature-bigcouch-clustered-eventsource-changes-feed from Damjan Georgievski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb-chttpd.git;h=002a908 ]

        Support Last-Event-ID header for eventsource changes feeds

        COUCHDB-1852

        Show
        ASF subversion and git services added a comment - Commit 002a908132bfe6a74e4a886fc539a3b7987d4c63 in couchdb-chttpd's branch refs/heads/1843-feature-bigcouch-clustered-eventsource-changes-feed from Damjan Georgievski [ https://git-wip-us.apache.org/repos/asf?p=couchdb-chttpd.git;h=002a908 ] Support Last-Event-ID header for eventsource changes feeds COUCHDB-1852
        Hide
        ASF subversion and git services added a comment -

        Commit 002a908132bfe6a74e4a886fc539a3b7987d4c63 in couchdb-chttpd's branch refs/heads/master from Damjan Georgievski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb-chttpd.git;h=002a908 ]

        Support Last-Event-ID header for eventsource changes feeds

        COUCHDB-1852

        Show
        ASF subversion and git services added a comment - Commit 002a908132bfe6a74e4a886fc539a3b7987d4c63 in couchdb-chttpd's branch refs/heads/master from Damjan Georgievski [ https://git-wip-us.apache.org/repos/asf?p=couchdb-chttpd.git;h=002a908 ] Support Last-Event-ID header for eventsource changes feeds COUCHDB-1852

          People

          • Assignee:
            Unassigned
            Reporter:
            Damjan Georgievski
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development