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

Set a verb-var from the result of a request #59

Open
astoff opened this issue Aug 25, 2023 · 9 comments
Open

Set a verb-var from the result of a request #59

astoff opened this issue Aug 25, 2023 · 9 comments

Comments

@astoff
Copy link

astoff commented Aug 25, 2023

This is similar but not quite the same as https://github.com/federicotdn/verb#storing-responses-by-key.

Suppose, for instance, that one would like to extract a login token and store it in a verb-var automatically after calling a login endpoint. It seems that a nice way to do this (and many other things) would be by specifying, as a request property, a hook that is run after the request completes:

* Do login
:PROPERTIES:
:Verb-Hook: (verb-set-var 'token (verb-json-get (oref verb-last body) "access_token"))
:END:
POST www.example.com/login

What do you think?

@federicotdn
Copy link
Owner

That is a good idea! In order to make it more consistent with Verb-Map-Request, I would instead make Verb-Hook receive a function that itself receives only one parameter, the response itself. For example:

(defun example (res)
  (verb-set-var 'token (verb-json-get (oref res body) "access_token")))

And then:

* Do login
:PROPERTIES:
:Verb-Hook: example
:END:
POST www.example.com/login

A lambda could also be specified there.
I would also maybe call the property Verb-Response-Hook instead.

@astoff
Copy link
Author

astoff commented Aug 28, 2023

I think I would either define a Verb-Map-Response property that takes a response and returns an actual response to display, or a Verb-Response-Hook that takes no arguments and just performs some side-effects (relying on verb-last).

But in the end anything that allows setting a "token" variable would do the job for me :-).

@bigodel
Copy link
Contributor

bigodel commented Nov 4, 2023

A bit late to the party, but Verb-Map-Response could be implemented on the user-side with:

(defun verb--apply-response-fn ()
  "Apply function in VERB-MAP-RESPONSE property to response object."
  (when-let ((res verb-last)
             (rs (oref res request))
             (form (thread-first
                     (format "%smap-response" verb--metadata-prefix)
                     (assoc-string (oref rs metadata) t)
                     cdr
                     verb--nonempty-string))
             (fn-form (verb--try-read-fn-form form))
             (fn (cond
                  ((functionp form) form)
                  ((listp form) (eval form)))))
    (with-current-buffer (other-buffer (current-buffer) 'visible-ok)
      (condition-case nil
          (funcall fn res)
        (error (message "Couldn't parse `%s' as a function." fn))))))

(add-hook 'verb-post-response-hook #'verb--apply-response-fn)

I have been using that particular piece of code for over a year now with no hiccups as far as I remember.

@federicotdn
Copy link
Owner

federicotdn commented Nov 10, 2023

Nice! Would you mind opening a PR for it? Note that the thread-first bit can be replaced with the new function verb--request-spec-metadata-get. @bigodel

@bigodel
Copy link
Contributor

bigodel commented Nov 17, 2023

Nice! Would you mind opening a PR for it?

Definitely could, but do we really need to add it to the code base? I would think that adding a section to the README would be enough for whoever wishes to have that behaviour to add that function to the response hook.

Note that the thread-first bit can be replaced with the new function verb--request-spec-metadata-get.

LOL I added that function and forgot to update my hook :P

@federicotdn
Copy link
Owner

Yes we could add it to the README, but I feel like this function specifically is useful enough to have it integrated in the package (for example, last week I also ran into a case where it would've been useful to have it). Since Verb as a package is relatively small I don't mind adding more stuff for the moment - and I also like it being "batteries included" when possible.

@bigodel
Copy link
Contributor

bigodel commented Nov 20, 2023 via email

@bigodel
Copy link
Contributor

bigodel commented Nov 20, 2023

I have created the PR to get some early reviews, but I still need to add tests for the new function that parses forms into function objects plus fixing the tests for the previous implementation of verb--try-read-fn-form.

@federicotdn
Copy link
Owner

Amazing! Thank you, let me know when I can review it

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

3 participants