valyala / quicktemplate

Fast, powerful, yet easy to use template engine for Go. Optimized for speed, zero memory allocations in hot paths. Up to 20x faster than html/template
MIT License
3.13k stars 150 forks source link

Dynamically load pages based on string parameter #58

Open frederikhors opened 5 years ago

frederikhors commented 5 years ago

First of all thanks a lot for quicktemplate, I'm newbie and this is great: I'm learning a lot from your code. Thanks again!

(Maybe) a silly question: I'm trying to use authboss with quicktemplate but I don't know if I'm doing it well.

Authboss has just one interface (https://godoc.org/github.com/volatiletech/authboss/#Renderer) for its rendering system. As stated in its README (https://github.com/volatiletech/authboss/blob/master/README.md#rendering-views):

The renderer knows how to load templates, and how to render them with some data and that's it.

...

When your app is a traditional web application and is generating it's HTML serverside using templates this becomes a small wrapper on top of your rendering setup. For example if you're using html/template then you could just use template.New() inside the Load() method and store that somewhere and call template.Execute() in the Render() method.

There is also a very basic renderer: Authboss Renderer which has some very ugly built in views and the ability to override them with your own if you don't want to integrate your own rendering system into that interface.

If you read the code for html.go you can see Load() and Render() methods.

I started copying that code and if I understand correctly:

func (h *HTML) Load(names ...string) error {
    if h.layout == nil {
        b, err := loadWithOverride(h.overridePath, "html-templates/layout.tpl") // I use an interface for layout page with quicktemplate
        if err != nil {
            return err
        }
        h.layout, err = template.New("").Funcs(h.funcMap).Parse(string(b)) // I don't need parsing anymore
        if err != nil {
            return errors.Wrap(err, "failed to load layout template")
        }
    }
    for _, n := range names {
        filename := fmt.Sprintf("html-templates/%s.tpl", n) // this is already an object in my code, right?
        b, err := loadWithOverride(h.overridePath, filename)
        if err != nil {
            return err
        }
        clone, err := h.layout.Clone() // this is already an object in my code, right?
        if err != nil {
            return err
        }
        _, err = clone.New("authboss").Funcs(h.funcMap).Parse(string(b)) // this is already an object in my code, right?
        if err != nil {
            return errors.Wrapf(err, "failed to load template for page %s", n)
        }
        h.templates[n] = clone // maybe something like this with functions?
    }
    return nil
}
func (h *HTML) Render(ctx context.Context, page string, data authboss.HTMLData) (output []byte, contentType string, err error) {
    // -------> Original authboss one, commented now
    // buf := &bytes.Buffer{}
    // tpl, ok := h.templates[page]
    // if !ok {
    // return nil, "", errors.Errorf("template for page %s not found", page)
    // }
    // err = tpl.Execute(buf, data)
    // if err != nil {
    // return nil, "", errors.Wrapf(err, "failed to render template for page %s", page)
    // }

    // -------> Mine
    buf := &bytes.Buffer{}
    templates.WritePage(buf, &templates.LoginPage{
        Data: data,
    })
    return buf.Bytes(), "text/html", nil
}

which has the problem I cannot dynamically load pages in templates.WritePage() method based on page parameter.

LoginPage is coming from a template like this:

{% import "github.com/volatiletech/authboss" %}

{% code
    type LoginPage struct { Data authboss.HTMLData } 
%}

{% func (p *LoginPage) Title() %}
    Login
{% endfunc %}

{% func (p *LoginPage) Body() %}
    <b>Data: {%v p.Data.Something %}</b>
    <form action="/login" method="POST">
        <input type="email">
        <input type="password">
        <button>Login</button>
        {% if p.Data["modules"] != nil %} 
            Something else with modules...
        {% endif %}
    </form>
{% endfunc %}

Maybe with reflection? Really? I read everywhere reflection is really slow and I need to use something else if possible.

I tried also with something like this:

{% import "github.com/volatiletech/authboss" %}

{% code var ALL_TEMPLATES map[string]*LoginPage %}

{% code
    func init() {
        ALL_TEMPLATES = make(map[string]*LoginPage)
        ALL_TEMPLATES["login"] = &LoginPage{}
    }
%}

{% code
    type LoginPage struct { Data authboss.HTMLData } 
%}

{% func (p *LoginPage) Title() %}
    Login
{% endfunc %}

{% func (p *LoginPage) Body() %}
    <b>Data: {%v p.Data.Something %}</b>
    <form action="/login" method="POST">
        <input type="email">
        <input type="password">
        <button>Login</button>
        {% if p.Data["modules"] != nil %} 
            Something else with modules...
        {% endif %}
    </form>
{% endfunc %}

but I think something is wrong here. I don't like ALL_TEMPLATES slice way of doing this.

What do you suggest?

I can write a Wiki (for newbies like me) in this project and in the authboss one.


I already opened an issue on authboss repo: https://github.com/volatiletech/authboss/issues/239.

valyala commented 5 years ago

You can try to put all the Write* functions from quicktemplate into a global map:

var templates = map[string]func(w io.Writer, data authboss.HTMLData) {
    "login_page": templates.WriteLoginPage,
    "main_page": templates.WriteMainPage,
}

All the quicktemplate functions must accept a single arg - data authboss.HTMLData:

{% func LoginPage(data authboss.HTMLData) %}
...
{% endfunc %}

{% func MainPage(data authboss.HTMLData) %}
...
{% endfunc %}

Then the map could be used from Render func:

var bb bytes.Buffer
f := templates[page]
f(&bb, data)
return bb.Bytes(), "text/html", nil

Probably this would work, but personally I don't like this abstraction, since it adds a level of indirection - a map of template functions - which makes code less clear. Additionally it restricts template functions to accept only a single argument - data authboss.HTMLData. Probably it would be better to don't use authboss.Renderer abstraction.

frederikhors commented 5 years ago

What a great honor for me to read your answer, @valyala. Thank you so much.

I thought of a similar solution (map[string]func()) in addition to the others already written before.

And I don't like it as a solution too.

Could you explain me better what you mean by:

Probably it would be better to don't use authboss.Renderer abstraction.

I think it's the only way to use authboss (with templates). Am I wrong?

frederikhors commented 5 years ago

What do you think about the code below, @valyala?

type AuthPage interface {
    Title() string
}

type LoginPage struct {
    Data authboss.HTMLData
}

func (lp *LoginPage) Title() string {
    return "title"
}

func InitializeLoginPage(data authboss.HTMLData) AuthPage {
    return &LoginPage{Data: data}
}

type RecoverPage struct {
    Data authboss.HTMLData
}

func (rp *RecoverPage) Title() string {
    return "title"
}

func InitializeRecoverPage(data authboss.HTMLData) AuthPage {
    return &RecoverPage{Data: data}
}

func main() {
    templates := map[string]func(authboss.HTMLData) AuthPage{
        "login":   InitializeLoginPage,
        "recover": InitializeRecoverPage,
    }
    newLoginPage := templates["login"](data)
    newRecoverPage := templates["recover"](data)
}
valyala commented 5 years ago

Could you explain me better what you mean by:

Probably it would be better to don't use authboss.Renderer abstraction.

I think it's the only way to use authboss (with templates). Am I wrong?

I have no experience with authboss, so cannot add anything. If authboss cannot be used without authboss.Renderer abstraction, then it would be better using another package. I'd recommend starting with plain Go code without using any third-party packages. This usually ends up with clearer code, which has no superfluous abstractions and workaround hacks for external frameworks.

frederikhors commented 5 years ago

Ok. Thanks.

Can you tell me about the code in previous comment (https://github.com/valyala/quicktemplate/issues/58#issuecomment-516373222)?

After I can close this issue.

I tried with factory pattern. What do you think?

valyala commented 5 years ago

I tried with factory pattern. What do you think?

I think it is better to use plain old switch instead:

// WritePage write page for the given templateName and the given arg into w.
func WritePage(w io.Writer, templateName string, args interface{}) {
    switch templateName {
    case "login":
        return WriteLoginPage(w, args)
    case "recover":
        return WriteRecoverPage(w, args)
    default:
        fmt.Fprintf(w, "unknown templateName=%q", templateName)
    }
}

This switch is simpler than the map or factory method because it avoids a level of indirection or two (in case of factory method), so the code is easier to understand, update and maintain.

frederikhors commented 5 years ago

The problem

Yes, the problem is I need initializers because I'm using a single func:

{% interface PageImpl {
        Title()
        Body()
    }
%}

{% func Page(p PageImpl) %}
  <html>
    ...
  </html>
{% endfunc %}

and every authboss page has:

{% code
    type LoginPage struct {
        Data authboss.HTMLData
    }
%}

{% func (p *LoginPage) Title() %}
  Login
{% endfunc %}

{% func (p *LoginPage) Body() %}
  ...
{% endfunc %}

so I don't have func like WriteLoginPage() and WriteRecoverPage(): just WritePage().

What have I done

Now I'm using authboss.Renderer interface's Load() method:

type HTML struct {
  ...
  templates    map[string]func(authboss.HTMLData) templates.PageImpl
  ...
}

func InitializeLoginPageType(data authboss.HTMLData) templates.PageImpl {
    return &templates.LoginPage{Data: data}
}

func (h *HTML) Load(names ...string) error {
    for _, n := range names {
        switch n {
        case "login":
            h.templates[n] = InitializeLoginPageType
        }
    }
    return nil
}

func (h *HTML) Render(ctx context.Context, page string, data authboss.HTMLData) (output []byte, contentType string, err error) {
    buf := &bytes.Buffer{}
    tpl, ok := h.templates[page]
    if !ok {
        return nil, "", errors.Errorf("...")
    }
    templates.WritePage(buf, tpl(data))
    return buf.Bytes(), "text/html", nil
}

The million dollar question

Do you see problems with this code?

Do you see parts to improve?

valyala commented 5 years ago

This approach looks OK from the first sight.

frederikhors commented 5 years ago

Your advice is invaluable, thank you very much.

I am still learning Go and every problem is an opportunity to learn, but only if I am "guided" by skilled and generous people like you.

I have created two different versions of the solution and would like to know which one is best or if there is a third one better.

  1. https://github.com/frederikhors/authbossQuicktemplate_1, factory pattern and initializators

  2. https://github.com/frederikhors/authbossQuicktemplate_2, with interface and a SetData(data authboss.HTMLData) (page PageImpl) method

I think you prefer the latter, but I don't know what to improve for performances.

valyala commented 5 years ago

Both versions look almost identical. Take the simplest version which will be easier to understand and maintain in the future. As for the performance, both versions contain superfluous memory allocation when returning the byte slice from bytes.Buffer. The memory allocation could be avoided if Render could accept io.Writer to write template output to.

frederikhors commented 5 years ago

The memory allocation could be avoided if Render could accept io.Writer to write template output to.

I can ask author to get rid of it.

Can I ask you what are you using for authentication when you cannot use third-party services (e.g. auth0)?

valyala commented 5 years ago

Can I ask you what are you using for authentication when you cannot use third-party services (e.g. auth0)?

I usually abstract away the authentication into a separate package auth and then use the following code in the beginning of request handlers, which must be authenticated:

if err := auth.Authenticate(req); err != nil {
    return fmt.Errorf("authentication failure on request %s: %s", req, err)
}
frederikhors commented 5 years ago

Ok. However, I mean the technology that you usually use as an alternative to everything that authboss makes available for free (user management with 2FA, recover, registration and so on ...).

Something like devise for Rails, I don't know if you know the library.

But I think I'm going off-topic.

Just your quick answer out of curiosity and then I close.