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

Library Design #1

Open
acheong08 opened this issue Sep 12, 2023 · 42 comments
Open

Library Design #1

acheong08 opened this issue Sep 12, 2023 · 42 comments

Comments

@acheong08
Copy link
Owner

I need some advice on how the websocket connection should work in JS and how data should be returned from the library. The current implementation is an experiment and should not be taken seriously.

The design in my head:
Connect(vaultID: string, keyHash: string, token: string) - Returns some sort of channel (Or whatever the equivalent is in JavaScript). Spawn a thread in the plugin to listen to push operations from the server.

Pull(uid: string) - Sends a pull request through the websocket. Returns the binary and metadata

Push(...) etc

At least this is how I would do this in Golang. However, in JS, websockets are implemented with an onmessage callback. I do not know of a way to receive objects in order and know what type of message is being received.

CC @oskardotglobal

@acheong08
Copy link
Owner Author

acheong08 commented Sep 12, 2023

There is of course the option to rewrite the API of the server which is trivial as the current implementation is very modular. We don't even have to break compatibility with the official API - Just extending it to make our own use easier while exposing the old endpoints.

(I would prefer to have the websocket only be used for pushes from the server and have everything else be handled through a REST API. - 1 way communication. I understand there will be some overhead with HTTP/HTTPS but I don't think that matters too much)

@acheong08 acheong08 changed the title Websocket Design Library Design Sep 12, 2023
@acheong08
Copy link
Owner Author

acheong08 commented Sep 12, 2023

Note

I have deleted the contents of this repository as it wasn't well thought out. This will take a few weeks (max 1 month) to get functional as I am busy relocating to a new country.

@oskardotglobal
Copy link

I'd say websockets are the best bet here.

However, in JavaScript a websocket does either text or binary data, mixing them is a bit of a pain. In this case I'd go with encoding all binary data as Data URLs (which isn't ideal, but only starts becoming a problem once we have files larger than ~256MB), so then we can send metadata and the file itself via one message.
We could also have a type field which then determines the type when deserializing JSON.
I don't know how you would order the files though, but I don't see how that's necessary either. If that's a problem, we could send a timestamp along with the metadata though

@oskardotglobal
Copy link

We also have to keep speed in mind
I am not sure on how to best incorporate this; I'll be digging through the current sync server to get a better feel for the architecture though

@acheong08
Copy link
Owner Author

My confusion revolves around how to pull and push data. Here is the code for pulling a file:

case "pull":
	var pull struct {
		UID any `json:"uid" binding:"required"`
	}
	err = json.Unmarshal(msg, &pull)
	if err != nil {
		ws.WriteJSON(gin.H{"error": err.Error()})
		return
	}
	var uid int = utilities.ToInt(pull.UID)
	file, err := vaultfiles.GetFile(uid)
	if err != nil {
		ws.WriteJSON(gin.H{"error": err.Error()})
		return
	}
	var pieces int8 = 0
	if file.Size != 0 {
		pieces = 1
	}
	ws.WriteJSON(gin.H{
		"hash": file.Hash, "size": file.Size, "pieces": pieces,
	})
	if file.Size != 0 {
		ws.WriteMessage(websocket.BinaryMessage, file.Data)
	}

When you send a pull request, it first sends the metadata: "hash": file.Hash, "size": file.Size, "pieces": pieces,. You must then check how many pieces there are to decide whether the next piece of data is binary. I can't wrap my head around how to do that when all incoming messages go through the onmessage callback. The metadata and binary data do not contain a tag to identify them as such. We have to somehow know what to do with the incoming data without knowing what triggered it.

One solution is to have a persistent variable that keeps track of what should come next but because it's async, it would be problematic if you receive a push signal from the server while expecting a pulled binary.

Another issue is how to return data from functions when the onmessage callback can only be set once. For example, calling Pull(UID: string) will send a pull request to the server but the function that receives the response is in whatever initiated the onmessage.

There is probably a smarter way to do this but I can't think of it

@oskardotglobal
Copy link

I feel like a more advanced websocket library like https://feathersjs.com/ or https://socket.io/ would be of good use here, which allow emitting multiple "events" on one socket which then can be handled separately, although we would have to check how that's compatible with gin & the ws library you use (I didn't try feathersjs, but I did try socketio and it's pretty good).

The problem with vanilla websockets is that it's unnecessarily hard to check if the received data is binary or json (text), so it is of no use here. By the way, if we do use socketio we may be able to mount the socket variable outside of the connection's callback scope like:

