search

zyro23 / grails-spring-websocket

93 stars 28 forks source link

cleanup #16

Closed burtbeckwith closed 10 years ago

burtbeckwith commented 10 years ago

One thing I didn't do but you should is to move ConfigUtils from grails-app/utils to src/groovy. The name of that folder is misleading. Originally it was intended to contain all of the various smaller artifact types that weren't significant enough to merit their own folders (like domains, controllers, etc.) But only Codecs ended up being put there - there weren't any other artifact types that made sense to have in core. So although you can put utility classes there and they'll get compiled, it's not really how things were designed. Not a big deal though, so feel free to do nothing :)

I also converted your README.md to Grails Wiki format and added it as plugin docs at the plugin portal: http://grails.org/plugin/spring-websocket It's as close as I could make it; the combinations of tables, bold, and monospaced font collided a lot so I had to omit the monospaced font in several instances.

zyro23 commented 10 years ago

thx.

burtbeckwith commented 10 years ago

Most casts on the right with 'as' work as regular typed variables with an implicit cast. I know that array<->collection works and I tested a couple of the changes I made here in the console to be sure. I think one place that this doesn't work is with numbers, I'm in the habit of doing stuff like "int foo = ... as Integer" and I don't remember what got me doing that, but I wouldn't double-declare like that if something hadn't happened to prompt it.

Testing just now, it looks like it works for only ints, but floats, doubles and BigDecimal all need some extra work on the right.

burtbeckwith commented 10 years ago

I guess I could have just added a link from the plugin page to your readme. There should be something there - either in-place docs or a link, but if you're going to use that as the full docs then have two is redundant.

zyro23 commented 10 years ago

i think your changes regarding config loading caused a npe during webSocketConfig configuration. i guess your intention was to avoid the use of Holders? the tricky part is that the WebSocketMessageBrokerConfigurer methods are called before the config bean itself is fully initialized (i.e. before property injection takes place). so the config object was still null at that point.

should be fixed with https://github.com/zyro23/grails-spring-websocket/commit/ad1591e9a9881d202151e382e251f0adbcc05dc7 (using constructor-based injection there now...)

burtbeckwith commented 10 years ago

Sorry about that. I originally was passing in the GrailsApplication in the constructor and getting the config from that, but I saw that you were building the config twice so I reused one with a setter. Should have put together a quick test app.