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

update local version of containing folder when a new items comes in from remote #759

Open
michielbdejong opened this issue Sep 12, 2014 · 11 comments

Comments

@michielbdejong
Copy link
Member

When a folder listing is fetched, and we become aware of items that exist remotely but not locally, the entry in node.local.itemsMap is set to false. This is good because it means when you do getListing, you only see items that actually exist in the cache.

However, when such an item is then later fetched, its entry in the containing folder's node.local.itemsMap should be changed from false to true, and we're not currently doing that.

@michielbdejong
Copy link
Member Author

PS: a false entry in the local itemsMap can also indicate a local deletion. The way to distinguish them is that for unfetched items, no node exist, whereas for deleted items a node exists for the item itself with a common version still containing the content that was deleted, and then a local version with body false. This node is then removed when the DELETE request is sent, and after that the updated folder will be fetched with a GET. So there it al works correctly.

@raucao
Copy link
Member

raucao commented Dec 10, 2014

Next step:

  • Write failing test

@raucao
Copy link
Member

raucao commented Jan 23, 2015

This is marked as blocker, but I think nobody currently understands the actual implications of the wrong behaviour, and yet we really want to release a new stable version with all the bugfixes since 0.11.0. @michielbdejong Do you think we should not release without fixing this? Or can we keep the blocker label, but at least release 0.11.1 to get all the other fixes out?

@michielbdejong
Copy link
Member Author

Yes, congrats on the 0.11.1 release! Ping me when you or someone else has time to pair on this one.

@raucao
Copy link
Member

raucao commented Jan 24, 2015

Unfortunately, our Internet connection here on Dominica is not sufficient for remote pairing.

@silverbucket
Copy link
Member

Hey @michielbdejong, I thought I remembered that, on our last call, you said you'd make some unit tests to illustrate the issue, do you think you'll have some time to do that soon?

@raucao
Copy link
Member

raucao commented Jan 24, 2015

Yer, that would explain it to the person who wants to fix it then.

@michielbdejong
Copy link
Member Author

Yes, but we'll do that as part of the pairing session. And then immediately fix it while we're at it. Let me know as soon as someone has time.

@raucao
Copy link
Member

raucao commented Jan 25, 2015

If you just write a test, someone can pick it up and fix it without pairing. As I said, Internet connection here is not sufficient for remote pairing.

Edit: or it'll have to wait until mid February.

@michielbdejong
Copy link
Member Author

@skddc do you want to pair on this now?

@raucao
Copy link
Member

raucao commented Mar 19, 2016

I can pair on this now.

@raucao raucao added the ready label Mar 19, 2016
@raucao raucao removed the ready label Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants