Custom variable support in Webhook URLs

[adriansmares]

Summary

Currently the final URL of a webhook is static, it does not support custom variables such as JoinEUI, DevID, DevEUI and DevAddr. The result should be that a base URL for webhook such as http://example.com/app1/wh1 can become http://example.com/app1/wh1/{{.DevEUI}} and allow the Webhook endpoint to route the uplinks better. This should also be applicable inside the message specific URLs (uplink message, downlink queued etc.)

Why do we need this?

This will allow better routing of the Webhooks and also enable us to write certain integrations (such as OpenSensors) only as WebHooks.

What is already there? What do you see now?

Static URL support - BaseURL is joined with the topic based on the uplink type, but there are no custom routing fields.

What is missing? What do you want to see?

The custom routing fields {{.AppEUI}}, {{.JoinEUI}}, {{.DevID}}, {{.DevEUI}}, {{.DevAddr}}.

Environment

Not applicable.

How do you propose to implement this?

Replace custom variables in the URL after the base URL + config URL join occurs in newRequest.

Can you do this yourself and submit a Pull Request?

Yes.

[htdvisser]

Based on how the fields look, I suspect that you were planning on implementing this with text/template. If that's indeed the case, then please be aware of the fact that parsing and executing templates (especially if it's not just the base URL but also the path) on every uplink can potentially be quite resource intensive. Also, make sure to use a very limited struct that the template will be executed with. Use only primitive types (so convert all EUIs/DevAddr to strings), avoid pointers.

[adriansmares]

I think we can cache the template itself using the Webhook ID + updated_at in order to avoid recompiling the template every time. Also it is indeed the case that only strings should be passed to the template itself (and I'd prefer we don't use anything else outside of the identifiers).

[johanstokking]

I think two options;

• Use text/template with a very simple struct specific for this use case. I.e., don't pass *ttnpb.ApplicationUp but WebhookTemplateFields with only the stuff that we support
• Use string replace on the known fields. Super fast, same result

[htdvisser]

If we go for the second, then we should probably not use the Go template syntax at all, because that would just confuse people (why the dot? is it padded with spaces?).

If we want to keep the validation rules the same, we could consider using placeholders like $app_id. That way the placeholders won't be valid in the host (because we don't want that), but they will be valid in the path (I think $ is allowed in URL paths, we need to check that).

[adriansmares]

$ is valid both as part of the host and the path as per RFC3986 section 2.2. With that being said, we are not forced to parse the host part of the url.URL.

The standard (as per RFC 6570, see 1.2 and 2.2) syntax for URI templates is {[operator]variable-list}. Since we are working in a hot spot, I do believe this is a good justification to drop the operators (since they would require extended processing), and allow only one variable. This allows us to use string.ReplaceAll without reinventing the wheel.

[johanstokking]

What do you mean with "allow one variable"?

I also think that user experience-wise, it's nicer not to use text/template, apart from the performance hit. I'm fine with PHP-style variable names.

[adriansmares]

The standard mentioned above would allow expressions such as {appEUI,devEUI}, which would not allow us to use the simple string replace. Note that I did not disallow expressions such as {appEUI}/{devEUI}, which have only one variable per capture, and are perfectly fine.

[johanstokking]

How about this one?

[adriansmares]

That one is an internal package and cannot be imported outside of the main package, but this should do it. Edit: The main package exposes the function as well.