Was BiscuitBuilder.build consuming itself intentional?

[jmhodges]

First off, thanks so much for building this library! It's been a huge boon.

I've been experimenting with writing a Python wrapper for biscuit-rust, but hit a thing I'm not sure was intentional or not. I'm definitely not as experienced in Rust as y'all are, so this is an earnest, gentle query.

Let me give some context first.

Because of Python's GC, we have to get a lil tricky with lifetime annotations. It's currently typical to have to do magic with ouroboros::self_reference like I have in biscuit-auth-python (and folks elsewhere have in crfs, etc) to get shared references to Rust objects like BiscuitBuilder that are contained in Python objects.

However, that's exposed the fact that the BiscuitBuilder.build method, as currently written, can't work on a shared reference, even a mutable one. In my Python lib, we have to throw a clone in to make things work

I expected a BiscuitBuilder to more like other builder patterned code and for it to be reused, to copy the data into a new Biscuit, and to not modify its own SymbolTable. Given the lifetime annotation work on BiscuitBuilder, I wondered: is it intentional to have it give up ownership of its fields to the Biscuit and modify the SymbolTable?

The modifying of SymbolTable was surprising on its own (split_at uses split_off which removes items from the Vec its called on). That made me wonder if perhaps there was something about BiscuitBuilder's purpose I misunderstood? Is there something about building Biscuits with partial views of a SymbolTable that's useful?

[jmhodges]

This might all come down to me not knowing what a Biscuit with only some of the symbols from the SymbolTable would mean?

[Geal]

right, so, the BiscuitBuilder holding a reference to the root key pair does not help much, it should instead receive the key pair as an argument to the build() method. This is something I intend to change for the next version.

The symbol table is used for string interning, and a part of the table can be known in advance (either the hardcoded default symbols, or a different symbol table can be provided during the build). So as Datalog elements are added to the builder, we add the new strings to the symbol table, and when building the token, we only keep the new strings to serialize them in the token, not the default ones.

So if you have a default symbol table holding "A", and "B", then add facts that use the strings "A", "C" and "D", you get a symbol table containing "A", "B", "C", and "D", but we split it to keep only the newer elements, "C", and "D".

But as with the root key pair, the builder should not hold the symbol table, instead it could be passed as argument to build()

[Geal]

FYI the builder API changed in #86, it does not hold a reference to the root key anymore