Issues mentioned on this page are all resolved, page available as archive only.
Lets collect here things to consider during the planned refactoring.
donepostfix
. relying-party.lisp uses URI postfix to identify an OpenID authentication transaction. It was really difficult to understand from the code. The word 'postfix' is uncertain, lets rename the paramether to something likeauth-transact-id
done; renamed toid
. It was difficult to understand from the code what is meant by the object named 'id'. In this object we store discovered information and other properties (return_to URL, timestamp). Naming it 'id' is confusing, because id (identifier) is expected to an URI like http://someone.livejournal.com/ (or XRI). Lets rename this object (i.e. function parameters etc.) to something like 'auth-transaction'.AUTH-PROCESS
donehandle-indirect-reply
. The term used by the spec is 'indirect response'. Lets rename the function tohandle-indirect-response
, it is better when one thing is always named by only one term.
Thread safety. Working with all global objects must be thread safe.done
Usage of package to store unique auth transaction IDs is unnecessary and it is a memory leak (interned symbols will remain in memory). To generate unique keys it is sufficient to use a counter.done
It is unclear to me whyeval-when
is used in the relying-party.lisp when defining thehtml
function and+openid-input-form+
. IMHO plain definitions will be OK here.
They won't. It is because (1) on re-evaluation of
DEFCONSTANT
form, if new value is notEQL
old value, implementation behaviour is unspecified and many implementations (including SBCL and CCL) signal an error -- see http://sbcl.sourceforge.net/manual/Defining-Constants.html . (2) theHTML
function is used in later definition in the same file, recompiling from scratch fails. Try to compile or load load following file:
(defun foo (x) (concatenate 'string "foo" x "foo")) (defconstant +foo+ (foo "test"))
I agree with rest of remarks, and will take them into account when refactoring.
--maciej
done(intern (string :cl-openid) :keyword)))
at the bottom of cl-openid.asd. We may just say:cl-openid
.