WebDAV backend (WIP)#117
Conversation
commit 8ca8033 Author: Julian Brown <julian@jlbprof.com> Date: Mon May 14 09:28:08 2018 -0500 updated commit 9d86efe Author: Julian Brown <julian@jlbprof.com> Date: Wed May 9 19:46:56 2018 -0500 I have the options working commit 4452106 Author: Julian Brown <julian@jlbprof.com> Date: Wed May 9 19:06:17 2018 -0500 Added renderOptions per Marcel commit 32286bc Author: Julian Brown <julian@jlbprof.com> Date: Tue May 8 18:17:33 2018 -0500 Initial commit Copied Fake.js adapter to WebDav.js adapter. Started moving stuff around. Added webdavlib.js - we need to figure out how to satisfy license agreement, will deal with that later.
* Getting 503 on a webdav put, not sure why because I can use the exact GET and have it pull (using wget). * will investigate the problem further * I see no network activity and wonder if there is some restriction on it.
Things still to be done: * Deal with locking of bookmarks file on webdav server * Deal with converting to xbel format. * Deal with highest ID and persistance of same
Add attempt at WebDav Lock, but it fails.
I am worried about the fact that I have to html encode the href url's in the bookmark entities. This may not reverse parse correctly. Will investigate more tomorrow.
Here is where I am: * I download the existing xbel file, parse it and create the bookmarks in the map. * I have not dealt with the tabs/spaces issue. * please consider something like: https://www.npmjs.com/package/js-beautify to keep us to a coding style. * This works except on the 2nd sync I get the following error: Syncing failed with Trying to create a URL that is already bookmarked. I do not know what I did wrong. *This is still based on 2.2.3, after the last rebase causing merge conflicts I wanted to leave that alonw. *The parsing (and xml generation) code could just as easily work with the bookmarks.html file that chrome and probably firefox already work with. So we can either offer a checkbox for the appropriate format or change to the html mechanism if we wanted to. *further, when this gets more solid, we need to think of assigning folders ids as well, they mean something in xbel and in html formats. It will also help with bookmark ordering.
* fixed some minor issues. * Still having problems with 2nd sync
|
Most of the changes are whitespace, I changed indents to 2 spaces. I fixed the case where if it fails the lock would not be deleted. I make sure that the db contains only bookmark objects. Image of my developer tools console In the test situation where this fails, I have a "test" folder on the bookmarks bar with one bookmark to duckduckgo. The image shows what my db contains. That is what would be returned from getBookmark. I do not know how to proceed to fix this. I have the "remoteId" set, but the "localId" is not set and do not know what to set that too. But in essence the remoteId and localId should be the same, and I even tried that but I keep getting the same error about the 2nd bookmark with the same url. And it is trying to create that 2nd copy of the url. I am going to need help debugging this, because this is probably a dance between your side and my side that is not going well. When we get this fixed, I will work the "path" problem with slashes. Thanx Julian PS. My editor (vim) does not handle the indents too well with the lack of semicolons. I will have to look for a newer syntax module for javascript for vim. |
| } | ||
|
|
||
| async updateBookmark (remoteId, newBm) { | ||
| console.log('Update bookmark', newBM, remoteId, this.server) |
There was a problem hiding this comment.
This throws a newBM is not defined Error :)
|
Ah, I see the problem now. So, when floccus is syncing from scratch without any mappings available yet. It tries to create all bookmarks that are in the local sync folder on the server as well. The bookmarks app has the constraint that every URL may only appear once, so instead of creating a new bookmark it modifies the existing one and returns the existing ID. Floccus relies on this to initialize its mappings. There are different ways forward from this. We can try to move the code that deals with the peculiarities of the bookmarks app into the Adapter and have floccus work on trees with folder ids and duplicate bookmarks instead of the flat hierarchy of the bookmarks app. Or we can mimick the bookmarks app with the WebDAV adapter and also prevent duplicates. |
|
Seems a more robust way would be keep any backend specific stuff in an Adapter. That way, if you add more backends in the future, they have the solid core of floccus to build off, rather than having to adapt to the Bookmarks app conventions. |
|
My recommendation is that we redesign the adapter to be a "tree" style
adapter. In the case of Nextcloud we would "flatten" the tree, but for any
adapter that can represent a tree like WebDav and XBel we can "enrich" the
user experience. Some of the biggest issues I have with Floccus is that I
cannot sort the order of the bookmarks. I think it would be an
appreciated feature. Nextcloud cannot do the tree, I understand that, so
it would have to deal with not allowing duplicate url's or keep the order
of bookmarks across browsers.
Let me know how to proceed on that and I will do so. If we do allow for a
new tree structure we should probably discuss the adapter changes, such as
methods and parameters.
I will fix the other parts about, path and the strings as an integer
problem tomorrow.
Julian
…On Tue, Jun 5, 2018 at 1:29 PM TeutonJon78 ***@***.***> wrote:
Seems a more robust way would be keep any backend specific stuff in an
Adapter. That way, if you add more backends in the future, they has the
solid core of floccus to build off, rather than having to adapt to the
Bookmarks app conventions.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#117 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJS39UpiMejK9TdtRfA2wP9IYfsPK2F5ks5t5s38gaJpZM4UX4Bf>
.
|
I agree that's probably the more sustainable option. @jlbprof I've been working on a UI refactoring which I would like to merge before we refactor the Adapter interface in general, so we spare ourselves some rebasing :D So before that, I'd like to merge this PR as-is once you've fixed the last bits other than the problem I described; then I'll merge my UI refactoring, adjust your adapter code to work with the new UI framework; then we work out the details on how to revamp the adapter interface. Perhaps, while I do the UI stuff, you can already sketch an interface that would suit your adapter as a first draft, on a wiki page or in an issue. |
|
I hope to have the changes in by tomorrow evening.
…On Tue, Jun 5, 2018 at 5:31 PM Marcel Klehr ***@***.***> wrote:
@TeutonJon78 <https://github.com/TeutonJon78> @jlbprof
<https://github.com/jlbprof>
My recommendation is that we redesign the adapter to be a "tree" style
adapter.
I agree that's probably the more sustainable option.
@jlbprof <https://github.com/jlbprof> I've been working on a UI
refactoring which I would like to merge before we refactor the Adapter
interface in general, so we spare ourselves some rebasing :D So before
that, I'd like to merge this PR as-is once you've fixed the last bits other
than the problem I described; then I'll merge my UI refactoring, adjust
your adapter code to work with the new UI framework; then we work out the
details on how to revamp the adapter interface. Perhaps, while I do the UI
stuff, you can already sketch an interface that would suit your adapter as
a first draft, on a wiki page or in an issue.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#117 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJS39fk2LmuuVBpHMOH1g_EU17DpGkb2ks5t5wavgaJpZM4UX4Bf>
.
|
* Fixed more whitespaces, sorry for the whitespace spam. * Clean up of many console.log statements. * Fixed bookmark id's being a string instead of an integer from the parsed xbel file.
|
Thanx
…On Wed, Jun 6, 2018 at 5:32 PM Marcel Klehr ***@***.***> wrote:
Merged #117 <#117>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#117 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AJS39SRZA_F__nZ0d23c6UsEsuAwXCggks5t6Fh3gaJpZM4UX4Bf>
.
|
|
Been thinking about the redesign with a tree style adapter.
I think the adapter would be interacted with via 3 methods.
syncStart
syncFail
syncComplete
When you call syncStart it will return and entire Tree object with the
server's bookmarks.
When you call syncComplete you pass it a Tree object, that would include
any changes you have made such as:
* Removed folders or bookmarks would be flagged
* Changes to title, url would be flagged.
That way the Nextcloud adapter would just pass through "the changed"
sections and make appropriate changes on the server.
The other adapters would output a new bookmark file with the changes in
place.
This would make the adapter trivial, your job as the sync process would be
more complex perhaps.
Because of the way I built the WebDav, I could have some more options
1) Output in XBEL format.
2) Output in bookmarks html formt.
Just something to think about.
Julian
…On Wed, Jun 6, 2018 at 6:15 PM Julian Brown ***@***.***> wrote:
Thanx
On Wed, Jun 6, 2018 at 5:32 PM Marcel Klehr ***@***.***>
wrote:
> Merged #117 <#117>.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#117 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AJS39SRZA_F__nZ0d23c6UsEsuAwXCggks5t6Fh3gaJpZM4UX4Bf>
> .
>
|
|
The syncComplete wouldn't necessarily need to be a tree, though, right? It could just be an array of operations with nodes referenced by id, no? |
|
A tree is just a suggestion. If the list of nodes show a hierarchy that
would be fine too.
…On Thu, Jun 7, 2018 at 6:35 PM Marcel Klehr ***@***.***> wrote:
The syncComplete wouldn't necessarily need to be a tree, though, right? It
could just be an array of operations with nodes referenced by id, no?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#117 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJS39bg5u1qAOzJA3Jxp89eECuCftA1Dks5t6bingaJpZM4UX4Bf>
.
|
3 participants
No description provided.