Validator Functions by sanderpick · Pull Request #416 · textileio/go-threads

sanderpick · 2020-08-06T18:02:55Z

Closes #418
Closes #406

Net

Allows a single thread host to maintain multiple logs for a given thread authored by different identities.
Provides a mechanism to allow an App to validate a Record before it's committed to a log.

DB

Collections can now take a JS function that validates collection writes. This opens the door to a lot of complex use cases, e.g., Buckets uses this feature to implement file-level access control.
Collections can now take a JS function that filters collection reads. This opens the door to a lot of complex use cases, e.g., Buckets uses this feature to implement file-level access control.

Misc.

Updates go-libp2p to v0.10.3
Removes the replace directives in go.mod, which are cumbersome for downstream projects.

Signed-off-by: Sander Pick <sanderpick@gmail.com>

core/app/app.go

core/db/db.go

net/net.go

Signed-off-by: Sander Pick <sanderpick@gmail.com>

jsign

LGTM! Very cool and powerful. 🚀
Left some comments but nothing important.

core/app/app.go

db/bench_test.go

db/collection.go

jsign · 2020-08-20T13:39:32Z

db/collection_test.go

+		c, err := db.NewCollection(CollectionConfig{
+			Name:   "Dog",
+			Schema: util.SchemaFromInstance(&Dog{}, false),
+			ReadFilter: `


Oh, it can also mutate the instance. Scary but powerful :P
Now feel ReadFilter might be a bit short in semantics since isn't only deciding to "show/not_show" but possibly changing. I don't have a good candidate name though.

jsign · 2020-08-20T13:46:04Z

db/db.go

-func (d *DB) HandleNetRecord(rec net.ThreadRecord, key thread.Key, lid peer.ID, timeout time.Duration) error {
-	if rec.LogID() == lid {
-		return nil // Ignore our own events since DB already dispatches to DB reducers
+func (d *DB) ValidateNetRecordBody(_ context.Context, body format.Node, identity thread.PubKey) error {


Nice. With this write validators, we should stress more than ever the single-writer approach.

That is, is very important that write validations relies on the assumption on getting writes in a single log where records are always presented in order. If some validation depends on two logs, depending on the order of processing you might have unwanted consequences. Sharing thoughts to validate mentality.

Maybe under more advanced validators, multi-writers might make sense... but thinking about all possible out of order arrivals to validate or not sounds hard.

jsign · 2020-08-20T13:56:28Z

jsonpatcher/jsonpatcher.go

+	JSONPatch  interface{} `json:"json_patch"`
+}
+
+func (je patchEvent) Marshal() ([]byte, error) {


Is this breaking change? Saying: changing the json format in the records?
Thinking about if older code can unmarshal correctly the new json tags? Sorry if confused!

Signed-off-by: Sander Pick <sanderpick@gmail.com>

* db: initial work for validator funcs Signed-off-by: Sander Pick <sanderpick@gmail.com> * db: adds collection read filter Signed-off-by: Sander Pick <sanderpick@gmail.com> * net: track logs by identity Signed-off-by: Sander Pick <sanderpick@gmail.com> * net: fix up tests Signed-off-by: Sander Pick <sanderpick@gmail.com> * net: add back compat for old own logs Signed-off-by: Sander Pick <sanderpick@gmail.com> * db: handle remaining db access Signed-off-by: Sander Pick <sanderpick@gmail.com> * logs: adds clear metadata method Signed-off-by: Sander Pick <sanderpick@gmail.com> * db: fix up tests Signed-off-by: Sander Pick <sanderpick@gmail.com> * net: fix record validation from net Signed-off-by: Sander Pick <sanderpick@gmail.com> * mod: migrate datastore to own namespace Signed-off-by: Sander Pick <sanderpick@gmail.com> * mod: use both go-datastores Signed-off-by: Sander Pick <sanderpick@gmail.com> * logstore: fix up tests Signed-off-by: Sander Pick <sanderpick@gmail.com> * cleanup: typos found during self review Signed-off-by: Sander Pick <sanderpick@gmail.com> * review: address feedback Signed-off-by: Sander Pick <sanderpick@gmail.com>

requilence · 2020-09-01T11:54:05Z

jsonpatcher/jsonpatcher.go


 type patchEvent struct {
-	Timestamp      time.Time
+	Timestamp      int64


@sanderpick FYI this change breaks the backward compatibility with existing logs/dbs...
Not sure if you have an intent to promise this by the moment.

Let me know, I can propose PR to support the old format

Hrm, okay. Let's see... what do you propose? To add some context, time.Time isn't cbor-serializable since it doesn't export any fields.

You can try to cbor-serialize the old-format patchEvent(with time.Time) and then try to deserialize it into the new format(with int64) or vice versa. You will get unmarshal error.
Here is the proof to test:
https://gist.github.com/requilence/d80e0eb1e346a8b40403e8593163aabf

output:

=== RUN TestJsonPatcher_Migration jsonpatcher_test.go:54: malformed stream: invalid appearance of uint token; expected start of map jsonpatcher_test.go:59: unmarshal error: cannot assign <{:0> to int64 field

Added a comment to your gist: https://gist.github.com/requilence/d80e0eb1e346a8b40403e8593163aabf#gistcomment-3438857

@sanderpick thanks, the solution you've proposed seems ok, but still older clients will not be able to read the data created with the newer version.

Personally, I don't like the way these cbor-serialized structs become no-upgradable at all. In case of any struct amend, you need to preserve the old struct and still break the non-upgraded clients that read the same data.

On the other hand, PB-encoded ipld nodes look more solid as it provides the way to add new fields and remove the old ones as far as you don't reuse the same indexes. It also ignores the extra fields, which means compatible with older clients. Maybe you can provide some context why did you prefer cbor?

Anyway, it may be also useful to add some comments on all cbor-encoded structs that it shouldn't be changed

Sorry for the delay. Coming up for air :) Agreed, the upgrade-ability is poor. IMO, CBOR is much nicer to work with compared to proto nodes in every other way. In any case, it would be an undertaking to migrate to proto nodes. Ideally, we could use any kind of format.Node at the net layer.

We could introduce a Version to the core.Event to make the handling a little cleaner. Still, older nodes are out of luck on this one.

I ended up going with a simpler solution: #439

sanderpick self-assigned this Aug 6, 2020

weekly-digest bot mentioned this pull request Aug 7, 2020

Weekly Digest (31 July, 2020 - 7 August, 2020) #417

Closed

sanderpick added 5 commits August 14, 2020 13:51

db: initial work for validator funcs

64996a2

Signed-off-by: Sander Pick <sanderpick@gmail.com>

db: adds collection read filter

ffc9f68

Signed-off-by: Sander Pick <sanderpick@gmail.com>

net: track logs by identity

734c5ae

Signed-off-by: Sander Pick <sanderpick@gmail.com>

net: fix up tests

8664b7e

Signed-off-by: Sander Pick <sanderpick@gmail.com>

net: add back compat for old own logs

e07eb8b

Signed-off-by: Sander Pick <sanderpick@gmail.com>

sanderpick force-pushed the sander/validator-funcs branch from 62a30f6 to e07eb8b Compare August 14, 2020 22:06

sanderpick added 6 commits August 19, 2020 08:14

db: handle remaining db access

5cef2fc

Signed-off-by: Sander Pick <sanderpick@gmail.com>

logs: adds clear metadata method

c24477b

Signed-off-by: Sander Pick <sanderpick@gmail.com>

db: fix up tests

2455c2a

Signed-off-by: Sander Pick <sanderpick@gmail.com>

net: fix record validation from net

6681ce5

Signed-off-by: Sander Pick <sanderpick@gmail.com>

mod: migrate datastore to own namespace

438962d

Signed-off-by: Sander Pick <sanderpick@gmail.com>

mod: use both go-datastores

f321fa7

Signed-off-by: Sander Pick <sanderpick@gmail.com>

sanderpick marked this pull request as ready for review August 20, 2020 00:52

logstore: fix up tests

27142fa

Signed-off-by: Sander Pick <sanderpick@gmail.com>

sanderpick commented Aug 20, 2020

View reviewed changes

sanderpick requested a review from jsign August 20, 2020 05:55

cleanup: typos found during self review

330f712

Signed-off-by: Sander Pick <sanderpick@gmail.com>

sanderpick force-pushed the sander/validator-funcs branch from 4cffd14 to 330f712 Compare August 20, 2020 06:03

jsign approved these changes Aug 20, 2020

View reviewed changes

This was referenced Aug 20, 2020

Allow collection read filter to process batches of instances #422

Open

Cache goja runtime and callables on Collection #423

Closed

review: address feedback

15eafc8

Signed-off-by: Sander Pick <sanderpick@gmail.com>

sanderpick merged commit c673f8a into master Aug 20, 2020

sanderpick deleted the sander/validator-funcs branch August 20, 2020 17:43

sanderpick restored the sander/validator-funcs branch August 20, 2020 23:10

sanderpick deleted the sander/validator-funcs branch August 20, 2020 23:13

requilence reviewed Sep 1, 2020

View reviewed changes

requilence mentioned this pull request Sep 1, 2020

revert patchEvent field change #430

Closed

This was referenced Sep 16, 2020

File-level bucket access control textileio/textile#316

Merged

jsonpatcher: empty interface timestamp #439

Merged

Conversation

sanderpick commented Aug 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Net

DB

Misc.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsign left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

requilence Sep 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

requilence Sep 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sanderpick commented Aug 6, 2020 •

edited

Loading

requilence Sep 1, 2020 •

edited

Loading

requilence Sep 1, 2020 •

edited

Loading