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

build InputHelper API #24

Open
warner opened this issue Apr 6, 2018 · 6 comments
Open

build InputHelper API #24

warner opened this issue Apr 6, 2018 · 6 comments

Comments

@warner
Copy link
Collaborator

warner commented Apr 6, 2018

As described in #8, the tab-completion type-in-the-code API uses an object named the InputHelper. In the sans-io approach, this object will be defined (and owned) by the IO/API glue layer, but the Core will define some API input and output events to communicate with it.

The Python API docs specify that you obtain a helper from the wormhole object with a synchronous function likehelper = w.input_code(), and then defines the Helper's API as follows:

  • refresh_nameplates(): requests an updated list of nameplates from the
    Rendezvous Server. These form the first portion of the wormhole code (e.g.
    "4" in "4-purple-sausages"). Note that they are unicode strings (so "4",
    not 4). The Helper will get the response in the background, and calls to
    get_nameplate_completions() after the response will use the new list.
    Calling this after h.choose_nameplate will raise
    AlreadyChoseNameplateError.
  • matches = h.get_nameplate_completions(prefix): returns (synchronously) a
    set of completions for the given nameplate prefix, along with the hyphen
    that always follows the nameplate (and separates the nameplate from the
    rest of the code). For example, if the server reports nameplates 1, 12, 13,
    24, and 170 are in use, get_nameplate_completions("1") will return
    {"1-", "12-", "13-", "170-"}. You may want to sort these before
    displaying them to the user. Raises AlreadyChoseNameplateError if called
    after h.choose_nameplate.
  • h.choose_nameplate(nameplate): accepts a string with the chosen
    nameplate. May only be called once, after which
    AlreadyChoseNameplateError is raised. (in this future, this might
    return a Deferred that fires (with None) when the nameplate's wordlist is
    known (which happens after the nameplate is claimed, requiring a roundtrip
    to the server)).
  • d = h.when_wordlist_is_available(): return a Deferred that fires (with
    None) when the wordlist is known. This can be used to block a readline
    frontend which has just called h.choose_nameplate() until the resulting
    wordlist is known, which can improve the tab-completion behavior.
  • matches = h.get_word_completions(prefix): return (synchronously) a set of
    completions for the given words prefix. This will include a trailing hyphen
    if more words are expected. The possible completions depend upon the
    wordlist in use for the previously-claimed nameplate, so calling this
    before choose_nameplate will raise MustChooseNameplateFirstError.
    Calling this after h.choose_words() will raise AlreadyChoseWordsError.
    Given a prefix like "su", this returns a set of strings which are potential
    matches (e.g. {"supportive-", "surrender-", "suspicious-"}. The prefix
    should not include the nameplate, but should include whatever words and
    hyphens have been typed so far (the default wordlist uses alternate lists,
    where even numbered words have three syllables, and odd numbered words have
    two, so the completions depend upon how many words are present, not just
    the partial last word). E.g. get_word_completions("pr") will return
    {"processor-", "provincial-", "proximate-"}, while
    get_word_completions("opulent-pr") will return {"opulent-preclude", "opulent-prefer", "opulent-preshrunk", "opulent-printer", "opulent-prowler"} (note the lack of a trailing hyphen, because the
    wordlist is expecting a code of length two). If the wordlist is not yet
    known, this returns an empty set. All return values will
    .startwith(prefix). The frontend is responsible for sorting the results
    before display.
  • h.choose_words(words): call this when the user is finished typing in the
    code. It does not return anything, but will cause the Wormhole's
    w.get_code() (or corresponding delegate) to fire, and triggers the
    wormhole connection process. This accepts a string like "purple-sausages",
    without the nameplate. It must be called after h.choose_nameplate() or
    MustChooseNameplateFirstError will be raised. May only be called once,
    after which AlreadyChoseWordsError is raised.

All Helper functions are synchronous (which makes them easier to use from a libreadline completion helper), but some can trigger refresh requests in the background.

For our approach, we'll add three inbound API events, sent from the Helper into the WormholeCore:

  • InputHelperRefreshNameplates
  • InputHelperChooseNameplate(nameplate)
  • InputHelperChooseWords(words)

and two outbound API actions, emitted by the WormholeCore. The IO/API glue layer is responsible for delivering these to the InputHelper:

  • InputHelperGotNameplates(nameplates)
  • InputHelperGotWordlist(wordlist)

All of the API events get routed to the Input machine. InputHelperRefreshNameplates causes a Refresh message to be sent to the Lister machine, which sends a TxList to Rendezvous the next time it's connected. The server responds with a NAMEPLATES message, which is translated into a RxNameplates being sent to Lister, which sends GotNameplates to Input, which should then send InputHelperGotNameplates to the API layer.

The choose-nameplate event causes the Input state machine to move, which claims the nameplate and retrieves the wordlist (which is static for now, but eventually the server will store+deliver some "nameplate attributes" including a wordlist identifier). This triggers the got-wordlist action, which the API-side Helper object can store and use to support the completion functions. When the user finally finishes typing the code and calls h.choose_words(words), the API layer sends in InputHelperChooseWords(words), and the Input machine sends the completed code to the rest of the state machines.

@alex
Copy link
Contributor

alex commented Apr 6, 2018

FWIW I've used the crate rustyline to table completion and it's great. Example usage here: https://github.com/alex/csv-sql/blob/master/src/main.rs#L129-L148

@warner
Copy link
Collaborator Author

warner commented Apr 6, 2018

Excellent, thanks!

@warner
Copy link
Collaborator Author

warner commented May 6, 2018

I read through @copyninja's input.rs from PR #36, and, yeah, let's stick to having two separate objects. Basically the Input machine will implement Start/GotNameplates/GotWordlist/ChooseNameplate/ChooseWords/RefreshNameplates, and nothing else. It will emit the new API events listed above: InputHelperGotNameplates and InputHelperGotWordlist. The Helper (which is a separate object) will react to these events by stashing the nameplates/wordlist and making them available to whatever completion technology the frontend is using. GotNameplates might be emitted multiple times: it happens when the server response comes back, and we ask the server at least once just after the input process starts, and again each time RefreshNameplates is triggered: for a tab-completion -style interface, each time you hit TAB, it should trigger a refresh request (in the background) and then immediately display the possible completions based on the nameplates list it already had. It shouldn't wait for a response before showing completions. The GotWordlist event will only happen once, at some point after the nameplate is selected.

The Helper object will send some new API events (RefreshNameplates, ChooseNameplate, and ChooseWords), and these will be processed by the Input machine: they all map directly to InputEvent types.

I've been assuming that the InputHelper object won't be a part of our "core" crate, but maybe I'm wrong. The InputHelper has two responsibilities. One is to deal with the concurrency approach used by any particular API flavor: it needs to send an API "InputHelperRefreshNameplates" to the Core each time someone calls refresh_nameplates() on the helper, and it needs to receive the InputHelperGotNameplates event which comes back out of the API (and react by stashing the nameplates for later completion attempts). This uses the same single entrypoint as the rest of the frontend API, so I've been imagining that whoever writes that frontend object (maybe using Tokio, or threads, or whatever) is also going to write the Helper. The API events coming out of the Core have to be dispatched to various approach-specific frontend API code, but the InputHelper events must be dispatched to the helper, and since both sets of events are coming from the same place, the frontend code has to be involved.

But the other thing the InputHelper does is to figure out the possible completions, by implementing h.get_nameplate_completions(prefix) and h.get_word_completions(prefix). This is a fiddly bit of prefix-matching work, and we should provide it to the API frontend authors instead of forcing each one to write their own.

I'm not sure how this should be shaped. One possibility (which I think @copyninja is exploring) is to put the Helper inside our core, next to (or merged into) the InputMachine. In this approach, the 5-ish Helper API calls (refresh_nameplates, get_nameplate_completions, choose_nameplate, get_word_completions, choose_words) are API events. This helps with the three that have side-effects: refresh_nameplates will cause a new NAMEPLATES request to be sent to the server, so it's going to be firing some IO actions, choose_nameplate is going to CLAIM the nameplate on the server, and choose_words will send the PAKE message. But it makes the synchronous non-side-effecting APIs more difficult: get_nameplate_completions is supposed to immediately return the completions for the currently-known list of nameplates, without talking to the network, and our APIEvent approach doesn't really enable synchronous returns for anything. So the closest we could get is that if you send in an APIEvent::InputHelperGetNameplateCompletions(prefix), you can expect to see an immediate APIAction::InputHelperNameplateCompletions(list) (and nothing else). The frontend API layer could rely on this convention and provide a distinct InputHelper object to applications that would implement the same API as the Python code, hiding the sync/async differences internally.

In this approach, the core wouldn't notify the outside world about a new list of nameplates being received, because the completion code that consumes that list would live inside the core. It would only reveal something about that list when asked to do a completion.

Another approach would be to provide an InputHelperHelper type, exposed just like the Core is. The API layer that provides a Wormhole type to application code would watch for a call to input_code(), and when that happens, it would:

  • build a new wormhole_core::InputHelperHelper object
  • send an APIEvent::InputCode into the core
  • watch for APIAction events that are relevant to the helper and route them to the InputHelperHelper
  • e.g. got_nameplates causes the InputHelperHelper to stash the new list of nameplates internally
  • build a real InputHelper, which holds an internal reference to the InputHelperHelper
  • the real InputHelper has the same API as the Python code. The synchronous get-completion methods just pass-through to the internal InputHelperHelper, which returns immediate answers based on the data it has stashed earlier. The async ones get translated into APIEvents like RefreshNameplates and ChooseWords, and don't return anything. This also gives the approach-specific InputHelper a chance to handle something like when_wordlist_is_available, which returns a Deferred in the python version, and has to be a Future or something Tokio-specific in the rust API layer. when_wordlist_is_available is not something that could be owned by the approach-agnostic InputHelperHelper, because it's too dependent on the specific asynchrony technology being used. But hopefully the InputHelperHelper code could provide enough generic support to enable various InputHelper implementations to provide something useful.
  • return the InputHelper to the application

In this approach, the fussy build-all-completions logic is written once, by us, in the InputHelperHelper type. The InputHelperHelper is not a state machine, and doesn't live in the core (behind the main API/event entry point). But it still lives in the core crate. Each API layer is obligated to build their own InputHelper type, but there's not very much in it, so we're not imposing a whole lot of duplication work on these authors.

I think I'm in favor of the second approach (but I can't claim to be too sure about that yet):

  • the API-layer author has to write an InputHelper type in both cases, but in the second approach we do most of the heavy lifting in the InputHelperHelper code
  • I'm not entirely happy with how the first approach would casually impose new behavior rules on our event-driven sans-io API. Certain API events are magic, and cause specific API actions to fire, in the very same return vector, with nothing else in that vector. The frontend API layer would normally call the entrypoint prepared to act upon all of the IO and API actions that came back out, but it would have a separate codepath that calls into the same entrypoint with different expectations.
  • we could do the first approach, but put the synchronous InputHelper methods on entirely separate entrypoints (core.do_api(APIEvent) -> Vec<Action>, core.do_io(IOEvent) -> Vec<Action>, completions = core.get_nameplate_completions(prefix), completions = core.get_word_completions(prefix), except that then half the API works one way and half works differently, and the helper calls aren't even legal unless you've previously done an APIEvent::InputCode. I like having context-specific methods live on an object that you only get back in the one context where they're legal, hence the InputHelper object to manage them.

I'd like to use the second approach, but I'm trying to think about how we might make it easier to use. We could add a method to WormholeCore named input_code which sends in APIEvent::InputCode and returns both a vector of actions and an InputHelper, and give that InputHelper a reference to the core so API calls on it can send events into the core too. But that API sounds fussy, and I suspect ownership issues will happen (does the InputHelper keep a mutable reference to the core? what about the API layer's own mutable reference? does the core retain a reference to the InputHelper so it can automatically dispatch the right events? that sounds like a reference cycle wating to happen).

I have to think about this more.. I'm not yet seeing a clear path.

@warner
Copy link
Collaborator Author

warner commented May 6, 2018

Hm, actually the ownership issues might make the second approach hard to implement for API-layer authors. They can't really provide a distinct InputHelper object because it needs to interact with the same core that their Wormhole object uses. To send events into the core, you need a mutable reference, and I don't think both can hold a mutable reference at the same time.

So they might be forced to put the InputHelper methods on their Wormhole object too, with a test that makes them throw an exception if you call them before/without calling w.input_code().

Incidentally, the reason these methods are split is because I've been thinking about a GUI application using the Wormhole library. The main code would call w.input_code() and then wait for w.got_code() to fire before proceeding. It would glue the InputHelper to some GUI popup that can check for completions as much as it wants: the popup only interacts with the InputHelper, and the last thing it does before closing is to call choose_words.

@warner
Copy link
Collaborator Author

warner commented May 12, 2018

I talked with @alex at pycon today, and we decided that a "1+2" API is probably be best way forward. The main API entry point should take both the normal API events, the IO events, and the subset of InputHelper events that can provoke IO (so refresh_nameplates, choose_nameplate, when_wordlist_is_available, and choose_words). Then we add two other API methods for the synchronous/return-value InputHelper methods (get_nameplate_completions and get_word_completions). These methods aren't allowed to trigger IO, and their return signature is a vector of nameplate completions and vector of word completions, respectively.

It's not super elegant, but it's better than trying to shoehorn everything into an API event return value. The core is allowed to panic if you call the helper methods without first sending in an Input::InputCode API event.

The frontend IO layer should probably merge the InputHelper functionality into the Wormhole object, rather than putting them into a separate object, because of the ownership issues. Maybe we split out the InputHelper methods into a separate trait, to at least pretend that they're distinct. If there's a way to make it into separate object entirely, that'd be great, but for now let's just merge them.

@warner
Copy link
Collaborator Author

warner commented May 14, 2018

Also, maybe we should have a simpler completion function that combines the namplate and the words. There's an important difference between the two (you can't query for words until you've committed to the nameplate), but maybe a stateful helper which remembers how far you've gotten, and calls the choose_nameplate() at the right time.

It would mean that this helper must be able to return/process IO events, though, since choose_nameplate() causes IO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants