matiwinnetou / spring-soy-view

Google Closure Templates integration with Spring MVC
Apache License 2.0
16 stars 9 forks source link

Issue 48 - Support content negotiation in SoyTemplateViewResolver #49

Closed asarkar closed 10 years ago

asarkar commented 10 years ago

1) Added content negotiation capability to SoyTemplateViewResolver. 2) Moved couple of classes to package 'view'.

asarkar commented 10 years ago

Thanks. I was planning to make the change for the if-else review comment, but it's a minor nit and since you've merged, I won't go through the trouble of creating a branch for that.

matiwinnetou commented 10 years ago

After merging your change spring-soy-view example stopped working. I dunno what is happening. Can you perhaps look at this in your spare time.

https://github.com/mati1979/spring-soy-view-example

Please note you will need to build locally a snapshot version.

I would also like to release new version but first I need to get this to work.

asarkar commented 10 years ago

I wasn't aware that there is an example else I'd have ran it locally first. Unfortunately, I'm moving and may not be able to get to fixing it quick enough. Did you notice that the SoyTemplateViewResolver and SoyView now live in a new package? If the package isn't the problem, we may have to go back to the previous version until I figure out what's wrong with the example.

matiwinnetou commented 10 years ago

Well, for some reason they are showing in an old package for me. Can you take a look, could this somehow be badly merged? I digged a bit into the problem and it seems content negotiator returns false for the example.

matiwinnetou commented 10 years ago

There seems to be something wrong with comparator: [INFO] Started Jetty Server [INFO] Starting scanner at interval of 1 seconds. [text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,/;q=0.8] canHandle:true isSupportedContentTypes(contentTypes):false Unable to handle view:soy:soy.example.index

asarkar commented 10 years ago

I need to go now but I was looking for this log statement that could explain what the comparator is doing and didn't see it in my logs. How are you configuring logging, I don't see any config files. logger.debug("Comparing supported content types with request content types: {}, {}.", supportedContentTypes, contentTypes);

matiwinnetou commented 10 years ago

I don't have a logging in sample app at the moment, it is just a simple app, I am using sys out logging :)

asarkar commented 10 years ago

On a different note, I think the example project should ideally be a module (not separate repo) that runs an integration test with the main build to prove that it works. That way, this can never happen again. I'll try to dig into this tonight but unfortunately, I'm now swamped. Sorry, I didn't know about the example project.

matiwinnetou commented 10 years ago

Actually it was a module some time ago and then I decided to extract this into a separate project. What I didn't like was that it was also released to maven central, which didn't make sense.

matiwinnetou commented 10 years ago

Don't worry, this is not burning at all.

matiwinnetou commented 10 years ago

Actually, accidentally it is good IMHO that these packages were not moved. This would break backwards compatibility. Was this a burning need to isolate code into a new package?

asarkar commented 10 years ago

No, it just felt like consistent with your project name (soy-view) to create a new package view. If you want to preserve backward compatibility, I can move them back.

asarkar commented 10 years ago

I'm actually working on the example project now and I think I see the problem. I'll post a pull once I get it fixed. The issue with module getting released to mvn Central scan be stopped. However, I'm not going to get into that now because if something else breaks, I'd not have time in the next week to fix it.

asarkar commented 10 years ago

I created a chat room if you need to discuss something real time. https://gitter.im/abhijitsarkar/spring-soy-view

matiwinnetou commented 10 years ago

At the moment, let's keep the same package and separate example project. I also think we could somehow stop maven central release but I didn't know how to, it was easier to separate the project to its own git repo.