metosin / compojure-api

Sweet web apis with Compojure & Swagger
http://metosin.github.io/compojure-api/doc/
Eclipse Public License 1.0
1.12k stars 149 forks source link

+routes+ error after 0.12.0 -> 0.13.1 #33

Closed dryewo closed 9 years ago

dryewo commented 10 years ago

Hi Tommi,

I've upgraded from 0.12.0 to 0.13.1 (to make use of {s/Keyword s/Any}) and started getting this error:

Unable to resolve symbol: +routes+ in this context, compiling:(my_project/apis/pizzas.clj:141:13)

I have the following structure: core.clj:

(ns my-project.core
  (:use clojure.pprint)
  (:require [com.duelinmarkers.ring-request-logging
             :refer [wrap-request-logging]]
            [compojure.api.sweet :refer :all]
            [clj-stacktrace.repl]
            [my-project.apis [pizzas :as pizzas]
              [sushis :as sushis]
              ;; some more imports
              ]
            [my-project.util [middleware :as middleware]]))

(defapi app
        (middlewares [middleware/wrap-catch-exceptions]
                     (swagger-ui)
                     (swagger-docs
                       :title "my project API"
                       :apiVersion "0.4.7"
                       :description "my project description"
                       :termsOfServiceUrl "http://www.google.com"
                       :contact "info@google.com"
                       :license "Eclipse 1.0"
                       :licenseUrl "http://www.eclipse.org/legal/epl-v10.html")
                     pizzas/route
                     sushis/route
                     ;; some more routes
                     )
        (compojure.route/not-found "<h1>No.</h1>"))

(def handler
  (-> #'app
      middleware/wrap-print-request
      (middleware/wrap-access-control-allow "*" "GET, POST, PUT, DELETE")
      ;middleware/wrap-print-response
      ;; Config in src/log4j.properties
      com.duelinmarkers.ring-request-logging/wrap-request-logging
      ))

apis/pizzas.clj:

;; ...

(defmodel Pizza
  ;; ...
  )

(defroutes* route
            (swaggered "pizzas"
                       :description "Pizzas API"
                       (context "/pizzas" []
                                (GET* "/" {pms :params}
                                      :summary "Get all pizzas"
                                      :return [Pizza]
                                      (ok (get-all-pizzas pms))))))

I like it because it allows me to isolate api groups from each other.

Now I started investigating and after removing (swaggered) from pizzas.clj made it successfully compile.But pizzas disappeared from the swagger front page.

Then I added (swaggered) to core.clj like this:

(swaggered "pizzas"
  :description "Pizzas API"
  pizzas/route)

Then it gave an error about "unable to resolve Pizza", because (:require [my-project.apis [pizzas :as pizzas]]), not (:require [my-project.apis.pizzas :refer :all]). I tried to :refer :all, but it gave errors about coinciding var names (like route and DATE_FORMAT, that I have in several files).

In fact, my APIs are more complicated than this, they include many schemas which I don't want to explicitly :refer in core.clj, and there are many vars with same names across different files.

Is there a possibility in 0.13.1 to define this structure without repeating and adding boilerplate code?

Thanks!

ikitommi commented 10 years ago

hi!

the defroutes* is a good feature, but the current implementation sucks (doesn't resolve the symbols), there is already an issue out of it (https://github.com/metosin/compojure-api/issues/24). Might be easy to fix or then have to rewrite the whole walker to be namespace-aware. Now off to Euroclojure, will play with this after that.

cheers,

Tommi

dryewo commented 10 years ago

Good luck! Spread the word about compojure-api!

ikitommi commented 10 years ago

thanks! :)

ikitommi commented 10 years ago

sorry for the lag on this, tried few solutions, none of them were elegant enough. Will return to this right after vacations. Current way to solve is to put all swaggered codes into same namespace as defapi and to import all stuff there. Butt-ugly, I know. Or to use pre 0.13.0-version which has still the global state.

ikitommi commented 10 years ago

discussed how to fix this properly. Will be fixed for the upcoming Swagger 2.0 version of both ring-swagger & compojure-api. Swagger 2.0 will have much better model for collecting routes, makes this fix much easier to implement. Will start to work on that this/next week. Until then, use the old version or import all stuff into the root hander (we are doing the latter currently).

dryewo commented 10 years ago

Glad to hear that, thank you! I'm still using 0.12.0, can't wait to update.

gbuisson commented 10 years ago

Did anyone try creating a bigger API ?

With 20 routes, defapi seem to produce too much code for a single method, then we hit the java max method size limit:

Exception in thread "main" java.lang.RuntimeException: Method code too large!

ikitommi commented 10 years ago

hi.

Haven't bump into this. Despite of the namespace resolution bug, we have distributed large apis (60+ routes) with defroutes*.

Will try to find time next week to resolve this (the original issue).

gbuisson commented 10 years ago

I tried also with defroutes* and also using with-routes instead of defapi, same result:

lein uberjar