let socket; // we could also then use a maybe type or something for this
io.on("connection", (s) => socket = s);

@acheong08
Copy link
Owner Author

After trying out a few libraries, I still went with vanilla websockets.

My idea goes something like this:
Connect handles the initial connection. onpush(callback: Function) can be used for when the server sends sync events
pull(uid:int) sends the pull request and waits for an event to be emitted (happens when either pull response is received with 0 pieces or binary data is received.) I'm still figuring out EventEmitter

@acheong08
Copy link
Owner Author

https://github.com/sindresorhus/p-event works well with emitters and allow me to simply wait for data to be emitted from onmessage callback in other functions

@acheong08
Copy link
Owner Author

acheong08 commented Sep 15, 2023

@oskardotglobal
TypeScript and Node is driving me insane.

Could you review my https://github.com/acheong08/obi-sync-lib/blob/main/tsconfig.json and https://github.com/acheong08/obi-sync-lib/blob/main/package.json?

Running tsc gives me broken imports in JS

Error [ERR_UNSUPPORTED_DIR_IMPORT]: Directory import '/var/home/acheong/Projects/obi-sync-lib/lib/src' is not supported resolving ES modules imported from /var/home/acheong/Projects/obi-sync-lib/lib/tests/vault_test.js

I can fix it manually in the JavaScript but is there a way to make sure it imports the right files on build?

@acheong08
Copy link
Owner Author

For example:

import { MakeKeyHash } from "./crypt";

is supposed to be

import { MakeKeyHash } from "./crypt.js";

@acheong08
Copy link
Owner Author

I ended up using esbuild to bundle everything together. Not sure if that is optimal but at least it works

@oskardotglobal
Copy link

You should really try https://bun.sh. It's a runtime, package manager and bundler fully compatible with node.
I'll review the whole code later today and might open a PR or two.

I don't know if the Eslint solution is ideal, I'll look into it.

@acheong08
Copy link
Owner Author

You should really try https://bun.sh/. It's a runtime, package manager and bundler fully compatible with node.

Just tried out Bun from their quickstart page. It worked on first try!

@acheong08
Copy link
Owner Author

Looks like there are still a few incompatibilities: e.g. crypto.subtle is missing when running bun build but works when running directly from the typescript

@oskardotglobal
Copy link

The crypto module isn't fully ported (see https://bun.sh/docs/runtime/nodejs-apis) which is expected since bun hit 1.0.0 like a week ago.
Related to your issue, did you try importing from node:crypto instead of crypto?

@acheong08
Copy link
Owner Author

acheong08 commented Sep 15, 2023

Related to your issue, did you try importing from node:crypto instead of crypto?

yes. didn't work.

🟡 Missing crypto.Certificate crypto.ECDH crypto.KeyObject crypto.X509Certificate crypto.checkPrime{Sync} crypto.createPrivateKey crypto.createPublicKey crypto.createSecretKey crypto.diffieHellman crypto.generateKey{Sync} crypto.generateKeyPair{Sync} crypto.generatePrime{Sync} crypto.getCipherInfo crypto.{get|set}Fips crypto.hkdf crypto.hkdfSync crypto.secureHeapUsed crypto.setEngine crypto.sign crypto.verify. Some methods are not optimized yet.

@acheong08
Copy link
Owner Author

esbuild works for now. Won't be looking into bun until it's stable.

I'll keep working on the websocket stuff over the next week

@acheong08
Copy link
Owner Author

@oskardotglobal Basic pull/push functionality is there. Mind taking a look at the code? Not sure if it's the right way of doing things.

To do:

  • End to end encryption
  • History
  • Delete
  • Restore

@oskardotglobal
Copy link

Aside from the nonexistent naming convention (which I can fix if you want me to) it looks great logic-wise. I have to dive deeper into the actual sync server though, I still don't fully understand how that works and it doesn't help there that my Go is a bit rusty
Once I get the gist of it, I might be able to contribute some of those features

@acheong08
Copy link
Owner Author

acheong08 commented Sep 23, 2023

Aside from the nonexistent naming convention

haha I just realized that ~ I'll fix it

Edit: Actually, I got no clue what the naming conventions are for TypeScript. I think I unconsciously went between Python and Golang conventions depending on what language I was just using before writing the code

acheong08 pushed a commit that referenced this issue Sep 23, 2023
@acheong08
Copy link
Owner Author

it looks great logic-wise

If so, I'll start working on the encryption and then a draft plugin. The other features can wait.

@baek-sang
Copy link

