rjmetrics / sweet-liberty Goto Github PK

Hi,

when posting a document, and passing on an ID in the document (rather than letting it auto-generate by the DB), I receive a 500 INTERNAL SERVER ERROR and see a stacktrace.

I think the desired behaviour would be a 409 CONFLICT and no stacktrace.

Calling str on something like {:id 4} will result in:

com.rjmetrics.sweet-liberty.db {"raw-query":["SELECT email, id FROM users WHERE id = ?","1"]}

Which throws an error in PSQL, it's not as forgiving as MySQL:

ERROR:  operator does not exist: integer = character varying at character 38
HINT:  No operator matches the given name and argument type(s). You might need to add explicit type casts.

Hi,

I set up a sample project, by doing copy and paste from the sweet-liberty-example.

The GET works fine, now I'm trying to implement the POST.

It also works fine when I use URL-encoded form params in the body, but I prefer to send a JSON.

So I set the Content-Type to application/json, and put a json (which looks the same like the one I receive from the GET) in the body.

Then I get a stacktrace with an empty INSERT statement:

INSERT INTO CustomDocumentTemplateEntity ( ) VALUES ( )

Looking into the handlers.clj of sweet-liberty, I see this:

(defn make-create-entity-fn
  "Create a function that creates a new entity in storage and returns the new
  primary key value on the ctx at ::post-id."
  [table db-spec name-transforms input-transform _ conditions]
  (fn [{{form-params :form-params body-params :body-params} :request :as ctx}]
    (let [data (keywordize-keys (or body-params form-params))

So I assume for that to work, the data must be either in the :form-params or in the :body-params field of the request.

I'm using ring.middleware.json/wrap-json-body, which packs the data into :body (but not into :body-params), which in my opinion is correct.

Should the code maybe be like this instead? (Haven't tested it yet, only a suggestion)

  (fn [{ {:keys [form-params body-params body]} :request :as ctx}]
    (let [data (keywordize-keys (or body-params form-params body))

Or do I miss something in the setup that re-wraps the json into body-params? I made sure I added wrap-params, but don't think it would help here?

Friend the Clojure authentication library attaches a collection of authentication fn / handlers which sweet-lib throws up on trying to parse as JSON.

Example:

[false false true false]
parsing: {}
[false false true false]
parsing: {}
[false false true false]
parsing: nil
[false false false true]
parsing: nil
[false false false true]
parsing: {}
[false false true false]
parsing: nil
[false false false true]
parsing: {}
[false false true false]
parsing: /users
[false true false false]
parsing: {}
[false false true false]
parsing: {:default-landing-uri /app, :login-uri /login, :credential-fn #object[budget_om.server$fn__28154 0x699d4a7c budget_om.server$fn__28154@699d4a7c], :workflows [#object[cemerick.friend.workflows$interactive_form$fn__19471 0x39b10857 cemerick.friend.workflows$interactive_form$fn__19471@39b10857]], :unauthenticated-handler #object[budget_om.server$fn__28152 0x4dee8226 budget_om.server$fn__28152@4dee8226]}
[false false true false]
parsing: /app
[false true false false]
parsing: /login
[false true false false]
parsing: #object[budget_om.server$fn__28154 0x699d4a7c budget_om.server$fn__28154@699d4a7c]
[false false false false]
parsing: [#object[cemerick.friend.workflows$interactive_form$fn__19471 0x39b10857 cemerick.friend.workflows$interactive_form$fn__19471@39b10857]]
[false false true false]

That last line [#object[cemerick.friend.workflows$interactive_form$fn__19471 0x39b10857 cemerick.friend.workflows$interactive_form$fn__19471@39b10857]] is a Vector of 1 object which cannot be JSON parsed as is.

Resulting in:

{:status 500, :headers {"Content-Type" "application/edn;charset=UTF-8", "Vary" "Accept", "Set-Cookie" ("ring-session=d22473ed-4e13-4a90-943f-3d2d9a669118;Path=/;HttpOnly"), "X-XSS-Protection" "1; mode=block", "X-Frame-Options" "SAMEORIGIN", "X-Content-Type-Options" "nosniff"}, :body "{\"exception-message\":\"Don't know how to write JSON of class cemerick.friend.workflows$interactive_form$fn__19471\"

Was able to get around this with:

(defn json-value-fn
  ([v]
   (if (coll? v)
     (apply str (map json-value-fn v)))
   (if (or (number? v) (string? v) (nil? v))
     v
     (str v)))
  ([_ v] (json-value-fn v)))

Would it be possible to add in support for SQL Like commands? I use it all the time to enable easy user workflow for searching paged data tables.

It has come up a few times that we needed the add-exists handler to produce a query clause that checks multiple values. Most notably if a resource is nested like /client/:client-id/resource/:id. Using (add-exists :id) allows an :id being returned that isn't associated with :client-id. The connections service uses two different ways to get around this. It has an input-transform that assoc's the :client-id into the key sweet-lib pulls from to generate the query and it also uses the :defaults sweet-lib key to assoc in a namespace to restrict queries for (/clients/:client-id/:namespace/connections). I think neither of these approaches are optimal and instead the add-exists handler should allow a variable number of arguments.

Right now, sweet-lib always returns a vector of results for expansions. In the case of one-to-one relationships, this means that you either have to use a controller to extract the single result, or push that logic out to clients.

It would be nice if sweet-lib could figure out or be told that the expansion was a one or many relationship and it DTRT.

Currently throws a gnarly stack trace that just confuses the issue. A 415 response with a message body describing acceptable formats will save people time debugging.

Hey,

another thing - you're using j/query to retrieve the actual data from the database.

When the optional parameter :identifiers is not set, it uses clojure.string/lower-case by default, which in my opinion is not exactly what you would want (by default).

I agree you wouldn't want to rely on the database to return the keys in the correct casing, but how about restoring it by using the keys from the table attributes? (By looking up the returned field value using the lowercase version of the field name, but returning a map with the original casing?)

Paging queries don't work with Microsoft SQL Server.

Database spec is as follows. Queries are working as well as filtered queries. The same issue should also occur when the classname is "com.microsoft.sqlserver.jdbc.SQLServerDriver", but I haven't tested that.

(def db-spec {:classname "net.sourceforge.jtds.jdbc.Driver"
              :subprotocol "jtds:sqlserver"
              :subname "subname"
              :user "user"
              :password "password"})

Looking at the tests, it looks like the paging queries are in the form

SELECT column_name, another_column_name, third_column 
FROM my_table 
ORDER BY column_name 
ASC LIMIT _pagesize OFFSET _page

Microsoft SQL Server 2005 and 2008 requires the queries in the form

SELECT * FROM
(SELECT ROW_NUMBER() OVER(ORDER BY column_name DESC) AS
rownum, column_name, another_column_name, third_column 
FROM my_table ) AS my_table
WHERE rownum BETWEEN (_page * _pagesize) AND ((_page+1) * _pagesize)
(
ORDER BY rownum DESC

Microsoft SQL Server 2012 requires the queries in the form (taken from http://raresql.com/2012/07/01/sql-paging-in-sql-server-2012-using-order-by-offset-and-fetch-next/ as I don't have a 2012 server handy to test)

SELECT column_name, another_column_name, third_column 
FROM my_table 
ORDER BY column_name 
OFFSET (_page*_pagesize) ROWS
FETCH NEXT (_pagesize) ROWS ONLY

Any idea if this is something that should be fixed in Honey SQL or handled in Sweet-Liberty?

I love the idea of expansions, and was wondering if there was a way that might make sense to enable an easier configuration of expansions within the same sweet-liberty project.

Here's a example of a simple resource configuration where it might come in handy.

{:countries {:table {:table-name :countries
                     :attributes [:code
                                  :name]
                     :primary-key :code}}

:provinces {:table {:table-name :provinces
                     :attributes [:id
                                  :id_country
                                  :name]
                     :primary-key :id}}

:cities {:table {:table-name :cities
                  :attributes [:id
                               :id_province
                               :name]
                  :primary-key :id}}}

At least from the way my existing databases are set up, this would bring a couple potential benefits

1. For databases with lots of joins and views, it would remove the need to define the views separately
2. No need to define service brokers for expansions contained within the same project, which takes away the need to define the path (which may be changing during development)
3. Queries can be created in honey sql to all hit the DB at the same time rather than having to do 2 separate api calls

It would be pretty easy from my perspective to just add a map to expandable attributes pointing to the table that they expand to, something along the lines of

:cities {:table {:table-name :cities
                  :attributes [:id
                               :id_province {
                                             :expansion {
                                                         :table-name :provinces
                                                         :foreign-key :id}
                               :name]
                  :primary-key :id}}}

Does that seem like it would be at all desirable, or am I just complicating things more than it's worth and we should just rely on service broker expansion for that type of use case?

I have put together a simple service and I am getting this error:

How do i Debug this SQL error to figure out what is going on?

{
"exception-message": "ERROR: syntax error at or near ")"\n Position: 23",
"stack-trace": [
"org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2198)",
"org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:1927)",
"org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:255)",
"org.postgresql.jdbc2.AbstractJdbc2Statement.execute(AbstractJdbc2Statement.java:561)",
"org.postgresql.jdbc2.AbstractJdbc2Statement.executeWithFlags(AbstractJdbc2Statement.java:419)",
"org.postgresql.jdbc2.AbstractJdbc2Statement.executeUpdate(AbstractJdbc2Statement.java:365)",
"clojure.java.jdbc$db_do_prepared_return_keys$exec_and_return_keys__1410.invoke(jdbc.clj:692)",
"clojure.java.jdbc$db_do_prepared_return_keys.invoke(jdbc.clj:707)",
"clojure.java.jdbc$multi_insert_helper$fn__1463.invoke(jdbc.clj:897)",
"clojure.core$map$fn__4245.invoke(core.clj:2559)",
"clojure.lang.LazySeq.sval(LazySeq.java:40)",
"clojure.lang.LazySeq.seq(LazySeq.java:49)",
"clojure.lang.RT.seq(RT.java:484)",
"clojure.core$seq.invoke(core.clj:133)",

we would need to ability to support composite keys and we are using non IDENTITY/generated fields so the insert! call is returning null.

See final portion of this issue for some context:

#2

Experienced this error:

clj-commons/camel-snake-kebab#15

Saw that bumping the version to 3.2 fixed it for me.

A nice feature would be to add support for Swagger similar to compojure-api. Since You're already working with SQL table definitions it's possible to get the data types for the request/response from the schema. It would be nice to be able to automatically generate a page such as this for each service.

Although I think the :indices parameter is a well-intentioned way to determine field filtering and sorting, I think it's going to cause more confusion than it solves. What if I have a composite index? What if I use a hashmap rather than a btree? As a user, I'd rather just be able to specify directly which fields should be sortable and which should be filterable. This also allows the code to serve as very clear documentation for allowable parameters.

I suggest we supercede :indices with two new params: :sort-fields and :filter-fields. They are both whitelists, if they are unspecified then no fields are sortable or filterable.