sps / mustache-spring-view

java spring framework mvc view for Mustache.js templates
68 stars 31 forks source link

ContentNegotiatingViewResolver not supported. #15

Closed wrey75 closed 7 years ago

wrey75 commented 10 years ago

Basically, the Spring Dispatcher authorizes the use of dispatching by contents. Unfortunately, your library does not support this because there is a FileNotFoundException raised when the template is not present. Then the Spring dispatcher is stopped and the other viewers have no chance to do be called.

I rewritten your code based on the Freemarker code available. I needed to do some changes, not very pretty but I think full compatible as I had success to run the modified code.

The MustacheView has changed a lot due to the copy/paste of code used in FreeMarker and then, I put the stuff (compiler, template and loader) directly in the view.

wrey75 commented 10 years ago

I have written a fix for this (linked to #12 because the root problem is the same). The tests are passed but the coverage is insufficient: 0.76 instead of 0.90. What do you suggest?

wrey75 commented 10 years ago

I just synchronized the last version and modified the code to accept the org.springframework.web.servlet.view.ContentNegotiatingViewResolver from Spring. The main changes are localized in MustacheViewResolver. in the BuildView() method, I check for missing template: in this case, I create a view without template then trapped in checkResource() method of the MustacheView. Basically, I have a preference to create a plain FileNotFoundException rather than a MustacheTemplateException having a FileNotFoundException cause but will not pass the tests, then I will keep it with minimal changes.

wrey75 commented 10 years ago

Sorry I am starting to use GitHub and this is my first pull request. Please apologize for the mess in adding such stuff.

sps commented 10 years ago

Hello,

You can still add commits to this pull request from your fork. If you could revert the pom changes and add some unit tests to verify the change in behavior, I'd be happy to accept this.

dane-king commented 10 years ago

I don't know if the chaining will work, you are not returning a null back from the resolver when the file is not found. I'm not sure where you are at, and I don't want to jump in and make changes if you are almost done, but here is the failing test

@Test public void testBuildViewReturnsNullWhenFileNotFound() throws Exception { doThrow(new MustacheTemplateException(new FileNotFoundException())).when(templateFactory).getTemplate(viewName); assertNull(viewResolver.buildView(viewName)); }

wrey75 commented 10 years ago

Hi, I will follow the advice from sps: I will create a dedicated branch and I will revert the pom changes. In addition, I will add the test case proposed by kingd9 then the patch will be perfect. Expect having this ready for next week.

dane-king commented 10 years ago

Just to be clear that is a failing test, the code still needs to be fixed to return a null

travi commented 9 years ago

Is there any update on this issue?

sps commented 7 years ago

Closing as stale. Please re-open if needed.