[cl-openid-devel] message object (was: cl-openid: API design, code finalization and nearest future plans)

[Anton Vodonosov]

on Wednesday, July 30, 2008, 2:03:04 PM Maciek wrote:

> As I wrote in yesterday's report, there's one more thing I decided to
> change.  I'd wish to turn from alists representing messages to
> OPENID-MESSAGE structure (it’s just a struct, since there’s no need
> for OOP here).  The structure would be a thin layer atop parameter
> alist, plus accessors.  This may make the code cleaner by use of
> clearly named accessors instead of sprinkling code with AGETs.  Also,
> commonly used lookups (such as namespace check to determine protocol
> version) can be moved to struct fields, which would make many alist
> lookups unnecessary.

> This is actually not a major change, but one I see really useful for
> cleaning up the code.

...

>> Please provide your comments. In your blog you mentioned some MESSAGE
>> object and I have impression that you are planing more complex
>> refractoring.

> Not really.  The OpenID protocol is concentrated on exchanging
> messages.  The messages take different forms, external encodings and
> transmission channels, but they’re essentially same thing all over.
> Protocol draws a distinct line between logic (behaviour) and data
> (message encoding, decoding, handling).

> In current code, it is not so.  The message handling code is mixed
> with logic, and done with general functions.  What I want to do is to
> separate message-handling code to its own file, with clearly named
> functions and accessors.

> Consider the fragment from auth-request.lisp:

> ,----
> | (let ((response (direct-request (aget :op-endpoint-url id)
> |                                 (acons "openid.mode" "check_authentication"
> |                                         (remove "openid.mode" parameters
> |                                                               :key #'car
> |                                                               :test #'string=)))))
> |   …)
> `----

> Protocol logic is mixed with low-level message mangling.  I want it to
> look more like this:

> ,----
> | (let ((response (direct-request (endpoint-url auth-process)
> |                                 (copy-message response
> |                                               :mode "check_authentication"))))
> |   …)
> `----

> This should also improve testability – it should be easy to write
> tests for internal message API, and it would ensure there are no
> alist-mangling-level bugs in the conversation implementation, so only
> the behaviour would need to be tested there.

OpenID messages are key-value lists, so alists are close enough...

Of course we should not sprinkle the code with AGETs, but you can defun
clearly named accessors operating directly on alists, without introducing
a MESSAGE struct:

(defun get-field (msg key)
  (aget key msg))

(defun copy-msg (msg alist-to-add)
  (append alist-to-add (remove-if (lambda (key)
                                    (member key alist-to-add
                                            :key #'car
                                            :test #'string=))
                                  msg
                                  :key #'car)))

(copy-msg response '(("mode" . "check_authentication")))

(The functions have not been tested)

Defining copy-msg to accept parameters as in your example
is not a problem too.

As for caching namespace check and other lookups into the MESSAGE
struct fields, it is unnecessary - alists in questing are short and
lookups are very cheap; also, in many cases namespace check
is performed only one, maybe two times during message lifetime,
so you will not save any performance (to be precise, you may even
lose it - parsing server parameters into message struct has some cost
too).

In sort, considering performance questions here is an exactly
"preliminary optimization".

As I see Python's open-id message class deals with some namespaces
for parameters, but this is for extensions as I understand and
we do not need it now.

I will try to think more about the messages tomorrow too.

Best regards,
-Anton