marijnh / Postmodern

A Common Lisp PostgreSQL programming interface
http://marijnhaverbeke.nl/postmodern
Other
400 stars 90 forks source link

:col-import function wants 2 arguments #278

Closed helmutkian closed 3 years ago

helmutkian commented 3 years ago

Functions passed to :col-import seem to require 2 arguments, contrary to what's documented. I am using the latest version of Postmodern from Quicklisp: postmodern-20210630-git

(defclass project ()
  (;; ...
   (build-schedule :initarg :build-interval
           :accessor build-schedule
           :initform :null
           :col-type (or pomo:text pomo:db-null)
           :col-import import-build-schedule
           :col-export export-build-schedule)))
  (:metaclass pomo:dao-class)
  (:table-name project)
  (:keys id))

(defun import-build-schedule (json-str-or-null)
  (if (eql json-str-or-null :null)
      :null
      (json-bind ((minute "minute" :optional t :default t)
          (step-minute "stepMinute" :optional t :default 1)
          (hour "hour" :optional t :default t)
          (step-hour "stepHour" :optional t :default 1)
          (day-of-week "dayOfWeek" :optional t :default t)
          (step-dow "stepDOW" :optional t :default 1)
          (day-of-month "dayOfMonth" :optional t :default t)
          (step-dom "stepDOM" :optional t :default 1)) (jsown:parse json-str-or-null)

      (list :minute (deserialize-interval minute)
         :step-minute step-minute
         :hour (deserialize-interval hour)
         :step-hour step-hour
         :day-of-week (deserialize-interval day-of-week)
         :step-dow step-dow
         :day-of-month (deserialize-interval day-of-month)
         :step-dom step-dom)))))

When calling (pomo:get-dao 'project project-id) I get an error that import-build-schedule is being called with 2 arguments. On inspection, they appear to be the project instance and the data in the build_schedule column. In order to set the value, had to modify import-build-schedule as such

(defun import-build-schedule (project json-str-or-null)
  (if (eql json-str-or-null :null)
      :null
      (json-bind ((minute "minute" :optional t :default t)
          (step-minute "stepMinute" :optional t :default 1)
          (hour "hour" :optional t :default t)
          (step-hour "stepHour" :optional t :default 1)
          (day-of-week "dayOfWeek" :optional t :default t)
          (step-dow "stepDOW" :optional t :default 1)
          (day-of-month "dayOfMonth" :optional t :default t)
          (step-dom "stepDOM" :optional t :default 1)) (jsown:parse json-str-or-null)
    (setf (build-schedule project)
          (list :minute (deserialize-interval minute)
            :step-minute step-minute
            :hour (deserialize-interval hour)
            :step-hour step-hour
            :day-of-week (deserialize-interval day-of-week)
            :step-dow step-dow
            :day-of-month (deserialize-interval day-of-month)
            :step-dom step-dom)))))
sabracrolleton commented 3 years ago

Thank you for the bug report. I think I know where it is happening, but I cannot replicate how you got there. Can you do me a favor? In the file postmodern/table.lisp starting on line 673 you can find the dao-from-fields function. Can you comment that out and insert the following?

(defun dao-from-fields (class column-map query-fields
                              result-next-field-generator-fn)
  (let ((instance (allocate-instance class)))
    (loop :for field :across query-fields
          :for writer := (cdr (assoc (field-name field)
                                     column-map
                                     :test #'string=))
          :do
          (etypecase writer
            (null (if *ignore-unknown-columns*
                      (funcall result-next-field-generator-fn field)
                    (error "No slot named ~a in class ~a. DAO out of sync with table, or incorrect query used."
                           (field-name field) (class-name class))))
            (symbol (setf (slot-value instance writer)
                          (funcall result-next-field-generator-fn field)))
            (function (let ((import-function-symbol
                             (find-import-function instance (field-name field))))
                        (cond ((and import-function-symbol
                                    (eq writer
                                        (fdefinition import-function-symbol)))
                               (format t "dao-from-fields 1~%")
                               (setf (slot-value instance (field-name-to-slot-name
                                                           class (field-name field)))
                                     (funcall writer
                                              (funcall result-next-field-generator-fn field))))
                              ((and import-function-symbol
                                    (not (functionp import-function-symbol)))
                               (format t "dao-from-fields 2~%")                               
                               (setf (slot-value instance (field-name-to-slot-name
                                                           class (field-name field)))
                                     (funcall (fdefinition import-function-symbol)
                                              (funcall result-next-field-generator-fn field))))
                              ((and import-function-symbol
                                    (functionp import-function-symbol))
                               (format t "dao-from-fields 3~%")
                               (setf (slot-value instance (field-name-to-slot-name
                                                           class (field-name field)))
                                     (funcall import-function-symbol
                                              (funcall result-next-field-generator-fn field))))
                              (t
                               (format t "dao-from-fields 4~%")
                               (funcall writer instance
                                          (funcall result-next-field-generator-fn field))))))))
    (initialize-instance instance)
    instance))

Can you tell me if that works with only one parameter and, if so, which logging format string was called?

helmutkian commented 3 years ago

@sabracrolleton Thanks for the quick response. I will try to recreate with the modified code this weekend and report the results.

sabracrolleton commented 3 years ago

Ok. I am pretty sure I found the problem. I did not take the possibility that something would change you out of your current package into account. (For example, if you are using hunchentoot easy handlers, the current package actually changes to cl-user.) Does this sound like something that might be happening in your situation?

Working on tests now and should have something for you to test with the documented one parameter version tomorrow.

helmutkian commented 3 years ago

Yup, that’s exactly the scenario. The function is being called from a Hunchentoot handler.

On Jul 10, 2021, at 1:47 PM, Sabra Crolleton @.***> wrote:

 Ok. I am pretty sure I found the problem. I did not take the possibility that something would change you out of your current package into account. (For example, if you are using hunchentoot easy handlers, the current package actually changes to cl-user.) Does this sound like something that might be happening in your situation?

Working on tests now and should have something for you to test with the documented one parameter version tomorrow.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

sabracrolleton commented 3 years ago

Before I push this to the main line, I want to make sure it works for you (export and single parameter import functions)

I uploaded changes to my personal fork at https://github.com/sabracrolleton/Postmodern. The only file you need to care about is https://github.com/sabracrolleton/Postmodern/blob/master/postmodern/table.lisp.

The commit is shown at https://github.com/sabracrolleton/Postmodern/commit/3342af623760a7fd64bbbfa5e52be696136f4b9a (but the commits to table.lisp (the only ones relevant to you) is about 80% down the page past a bunch of documentation changes).

sabracrolleton commented 3 years ago

Resolved with today's commit.

helmutkian commented 3 years ago

Sorry for the delayed response. The issue is resolved for me with the update. Thank you for your hard work!