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

Durable users - WIP / for discussion #78

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 58 additions & 3 deletions src/yetibot/core/adapters/adapter.clj
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,63 @@

(defprotocol Adapter

;; TODO implement this in IRC and Slack adapters, then call it from the
;; respective functions in each adapter that handle incoming messages.
(resolve-users
[_ event]
"Given some kind of event, figure out if users are involved and resolve them
(where resolve means look them up in the DB, creating them on demand if
they don't already exist).

Users are accreted lazily over time in Yetibot as they are:

- observed because they authored a message
- mentioned in another user's message
- found to be in a channel after a user asked for the members of a channel
- possibly other ways we haven't thought about yet

Note: there is no standard notion of events shared between adapters and
there doesn't necessarily need to be, since the event is handled completely
internally by each adapter.

Types of events and corresponding users could include:

1. Incoming message
- An incoming message has an author
- A message may mention a user or users. In IRC this could rely on a
convention of @username style mentions. In Slack, it's explicit since
Slack encodes users.

2. Channel join or leave
- This is a more eager resolution of users - probably not worth doing in
the initial pass, but listed here as an example of the kind of thing we
*could* do.

For each detected user call `resolve-user!` to ensure it's persisted and
canonicalized.

Returns a map of {id user}.
")

;; TODO implement
(resolve-user! [_ adapter-user]
"Takes a user in the shape that a specific adapter provides and
canonicalizes and persists (if not already).

See yetibot.core.models.users/create-user for the existing attempt to do
this (in memory representation only - no persistence).

Returns the canonicalized user.")

;; TODO implement in all adapters
(users [_ channel]
"Given a channel, figure out which users are in the channel and resolve all
of them via `resolve-user!`.

Returns a map of {id user}")

(uuid [_] "A UUID that represents an instance, represented in config by the
:name key")
:name key")

(platform-name [_] "String describing the chat platform this adapter supports.")

Expand All @@ -18,12 +73,12 @@
(send-msg [_ msg] "Single message to post")

(join [_ channel]
"join channel - may not be supported by all adapters, e.g. Slack. In this
"join channel - may not be supported by all adapters, e.g. Slack. In this
case the adapter should return instructions for its method of joining
(e.g. /invite in Slack).")

(leave [_ channel] "leave channel - may not be supported - should give instructions
just like join.")
just like join.")

(chat-source [_ channel] "Define a chat-source map specific to this adapter")

Expand Down
27 changes: 15 additions & 12 deletions src/yetibot/core/adapters/irc.clj
Original file line number Diff line number Diff line change
Expand Up @@ -119,18 +119,21 @@
"Recieve and handle messages from IRC. This can either be in channels yetibot
is listening in, or it can be a private message. If yetibot does not
recognize the :target, reply back to user with PRIVMSG."
[a irc info]
(log/info "handle-message" (pr-str info))
(let [config (:config a)
{yetibot-nick :nick} @irc
yetibot-user (construct-yetibot-from-nick yetibot-nick)
user-id (:user info)
chan (or (recognized-chan? a (:target info)) (:nick info))
user (users/get-user (chat-source chan) user-id)]
(log/info "handle message" info "from" chan yetibot-user)
(binding [*target* chan]
(handle-raw (chat-source chan)
user :message yetibot-user {:body (:text info)}))))
[a irc info]
(log/info "handle-message" (pr-str info))
;; TODO - call (a/resolve-users info) to ensure users are resolved, then
;; pass the resulting map along to `handle-raw` below in addition to `:body`.
;; Maybe we call it :resolved-users?
(let [config (:config a)
{yetibot-nick :nick} @irc
yetibot-user (construct-yetibot-from-nick yetibot-nick)
user-id (:user info)
chan (or (recognized-chan? a (:target info)) (:nick info))
user (users/get-user (chat-source chan) user-id)]
(log/info "handle message" info "from" chan yetibot-user)
(binding [*target* chan]
(handle-raw (chat-source chan)
user :message yetibot-user {:body (:text info)}))))

(defn handle-part
"Event that fires when someone leaves a channel that Yetibot is listening in"
Expand Down
3 changes: 3 additions & 0 deletions src/yetibot/core/adapters/slack.clj
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@
yetibot? (= yetibot-uid (:user event))
user-model (assoc (users/get-user cs (:user event))
:yetibot? yetibot?)
;; TODO - call (a/resolve-users event) to ensure users are resolved,
;; then pass the resulting map along to `handle-raw` as
;; `:resolved-users` below in addition to `:body`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a potential starting place.

  1. Add a/resolve-users function
  2. call it via (a/resolve-users adapter event)
  3. in a/resolve-users log the event and start looking at how we can pull users out of it and turn them into canonical persistent records

body (if (s/blank? (:text event))
;; if text is blank attempt to read an attachment fallback
(->> event :attachments (map :text) (s/join \newline))
Expand Down
3 changes: 2 additions & 1 deletion src/yetibot/core/commands/alias.clj
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
;; evaluation of piped expressions (i.e. %s instead of $s)
expr (binding [*subst-prefix* method-like-replacement-prefix]
(str command/config-prefix (pseudo-format-n cmd args)))
results (record-and-run-raw expr user yetibot-user
resolved-users nil ;; TODO
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure aliases would ever mention users. Either way, we can defer this to a later PR if we want.

results (record-and-run-raw expr user resolved-users yetibot-user
;; avoid double recording the yetibot
;; response since the parent command
;; execution that evaluated the alias
Expand Down
2 changes: 1 addition & 1 deletion src/yetibot/core/commands/cron.clj
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
*adapter-uuid* (read-string chat-source-adapter)]
(let [expr (str command/config-prefix cmd)
[{:keys [error? timeout? result] :as expr-result}]
(record-and-run-raw expr nil nil)]
(record-and-run-raw expr nil nil nil)]
(info "cron result" (pr-str expr-result))
(if (and result (not error?) (not timeout?))
(chat-data-structure result)
Expand Down
3 changes: 2 additions & 1 deletion src/yetibot/core/commands/observe.clj
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@
;; username and channel name with no explicit
;; piping behavior
rendered-cmd))
resolved-users nil ;; TODO
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can defer.

[{:keys [error? timeout? result] :as expr-result}]
(record-and-run-raw expr user yetibot-user)]
(record-and-run-raw expr user resolved-users yetibot-user)]
(if (and result (not error?) (not timeout?))
(chat-data-structure result)
(info "Skipping observer because it errored or timed out"
Expand Down
22 changes: 22 additions & 0 deletions src/yetibot/core/db/user.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
(ns yetibot.core.db.user
(:require
[yetibot.core.db.util :as db.util]))

(def schema {:schema/table "user"
:schema/specs
(into [[:chat-source-adapter :text]
[:user-id :text "NOT NULL"]
[:username :text "NOT NULL"]
[:display-name :text "NOT NULL"]
;; should we attempt to keep track of which channels the
;; user is in? 🤔
;; if we wanted to store it as a column here it's have to be
;; some serialized form of a Clojure collection
]
(db.util/default-fields))})

(def create (partial db.util/create (:schema/table schema)))

(def query (partial db.util/query (:schema/table schema)))

(def delete (partial db.util/delete (:schema/table schema)))
99 changes: 51 additions & 48 deletions src/yetibot/core/handler.clj
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@
(defn handle-parsed-expr
"Top-level for already-parsed commands.
Turns a parse tree into a string or collection result."
[chat-source user yetibot-user parse-tree]
[chat-source user resolved-users yetibot-user parse-tree]
(binding [interp/*current-user* user
interp/*yetibot-user* yetibot-user
interp/*resolved-users* resolved-users
interp/*chat-source* chat-source]
(transformer parse-tree)))

Expand Down Expand Up @@ -88,10 +89,10 @@

Otherwise returns nil for non expressions.
"
[body user yetibot-user & [{:keys [record-yetibot-response?]
:or {record-yetibot-response? true}}]]
[body user resolved-users yetibot-user & [{:keys [record-yetibot-response?]
:or {record-yetibot-response? true}}]]
(trace "record-and-run-raw" body record-yetibot-response?
interp/*chat-source*)
interp/*chat-source*)
(let [{:keys [adapter room uuid is-private]
:as chat-source} interp/*chat-source*
timestamp (System/currentTimeMillis)
Expand All @@ -101,15 +102,15 @@
;; - the result that Yetibot posts back to chat
correlation-id (str timestamp "-" (hash [chat-source user body]))
parsed-normal-command (when-let
[[_ body] (extract-command body)]
[[_ body] (extract-command body)]
(parser body))

parsed-cmds
(or
;; if it starts with a command prefix (e.g. !) it's a command
(and parsed-normal-command [parsed-normal-command])
(and parsed-normal-command [parsed-normal-command])
;; otherwise, check to see if there are embedded commands
(when (embedded-enabled?) (embedded-cmds body)))
(when (embedded-enabled?) (embedded-cmds body)))
cmd? (boolean (seq parsed-cmds))]

;; record the body of users' messages if the user is not Yetibot
Expand All @@ -134,51 +135,53 @@
(when (and cmd? (not (:yetibot? user)))
(let [[results timeout-result]
(alts!!
[(go (map
(fn [parse-tree]
(try
(let [original-command-str (unparse parse-tree)
{:keys [value error]} (handle-parsed-expr
chat-source user
yetibot-user
parse-tree)
result (or value error)
error? (not (nil? error))
[formatted-response _] (format-data-structure
result)]
[(go (map
(fn [parse-tree]
(try
(let [original-command-str (unparse parse-tree)
{:keys [value error]} (handle-parsed-expr
chat-source
user
resolved-users
yetibot-user
parse-tree)
result (or value error)
error? (not (nil? error))
[formatted-response _] (format-data-structure
result)]
;; Yetibot should record its own response in
;; `history` table before/during posting it back to
;; the chat adapter. Then we can more easily
;; correlate request (e.g. commands from user) and
;; response (output from Yetibot)
(trace
record-yetibot-response?
"recording history" uuid room is-private
original-command-str formatted-response)
(when record-yetibot-response?
(h/add {:chat-source-adapter uuid
:chat-source-room room
:is-private is-private
:correlation-id correlation-id
:user-id (-> yetibot-user :id str)
:user-name (-> yetibot-user :username str)
:is-yetibot true
:is-command false
:is-error error?
:command original-command-str
:body formatted-response}))
(trace
record-yetibot-response?
"recording history" uuid room is-private
original-command-str formatted-response)
(when record-yetibot-response?
(h/add {:chat-source-adapter uuid
:chat-source-room room
:is-private is-private
:correlation-id correlation-id
:user-id (-> yetibot-user :id str)
:user-name (-> yetibot-user :username str)
:is-yetibot true
:is-command false
:is-error error?
:command original-command-str
:body formatted-response}))
;; don't report errors on embedded commands
{:embedded? (not parsed-normal-command)
:error? error?
:result result})
(catch Throwable ex
(error "error handling expression:" body
(format-exception-log ex))
{:embedded? (not parsed-normal-command)
:error? true
:result (format exception-format ex)})))
parsed-cmds))
(timeout expr-eval-timeout-ms)])]
{:embedded? (not parsed-normal-command)
:error? error?
:result result})
(catch Throwable ex
(error "error handling expression:" body
(format-exception-log ex))
{:embedded? (not parsed-normal-command)
:error? true
:result (format exception-format ex)})))
parsed-cmds))
(timeout expr-eval-timeout-ms)])]
(or results
[{:timeout? true
:result (str "Evaluation of `" body "` timed out after "
Expand Down Expand Up @@ -212,14 +215,14 @@
:react"
[{:keys [adapter room uuid is-private] :as chat-source}
user event-type yetibot-user
{:keys [body reaction] :as event-info}]
{:keys [resolved-users body reaction] :as event-info}]
;; Note: only :message and :react have a body
(when (and body (= event-type :message))
(binding [interp/*chat-source* chat-source]
(go
;; there may be multiple expr-results, as in the case of multiple
;; embedded commands in a single body
(let [expr-results (record-and-run-raw body user yetibot-user)]
(let [expr-results (record-and-run-raw body user resolved-users yetibot-user)]
(run!
(fn [{:keys [timeout? embedded? error? result]}]
(if (or (not error?) (not embedded?))
Expand Down
3 changes: 3 additions & 0 deletions src/yetibot/core/interpreter.clj
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

(def ^:dynamic *current-user*)
(def ^:dynamic *yetibot-user*)
(def ^:dynamic *resolved-users*)
(def ^:dynamic *chat-source*)

(defn handle-cmd
Expand Down Expand Up @@ -38,6 +39,7 @@
{previous-value :value
previous-data-collection :data-collection
previous-data :data} acc
;; this is the context map that's passed to all command handlers
extra {:raw previous-value
:data (or previous-data previous-value)
:data-collection previous-data-collection
Expand All @@ -46,6 +48,7 @@
:next-cmds next-cmds
:user *current-user*
:yetibot-user *yetibot-user*
:resolved-users *resolved-users*
:chat-source *chat-source*}
possible-opts (to-coll-if-contains-newlines previous-value)]

Expand Down