WIP: Add support for flash messages.

[jsthomas]

This work-in-progress PR adds support for flash messages to the session middleware to address issue #43. It adds these new items:

val flash_messages : middleware
val put_flash : string -> string -> request -> unit
val get_flash : request -> (string * string) list

The function put_flash takes a category and message as arguments; each category/message pair is stored in a list inside a cookie. This means flash messages are limited to 4096 bytes. The stored pairs can be retrieved on the next request (and only the next request) using get_flash. Message categories are arbitrary strings, though one could add helper functions for common categories like info, error, etc.

A new example, b-flash-message illustrates how to use the two functions.

Not yet addressed:

• No helpers to simplify working with flash messages in eml templates.
• Documentation

[aantron]

but got stuck when I tried to generalize the session and operations record types or modify the put operation.

Can flash messages be stored inside fieldls of sessions? See Dream.put_session.

[aantron]

I don't see why flash messages need all this machinery. It seems they should be either session fields, or their own simple cookies. Do we need to ever realistically store flash messages on the server, given how ephemeral they are? If not, they can just be custom cookies. Is there a need for a database back end at all for flash messages?

[jsthomas]

Thanks for your feedback. I've made some revisions to store messages in sessions instead.

[aantron]

Thanks, it looks much neater now, too!

I still want to ask about sessions vs. bare cookies. With sessions, flash messages will sometimes be stored in database (depending on the session back end). Do flash messages really need that? This has potential performance implications from committing the flash message data to storage. If we directly store flash messages in a new kind of cookie (dream.flash?), we untangle them from sessions completely, and ensure that they always stay out of the session back end. Is that enough for flash messages? Is there any reason for them to ever end up in persistent storage server-side? (I actually don't know)

[aantron]

Also, cc @tmattio.

[tmattio]

Thanks for starting this @jsthomas! 🙂

Here are my two cents after reading the PR:

• We might want to add a middleware that invalidates flash messages. With the current implementation, they are only removed when requested by the user, which allows for the weird state where old flash messages can be displayed where they have nothing to do.
• IIUC, the current PR supports setting multiple flash messages if they are of different levels. Should we support multiple same-level flash messages? Here is a relevant issue: Allow multiple flash messages on same key phoenixframework/phoenix#301
• The levels are arbitrary, we could also add a variant for the user-defined key.

I'm also of the opinion that flash messages should be dead simple. They don't need to be signed or persisted, so they can be decoupled from sessions to keep the implementation simple.

In fact, Phoenix (sorry if I'm always putting the same framework on the table, it's the one I'm most familiar with) just provides a few functions to work with Flash:

put_flash(conn, key, message)
get_flash(conn)
clear_flash(conn)

It's simiar to the API provided in this PR, so this is a good signal 👍

I personally also like APIs that offer helpers for the common levels. E.g:

val get_flash_info : request -> string option promise

[aantron]

I personally also like APIs that offer helpers for the common levels. E.g:

val get_flash_info : request -> string option promise

If we do this, I suggest factoring out flash messages to a separate library.

[jsthomas]

I spent some time researching how other frameworks store flash messages in an effort to better understand the sessions versus bare cookies issue.

Rails stores flash messages as part of the session, which means a cookie session by default (more here and here) but can also be a relational database. In Django, flash messages can be stored either in cookies or in the session, and the session backend can be a relational database (more here and here). Django actually goes a bit further, and has a "fallback" storage option where flash messages will be stored in a cookie if they fit and the (DB or memory) session otherwise. Flask appears to be similar (see here and here). The Flask docs have an explicit warning that flash messages that are too big may fail silently.

So there is a precedent for using session storage and/or a database to store this data, with cookie-based storage a consistent default. The downside to a cookie-only approach is that it limits flash messages to 4096 bytes. I struggled to think of a realistic use case where a flash message (or set of messages) needs to be that big. Maybe putting a stack trace or other instrumentation into a debug level message? Personally, I prefer see that type of data in a log.

I haven't thoroughly researched legal or security reasons that one might prefer non-cookie based flash message storage. The wikipedia article on secure cookies asserts, "even with Secure, sensitive information should never be stored in cookies, as they are inherently insecure and this flag can't offer real protection". This MDN article points out that some countries have regulations about "allowing users to opt out of receiving some or all cookies". Perhaps one might want to develop an app that uses as little cookie data as possible for security or legal reasons; I haven't encountered this myself.

Overall I lean toward the bare-cookie approach because it's simple, covers most use cases, and separates concerns (no overlap with sessions).

[aantron]

Overall I lean toward the bare-cookie approach because it's simple, covers most use cases, and separates concerns (no overlap with sessions).

I agree with this. Depending on the API, it might be easy, upon future request, to offer session storage as an option. However, let's implement in cookies only to get started.

[aantron]

Should this return a string list? Should it be possible to store multiple messages of the same level? Should get_flash return all messages of the same level or higher?

[jsthomas]

Should this return a string list? Should it be possible to store multiple messages of the same level?

Django allows you to store multiple messages of the same level; Phoenix appears to just let you store one (support for multiple messages was added then removed). Looking in the Phoenix issue linked earlier, one of the maintainers wrote:

Going through my Ruby web apps, I almost always have a single flash message per key. The caveat of multiple is it makes the single case more verbose.

This, and the fact that multi-message support was added and then later removed makes me think the single message approach is better. But I'm happy to revise if you'd prefer the other approach.

Should get_flash return all messages of the same level or higher?

I'm speculating, but I don't think most web developers treat flash messages' levels as totally ordered; they're more like labels for ensuring a particular message will be shown with relevant styling (green for info, red for error, etc.). It might have been better to choose some other word than "level", like "key" or "label".

[aantron]

This, and the fact that multi-message support was added and then later removed makes me think the single message approach is better. But I'm happy to revise if you'd prefer the other approach.

The comments since phoenixframework/phoenix#304 (comment) are about adding multiple message support, and listing workarounds.

It seems worrying that flash messages can be lost — at least for me personally, it would give me anxiety. In more "technical" terms, it makes usage of flash message non-composable, since you cannot predict, purely locally, if a message you are adding will actually be displayed, despite there not being a clear_flash.

Out of the frameworks I checked/confirmed so far, Phoenix and Laravel don't support multiple messages. Django, Flask, Express, and Rails do, and expect you to iterate over the messages. It seems that the most used frameworks (Rails, Express, Django) use the multiple-messages style.

I personally don't see how code can be "correct" and not iterate over messages, given that there are already, at the very least, multiple levels, even if with only one message each. Wouldn't you just define a sub-template for doing this iteration, and base styles on each message's "level," which you get as an "output" of get_flash rather than pass as an "input?"

Is there a reason to ever hold on to some flash messages for display in response to a later request? Do you want to e.g. choose only the info messages, and not display errors?

I think the base flash functionality of Dream should be "correct" and worry-free, and we can build a single-message getter on top of it later, if it is needed.

[aantron]

I also want to note that it is pretty easy for users to List.filter flash messages, or take the head, and it is easy to define the right helper based on a base functionality that doesn't ever lose messages. The converse is not true — it's not (easily) possible to define a multi-message behavior on top of a mutable, overwriting, single-message behavior, unless you wrap everything (not just the getter locally) so as to flatten lists into some kind of format inside strings, and parse them when retrieving.

[aantron]

If you have experience with the Phoenix style of flash messages, could you explain the advantage of

val get_flash : category -> request -> message (option or list)

over

val get_flash : request -> (category * message) list

What benefit does Phoenix/its users gain from choosing a single category, and passing it as argument?

[jsthomas]

Reading this over in the last few days, I now think there are more advantages to the multi-message approach. I thought this point against the single-message approach was especially persuasive:

it makes usage of flash message non-composable, since you cannot predict, purely locally, if a message you are adding will actually be displayed

I've revised to allow for multiple messages of the same category, and done away with fixed message categories.

[jsthomas]

Is mixing Tyxml (in results_page) and templates (in input_form) like this idiomatic? I couldn't figure out how to implement results_page using templates.

[aantron]

I'll edit this example to not use TyXML after the merge, and ping.

[jsthomas]

I'm not sure if using Yojson to serialize is the right approach.

[aantron]

It is a right approach.

[aantron]

Thanks for the PR and research!

Also thanks to @tmattio for the review.

[jsthomas]

Thanks for reviewing, I learned a lot!