Compiling lightning.handler java.lang.RuntimeException: Method code too large! at clojure.asm.MethodWriter.getSize(MethodWriter.java:1872) at clojure.asm.ClassWriter.toByteArray(ClassWriter.java:775) at clojure.lang.Compiler.compile(Compiler.java:7382) at clojure.lang.RT.compile(RT.java:398) at clojure.lang.RT.load(RT.java:438) at clojure.lang.RT.load(RT.java:411) at clojure.core$load$fn__5066.invoke(core.clj:5641) at clojure.core$load.doInvoke(core.clj:5640) at clojure.lang.RestFn.invoke(RestFn.java:408) at clojure.core$load_one.invoke(core.clj:5446) at clojure.core$compile$fn5071.invoke(core.clj:5652) at clojure.core$compile.invoke(core.clj:5651) at user$eval7$fn14.invoke(form-init7592595820997304642.clj:1) at user$eval7.invoke(form-init7592595820997304642.clj:1) at clojure.lang.Compiler.eval(Compiler.java:6703) at clojure.lang.Compiler.eval(Compiler.java:6693) at clojure.lang.Compiler.load(Compiler.java:7130) at clojure.lang.Compiler.loadFile(Compiler.java:7086) at clojure.main$load_script.invoke(main.clj:274) at clojure.main$init_opt.invoke(main.clj:279) at clojure.main$initialize.invoke(main.clj:307) at clojure.main$null_opt.invoke(main.clj:342) at clojure.main$main.doInvoke(main.clj:420) at clojure.lang.RestFn.invoke(RestFn.java:421) at clojure.lang.Var.invoke(Var.java:383) at clojure.lang.AFn.applyToHelper(AFn.java:156) at clojure.lang.Var.applyTo(Var.java:700) at clojure.main.main(main.java:37) Exception in thread "main" java.lang.RuntimeException: Method code too large!, compiling:(/private/var/folders/4r/242337xj5x18_g14dlnwtcxc0000gn/T/form-init7592595820997304642.clj:1:4)

I tried, removing nearly all my routes, it works with just one, barely with 2 routes. I comment on this issue because my first guess is that the macro generates a single too large method.

ikitommi commented 10 years ago

I couldn't reproduce this problem. Can you provide a minimalistic setup for a failed case?

gbuisson commented 10 years ago

I fixed it by "lazy loading" the routes, with something like:

(defn -main [] (require 'myapp.handler) (let [app (eval `myapp.handler/app)](httpkit/run-server app {:port 8081 :thread 8})))

otherwise, if i pass handler directly to http-kit/run server it fails with "Method code too large!" i think i saw another issue describing this behaviour but cannot find it.

ikitommi commented 10 years ago

great to hear you got it working! We used AOT for the routes in one app with tens of api-routes, worked ok in that - but the build times were really bad and din't seem to effect much in the startup time for some reason. Might be differences in the JVMs being used. Currently use the same lazy-loading in all apps.

gbuisson commented 10 years ago

Thank you for your support, this is a nice project, keep up the good work ;-)

Deraen commented 10 years ago

The project Tommi mentioned used Jetty. We should maybe test AOT + Http-kit + large api?

gbuisson commented 10 years ago

I think it is not about ring vs http-kit, small or large API but more about aot. i tested with only 2 routes and got the same result.

In http-kit case, you have to instanciate a server in your code and pass the api, then handler code is fully evaluated and compiled, then it breaks.

when you use ring, i think the server launches but code is evaluated in a JIT fashion. thus the handler method is evaluated piece by piece and it still runs without breaking the method size limit.

if you specify full aot with ring, in my tests it also breaks.

let me know if you have the same behaviours or if you need more details.

ikitommi commented 10 years ago

I have AOTted lots of ring-apps, small and large, never had that error. If you can reproduce this (via a github repo for example), would love to investigate.

On Tue, Sep 9, 2014 at 3:00 PM, gbuisson notifications@github.com wrote:

I think it is not about ring vs http-kit, small or large API but more about aot. i tested with only 2 routes and got the same result.

In http-kit case, you have to instanciate a server in your code and pass the api, then handler code is fully evaluated and compiled, then it breaks.

when you use ring, i think the server launches but code is evaluated in a JIT fashion. thus the handler method is evaluated piece by piece and it still runs without breaking the method size limit.

if you specify full aot with ring, in my tests it also breaks.

let me know if you have the same behaviours or if you need more details.

— Reply to this email directly or view it on GitHub https://github.com/metosin/compojure-api/issues/33#issuecomment-54958898 .

gbuisson commented 10 years ago

I'll try to make a repo as soon as i have more spare time.

I'll let you know.

Thank you.

ikitommi commented 9 years ago

will be fixed in https://github.com/metosin/compojure-api/tree/swagger2routes

ikitommi commented 9 years ago

fix merged to master, will be in [0.20.0]

dryewo commented 9 years ago

wow thanks finally we have it :)