Re: Refactor elpa/oauth2.el to use plist over cl-defstruct

[Xiyue Deng]

Soham Gumaste <sohamg2@gmail.com> writes:

> Hello again!
>
>> 
>> I will report back in this thread with a proof of concept soon, with the 
>> current cl-defstruct implementation. We can discuss further based on that.
>> 
>> Thanks
>
> Here is a first-draft diff I have for now. I still need to work on 
> making it match the ":port" key properly. As you can see in the diff, 
> storing the struct as a plist generates a lot of ugly boiler plate code.
>
> Please note that this is not ready to merge yet.
>

I think the patch format was broken by the mail.  Maybe it's better to
send it as an attachment?

> --- Begin Diff ---
> diff --git a/oauth2.el b/oauth2.el
> index 87e0c39..5bb35af 100644
> --- a/oauth2.el
> +++ b/oauth2.el
> @@ -3,6 +3,7 @@
>   ;; Copyright (C) 2011-2021 Free Software Foundation, Inc
>
>   ;; Author: Julien Danjou <julien@danjou.info>
> +;; Author: Soham S Gumaste <sohamg2@gmail.com>
>   ;; Version: 0.17
>   ;; Keywords: comm
>   ;; Package-Requires: ((cl-lib "0.5") (nadvice "0.3"))
> @@ -174,7 +175,7 @@ TOKEN should be obtained with `oauth2-request-access'."
>       auth-url client-id scope state redirect-uri)
>      redirect-uri))
>
> -(defcustom oauth2-token-file (concat user-emacs-directory "oauth2.plstore")
> +(defcustom oauth2-token-file (concat user-emacs-directory "oauth2.plist")

As mentioned in the previous mail, this could break existing use of
clients.  As this is a defcustom, people can just change to their
liking.  Or maybe check whether there is an existing file before using
the new default?

>     "File path where store OAuth tokens."
>     :group 'oauth2
>     :type 'file)
> @@ -185,8 +186,9 @@ This allows to store the token in an unique way."
>     (secure-hash 'sha512 (concat auth-url token-url scope client-id)))
>
>   ;;;###autoload
> -(defun oauth2-auth-and-store (auth-url token-url scope client-id 
> client-secret &optional redirect-uri state)
> -  "Request access to a resource and store it using `plstore'."
> +(defun oauth2-auth-and-store (auth-url token-url scope client-id 
> client-secret &optional redirect-uri state user-data)
> +  "Request access to a resource and store it using `plstore'.
> +`user-data' is a plist with keys :host, :user, and :port as expected by 

I would prefer to store those in authinfo, as I mentioned in the
previous, those are not ephemeral data.

> `auth-sources'."
>     ;; We store a MD5 sum of all URL
>     (let* ((plstore (plstore-open oauth2-token-file))
>            (id (oauth2-compute-id auth-url token-url scope client-id))
> @@ -211,6 +213,23 @@ This allows to store the token in an unique way."
>                                         ,(oauth2-token-access-token token)
>                                         :refresh-token
>                                         ,(oauth2-token-refresh-token token)
> +                                      :secret
> +                                      (lambda (&rest unused) ;; This is 
> called with `funcall', but somehow gets args, instead of nil?
> +                                        (let ((saved-tok 
> (make-oauth2-token :plstore (plstore-open ,(plstore-get-file 
> (oauth2-token-plstore token)))

The formatting looks broken after this line.

> + 
>      :plstore-id ,(oauth2-token-plstore-id token)
> + 
>      :client-id ,(oauth2-token-client-id token)
> + 
>      :client-secret ,(oauth2-token-client-secret token)
> + 
>      :access-token ,(oauth2-token-access-token token)
> + 
>      :refresh-token ,(oauth2-token-refresh-token token)
> + 
>      :token-url ,(oauth2-token-token-url token)
> + 
>      :access-response ,(oauth2-token-access-response token))))
> +                                          (oauth2-token-access-token 
> (oauth2-refresh-access saved-tok))))
> +                                      :host
> +                                      ,(plist-get user-data :host)
> +                                      :user
> +                                      ,(plist-get user-data :user)
> +                                      :port
> +                                      ,t
>                                         :access-response
>                                         ,(oauth2-token-access-response 
> token)))
>           (plstore-save plstore)
>

-- 
Regards,
Xiyue Deng

signature.asc
Description: PGP signature