jooby icon indicating copy to clipboard operation
jooby copied to clipboard

ModelAndView that is not a Map

Open satsen opened this issue 10 months ago • 6 comments

I am using the jte.gg templating engine and it allows your model to be a class, so if I have a class called Model and want to pass this to ModelAndView I cannot do so because ModelAndView only takes a Map<String, Object>.

~~I tried to pass a Map.of("model", model) but the JTE file does not recognize this, the @param value becomes null. I am using precompiled templates with JTE.~~ EDIT: That was due to another bug which I have submitted a fix for now, #3300.

satsen avatar Sep 02 '23 21:09 satsen

found this too, but not sure how to fix it without introducing a break change. So probably this will be for 3.1 at least

Have you look here https://jooby.io/modules/jte/?

@param String name

<p>Hello ${name}!</p>
import io.jooby.jte.JteModule;

{
  install(new JteModule(Paths.of("src", "main", "jte")));         (1)

  get("/", ctx -> {
    return new ModelAndView("hello.jte", Map.of("name", "Jte"));  (2)
  });
}

jknack avatar Sep 04 '23 00:09 jknack

Probably a better solution for JTE 3.0 and greater is to use https://github.com/casid/jte/tree/main/jte-models

And to do what Rocker and JStachio do of returning the model instance directly instead of ModelAndView.

agentgt avatar Sep 04 '23 15:09 agentgt

need to look into models, yea. But we need something to mark it requires a template engine and not a message encoder... they work more or less the same in jooby, except we allow template engines to have more than one due they work by type (modelAndView) and by file extension

jknack avatar Sep 04 '23 15:09 jknack

There is also the ResultHandler way as well. I was unaware of that way till I looked at Rocker's Jooby integration.

To be honest I'm not really sure when the ResultHandler way is picked over the MessageEncoder but from experimentation it appears that MVC picks MessageEncoder and programmatic/lambda picks ResultHandler. Is that right?

The ResultHandler seems to have a pro in that a buffer can be reused since it uses ByteBuffer instead of byte[] as well as more guarantees that the buffer is not shared I think. This is sort of related to #2100 .

agentgt avatar Sep 05 '23 18:09 agentgt

Yeah @jknack it's pretty simple to do it in a backwards-compatible way. Just create a superclass for ModelAndView or interface that ModelAndView implements (name suggestions: CustomModelAndView or AnyModelAndView) and put the method Object getModel() there. You can decide if you want getModel() to have the return type Object or a type parameter T from the new superclass/interface, as Java allows subclasses to have return types that are subclasses of the parent method's return type.

Then of course you need to change the code and API to take this new superclass/interface as a param but that is backwards-compatible.

But if you want to do another method using a breaking change that is a possibility as well

satsen avatar Sep 08 '23 13:09 satsen

@satsen FWIW that is more or less what we do albeit not for the exact reasons. Our need is to hand over some attributes that our bound to the request to the model.

We have custom TemplateEngine and either extend the provided ones or make our own:

public interface ContextAwareModel {

    Map<String, @Nullable Object> getModel();

    default Map<String, @Nullable Object> getModel(
            Context ctx) {
        return addAttributes(ctx, getModel());
    }

    public static Map<String, @Nullable Object> addAttributes(
            Context ctx,
            Map<String, @Nullable Object> oldModel) {
        Map<String, @Nullable Object> model = new HashMap<>(ctx.getAttributes());
        model.putAll(oldModel);
        return model;
    }

}
//roughly... some things omitted
class CustomHandlebarsTemplateEngine implements TemplateEngine {

     @Override
    public String render(
            Context ctx,
            ModelAndView modelAndView)
            throws Exception {
        Template template = handlebars.compile(modelAndView.getView());
        Map<String, @Nullable Object> model;
        if (modelAndView instanceof ContextAwareModel cam) {
            model = cam.getModel(ctx);
        }
        else {
            model = ContextAwareModel.addAttributes(ctx, modelAndView.getModel());
        }
        var m = com.github.jknack.handlebars.Context.newBuilder(model)
            .resolver(
                    CustomMapValueResolver.CUSTOM_MAP_VALUE_RESOLVER,
                    RecordValueResolver.INSTANCE,
                    CustomJavaBeanValueResolver.INSTANCE,
                    EnumNameValueResolver.INSTANCE)
            .build();
        return template.apply(m);
    }

As you noted in your case though you will be returning something else other than a Map for getModel.

agentgt avatar Sep 08 '23 14:09 agentgt

Changes:

  • <T> getModel()
  • Remove all put method from ModelAndView
  • Replace ModelAndView(String) by ModelAndView.map(String)

See #3395

jknack avatar Mar 18 '24 00:03 jknack

Thank you @jknack. Honestly I think it would have been better to make a <T> getModel() superclass for ModelAndView and keep make ModelAndView extend that with OtherModel<Map<String, Object>> to keep backwards compatibilty but it is your choice and also too late now.

satsen avatar May 17 '24 14:05 satsen