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

Implement ETag & Last-modified #70

Open
brantje opened this issue Apr 10, 2018 · 9 comments
Open

Implement ETag & Last-modified #70

brantje opened this issue Apr 10, 2018 · 9 comments

Comments

@brantje
Copy link
Owner

brantje commented Apr 10, 2018

This will greatly speed up the api.
For reference we can use this.
Shouldn't be too difficult.
@ThomasDaheim Want to give this a try? If you want to, make sure you branch of the master branch.

cc @korelstar Can you guide us along the way?

@ThomasDaheim
Copy link
Collaborator

Will have a look at that. From the first glance it seems to be incompatible with folder mode since it needs to store data per note in the database?

@brantje
Copy link
Owner Author

brantje commented Apr 10, 2018

Afaik, the notes app is using files. I'll dig a bit into it later.

@ThomasDaheim
Copy link
Collaborator

Well, they now also use a database for metadata. Maybe thats something we could live with.

@ThomasDaheim
Copy link
Collaborator

Would want do clone the notes repo anyways to see how the resolved "$note->getId()" in case only files are used. Not sure where you would store an id in that case...

@brantje
Copy link
Owner Author

brantje commented Apr 10, 2018

getId() returns always an int.

@ThomasDaheim
Copy link
Collaborator

Yes, have seen that. Its somehow info stored by nextcloud along with the file. So it should be relatively easy to retrieve it for a file and (hopefully) also find the file for an id :-)

@ThomasDaheim
Copy link
Collaborator

ThomasDaheim commented Apr 10, 2018

BTW, how could this be easily tested once added as feature? I don't know to much about testing in such an environment...

@brantje
Copy link
Owner Author

brantje commented Apr 10, 2018

I think if we follow this:



## API-Changes

**1. Don't transfer data if nothing has changed (`ETag`/`If-None-Match`)**

The server now sends the HTTP header field `ETag`, which is a hash value of the server response. After processing the server response, clients should save this value locally and send the HTTP header `If-None-Match` using the saved value in their next request to the server. If the server wants to response with the same data, it will send `304 Not Modified` without any data instead. This behavior is conform to the HTTP standard and fully backward compatible.

**2. Only transfer changed data, if something has changed (`pruneBefore`)**

If only a single note is changed (edited or deleted), we nevertheless don't want to transfer all notes again. Therefore, the server response includes the HTTP header `Last-Modified`. After processing the server response, clients should save this value locally and use the parameter `pruneBefore` in their next request to the server. Then, the server prunes all notes which where changed before this date (i.e. only their `id` is transferred). With this approach, clients are able to update changed notes and also remove deleted notes while reducing the transferred data size significantly. This behavior is fully backward compatible.

It should be fine, otherwise i hope that @korelstar could help a bit out.

@ThomasDaheim
Copy link
Collaborator

Long time no progress here. By now I have managed to upgrade my NAS so that its fast enough for development & testing.

I would think this is a feature to add after v1.0 is ready? Stable first - fast later :-) Or is that an essential feature right away?

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

2 participants