@acheong08
I was wondering if you have any plans for development.

@acheong08
Copy link
Owner Author

I've lately been using Neovim + Syncthing to take my notes as I currently don't have the bandwidth to maintain anything (currently in university).

@acheong08
Copy link
Owner Author

acheong08 commented Dec 10, 2023

One thing you can do is unpack the Obsidian app's asar file and remove the bits of code that prevent the current plugin from being used, and pack it back up. I can post instructions for that if anyone is interested.

@benkaiser
Copy link

I got kind of frustrated with the fact obsidian isn't open source, so ended up switching to an actual open source note taking server that tries to do things similar to obsidian called Trillium.

The plugin scene is nowhere near as developed, but the sync story is simple at least, since you just self-host it on a server and you're done. Downside of this approach is no offline support (unless you runs a copy locally and sync them).

@oskardotglobal
Copy link

I tried out and strongly dislike Trilium since getting out your data is hard since it isn't actual markdown, the formatting differs and since like you mentioned it required an internet connection 100% of the time

@xcnick
Copy link

xcnick commented Dec 14, 2023

One thing you can do is unpack the Obsidian app's asar file and remove the bits of code that prevent the current plugin from being used, and pack it back up. I can post instructions for that if anyone is interested.

Could you please post instructions about it? Thanks.

@azoller1
Copy link

I would recommend looking at this if you have time. Logseq could be a better option.

https://github.com/bcspragu/logseq-sync

@oskardotglobal
Copy link

Well this library is intended to not be bound to obsidian but to be usable with any kind of client plugin, so in logseq too

@acheong08
Copy link
Owner Author

Well this library is intended to not be bound to obsidian but to be usable with any kind of client plugin, so in logseq too

This is abandoned though.

Logseq could be a better option.

Definitely, although it still requires building it yourself & lacks IOS support.

@acheong08
Copy link
Owner Author

acheong08 commented Dec 23, 2023

@xcnick

Download release from https://github.com/obsidianmd/obsidian-releases/releases/tag/v1.4.16 & extract it

npx asar extract obsidian.asar in obsidian-1.4.16/resources

14204,14206c14204
<       var Yt =
<         "https://" +
<         [String.fromCharCode(97, 112, 105), "obsidian", "md"].join(".");
---
>       var Yt = "https://obsidian.yourdomain.com";
152934,152935d152931
<                           if (!HJ.call(u, ".obsidian.md") && "127.0.0.1" !== u)
<                             return s(new Error("Unable to connect to server."));
1241,1243d1240
<     (e.onBeforeRequest = r),
<       (e.onBeforeSendHeaders = r),
<       (e.onHeadersReceived = r);

npx asar pack obsidian obsidian.asar back in the same place

p.s. The error message "Unable to connect to server" is so misleading and obviously malicious...

@oskardotglobal
Copy link

This is abandoned though.

Maybe I'll continue the development myself.

@negue
Copy link

negue commented Dec 24, 2023

Logseq could be a better option.

Definitely, although it still requires building it yourself & lacks IOS support.

Logseq does have iOS Support - its available to download under https://logseq.com/downloads

@baek-sang
Copy link

Hopefully the obi-sync plugin will be revived.

@acheong08
Copy link
Owner Author

Logseq could be a better option.

Definitely, although it still requires building it yourself & lacks IOS support.

Logseq does have iOS Support - its available to download under https://logseq.com/downloads

No plugins on IOS (last I checked)

@xcnick
Copy link

xcnick commented Dec 27, 2023

After upgrading to version 1.5.3, it's not possible to create new vault in the vault list interface. How can this issue be resolved?
1703661216217

@acheong08
Copy link
Owner Author

acheong08 commented Dec 27, 2023

The error message says that you've reached the maximum of 5 vaults. Do you already have too many vaults? I can't find any code in obi-sync that raises that error. Could it be hard coded into the client?

@acheong08
Copy link
Owner Author

Ok I've downloaded the new version and am indeed facing the same issue. It's client side

@acheong08
Copy link
Owner Author

@xcnick Fixed: https://github.com/acheong08/obi-sync/releases/tag/v0.1.6

@johe123qwe
Copy link

johe123qwe commented Dec 31, 2023

I've updated to v0.1.6 but still have the same problem.
Obsidian v1 5 3 2023-12-31 18-56-24

@acheong08
Copy link
Owner Author

I'm pretty sure it's fixed. Check if the response is returning a limit. If not, then make sure that you have actually upgraded (build/restart server)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants