earthstar-project / earthstar

Storage for private, distributed, offline-first applications.
https://earthstar-project.org
GNU Lesser General Public License v3.0
638 stars 20 forks source link

Change optional document fields to required and nullable #45

Closed cinnamon-bun closed 4 years ago

cinnamon-bun commented 4 years ago

What's the problem you want solved?

The deleteAfter field in a Document is optional and not nullable.

    deleteAfter?: number,

This seems to be awkward in GraphQL? Earthstar-graphql returns documents with deleteAfter: null.

Another optional field is also coming soon, workplaceSignature, for invite-only workspaces.

Is there a solution you'd like to recommend?

We could make the fields required and nullable.

    deleteAfter: number | null,

I actually prefer this since it makes the documents more self-documenting -- as a programmer, you won't ever be surprised by a document with a field you haven't seen before.

I had made them optional to save space, but it's only 17 bytes extra per document to make them required and null.

sgwilym commented 4 years ago

Right. GraphQL servers and clients assume that a schema is determined before runtime, so having optional fields which are determined dynamically are impossible.

One way to handle this case is nullable fields, which I think is the right call in this context. 👍

Another way we can go, and one which could make sense for workspaceSignature, is to create multiple types e.g. PrivateWorkspaceES4Document (great name, right?), which has a non nullable workspaceSignature, and have regular ES4Document have no field at all. There would definitely be trade-offs to doing this, but the benefit is that it makes certain impossible states impossible, e.g. like having a document with no workspaceSignature in an invite only workspace.

GraphQL also has support for interfaces, so both the private workspace and public workspace document types could share a common ES4Document interface. Hmm, I quite like the sound of that, actually...

cinnamon-bun commented 4 years ago

Ok, I'll change deleteAfter to null | number. 👍


I'm holding back on workspaceSignature because I don't know how to tell if a workspace is invite-only or not, just from its address.

Workspace addresses can look like...

Memorable
+gardening.friends

Hard to guess (any length of random stuff)
+gardening.aoifja3aijfaof

Invite only (public key, 53 chars starting with 'b')
+gardening.bk4cq3xogyakm3ij33grk7yo3bhsfxyatfxs675tbgeedhgppvzaa

If the last section could be a public key, does that mean it's automatically treated as invite-only and requires a workspaceSignature? Or should the address be more explicit that it's invite-only?

Maybe format could be different, like es.4.invite-only... I don't know what patterns to use in format strings yet either.

sgwilym commented 4 years ago

I'm certain I don't understand the implications of what I'm suggesting, but what if private workspaces had a different sigil? Eg -ourspace.z98 vs +freeforall.a66

cinnamon-bun commented 4 years ago

✅ Changed deleteAfter to a required, nullable field (number | null).

Commit: b5811432b1573673846c6cda40373f6f7877a974

@sgwilym: interesting idea about workspace sigils. I'll meditate on that...