automod context refactor (#500)
Builds on warpfork's progress, and our co-working a couple days back.
Changelog:
- all state mutations in a single dedicated `Effects` struct
- consolidated record event metadata into a single RecordOp struct.
intended to be immutable (kind of like AccountMeta, but with less
fetched context). aligns functionally with repoOp from the atproto
firehose schema, but has somewhat different syntax. encompasses any of
create, update, or delete ops on a single record
- removed "*Event" and effectively replaced with "*Context". Didn't have
immutable "Event" as sub-fields on Context, but the AccountMeta and
RecordOp are basically that. engine and effects are (intentionally)
private fields. `*Context` is the API exposed to rule functions
- cherry-picked an unrelated API/schema hotfix patch in to this branch
- merged effects package into engine
- split out capture (and "fetch") code to a package
- moved engine-specific tests into the engine package
- re-export some types from the top-level `automod` package
- nuked the `automod/util` package, for now (just copied around
dedupeStrings)
- cleaned up some code duplication in tests (just use ProcessRecordOp
instead of duplicating code)
Questions for review:
- should canonical-log-line be a method on Engine, or Context, or
neither? feels like rules should not have access to this method. I
suspect the logic around notifications and dev-observability will get
more sophisticated soon, like only logging or notifying in slack for
specific effects. the effects split-out should help a lot with this, and
simplifying the existing "persist" methods
- should "effects" field be exposed ("Effects") on Context, and methods
called directly on that? I decided not for now, and put thin wrapper
methods directly on Context. I think we've achieved the state/mutation
split in implementation/internals, and rule-writers don't need to know
or think about this distinction
- we *could* hide more fields on the Context structs and make them
accessible only via methods (`c.Account().Identity` instead of
`c.Account.Identity`). I think for the "core" metadata it is fine as
fields, even though this locks us in API-wise. maybe things like the
admin "Private" metadata (which may or may not be present) should be
behind a method call.
- probably should do a review of expected behaviors around
partial-processing (eg, if an error is encountered mid-persist) and
ensure that is sane and documented
Current state: all tests updated and passing. README updated. all the
docs/comments likely need a review and update, but I think that can be a
separate PR. This feels pretty good and my disposition is to merge this
pretty soon, as the branch has gotten large and will cause merge
conflicts (we knew this when we started). the last thing I want to do is
testing in staging and against prod firehose (from my laptop).