change: Don't require specifying update type for watchers with labels

[cheerfulstoic]

So if you have a watcher using the label option like this:

     {MyApp.Accounts.User, :updated, trigger_columns: [:email, :phone], label: :user_contact_info_updated},

Before you would have:

EctoWatch.subscribe(:user_contact_info_updated, :updated)
# or
EctoWatch.subscribe(:user_contact_info_updated, :updated, package.id)

def handle_info({:updated, :user_contact_info_updated, %{id: id}}, socket) do

Now you don't have the update type (:inserted / :updated / :deleted) because the label is the fully identifying piece of information:

EctoWatch.subscribe(:user_contact_info_updated)
# or
EctoWatch.subscribe(:user_contact_info_updated, package.id)

def handle_info({:user_contact_info_updated, %{id: id}}, socket) do

Labels can be used whenever, but they are required when using the trigger_columns option or when specifying a map instead of an ecto schema (just added in #18)

[cheerfulstoic]

Would love feedback from recent contributors @frerich and @barrelltech on this since it's a breaking change

[cheerfulstoic]

Something the occurs to me that maybe should happen with this update. When not using a label, perhaps the schema module / update type should be combined in a tuple so that the message is always a 2-element tuple:

def handle_info({{MyApp.Accounts.User, :inserted}, %{id: id}}, socket) do

🤷 ?

This would also mean that the messages would jive with how the subscribe function works.

[frerich]

I like the idea of repeating the schema/action tuple used in the watchers in the message, i.e. if you have

watchers: [
     # ...
     {MyApp.Accounts.User, :updated, trigger_columns: [...]}
     # ...
   ]}

then you would get calls to

def handle_info({{MyApp.Accounts.User, :updated}, _columns}, socket) do

Similarly, when specifying a schema/table manually as in

watchers: [
    {
      %{
        table_name: "comments",
        primary_key: :ID,
        columns: [:title, :body, :author_id, :post_id],
        association_columns: [:author_id, :post_id]
      }, :updated, extra_columns: [:post_id]
    }

it would result in message to

def handle_info({%{table_name: "comments", :updated}, _params}, socket) do

In fact, would this remove the need to have a notion of labels completely? When would you ever specify a label if this is possible?

[cheerfulstoic]

The thing is that somebody could specify two conflicting table definitions, like:

    {
      %{
        table_name: "comments",
        primary_key: :comment_id,
        columns: [:title, :body, :author_id, :post_id],
        association_columns: [:author_id, :post_id]
      }, :updated, extra_columns: [:post_id]
    },
    {
      %{
        table_name: "comments",
        primary_key: :the_id,
        columns: [:title, :description, :author_id, :post_id],
        association_columns: [:author_id, :post_id]
      }, :updated, extra_columns: [:post_id]
    }

The table_name isn't a unique piece of the whole table specification, and sending the whole table specification in the message and expecting the handle_info to match on it seems unreasonable...

I feel like labels are quite important. Firstly they are required by ecto_watch when using the trigger_columns option because you could potentially want to handle messages from the same table but with different triggers:

     {MyApp.Accounts.User, :updated, trigger_columns: [:email, :phone], label: :user_contact_info_updated},
     {MyApp.Accounts.User, :updated, trigger_columns: [:password_hash], label: :user_password_updated},

But even in cases where label isn't required you still might want to have two different separate messages that you'd want to handle:

     {MyApp.Accounts.User, :updated, extra_columns: [:email, :phone], label: :user_updated_with_contact_info},
     {MyApp.Accounts.User, :updated, trigger_columns: [:name, :role], label: : user_updated_with_identifying_info},

This way different processes could subscribe to updates without having everything in the messages.

[frerich]

The thing is that somebody could specify two conflicting table definitions

That's a good point, I didn't consider that!

sending the whole table specification in the message and expecting the handle_info to match on it seems unreasonable...

Here's a crazy idea: how about aligning the configuration and the message format such that the entire table definition as written in the watchers option is sent with the message? Hear me out:

Each message sent by ecto_watch could be a 3-tuple consisting of

1. The atom :ecto_watch
2. The tuple describing the table definition
3. The map of column values sent by the database trigger.

E.g. given a table definition (based on an Ecto schema) as in

watchers: [
     {MyApp.Accounts.User, :inserted}
]

The handle_info/2 clause would change from

-def handle_info({:inserted, MyApp.Accounts.User, %{id: id}}, socket) do
+def handle_info({:ecto_watch, {MyApp.Accounts.User, :inserted}, %{id: id}}, socket) do

With this change, more complex table definition lists like in your example:

    {
      %{
        table_name: "comments",
        primary_key: :comment_id,
        columns: [:title, :body, :author_id, :post_id],
        association_columns: [:author_id, :post_id]
      }, :updated, extra_columns: [:post_id]
    },
    {
      %{
        table_name: "comments",
        primary_key: :the_id,
        columns: [:title, :description, :author_id, :post_id],
        association_columns: [:author_id, :post_id]
      }, :updated, extra_columns: [:post_id]
    }

...would naturally work the same way, the second tuple element of the :ecto_watch message is the table definition - but thanks to pattern-matching, users only need to match on as many fields as necessary to disambiguate. E.g.

def handle_info({:ecto_watch, {%{table_name: "comments", primary_key: :comment_id}, _action, _opts}, columns}, socket)
def handle_info({:ecto_watch, {%{table_name: "comments", primary_key: :the_id}, _action, _opts}, columns}, socket)

If there is just one table definition, no matter how complicated, users could even just use

def handle_info({:ecto_watch, _table_def, columns}, socket)

I.e. the value of the table definition tuple is used as the 'name' of the watcher, users can pattern-match in as much detail as needed to disambiguate.

This might remove the necessity for a 'label' notion -- and it would also align the order of values in the message with what's used in the configuration: right now, in the configuration, you would do

watchers: [
     {MyApp.Accounts.User, :inserted}
]

but in the message, the order of schema name and action is reversed:

def handle_info({:inserted, MyApp.Accounts.User, %{id: id}}, socket) do

I.e. whatever users put into the configuration, they'll get the same thing in the message.

[cheerfulstoic]

I.e. the value of the table definition tuple is used as the 'name' of the watcher, users can pattern-match in as much detail as needed to disambiguate.

Honestly this just makes me think that we should have stuck with requiring users to define an Ecto schema module rather than allowing general table definitions 😅 That doesn't mean that I think the feature should be removed! Just that I think it's important to stick to simple things to match on. I've created a new discussion about this so that this PR doesn't go off-topic.

I've also created a separate discussion regarding the idea of having :ecto_watch as the first element in the tuple.

While the ideas are somewhat related (because of the idea of labels, for one thing), I'd like to keep this PR's thread focused on the question at hand: Combining the watcher identifier and update_type into a single element which is sent as a message. So for example this basic watcher definition:

  {MyApp.Accounts.User, :inserted},

Is handled in this way:

def handle_info({:inserted, MyApp.Accounts.User, %{id: id}}, socket) do

However if you define watchers with the trigger_columns option, you are required to use a label:

     {MyApp.Accounts.User, :updated, trigger_columns: [:email, :phone], label: :user_contact_info_updated},
     {MyApp.Accounts.User, :updated, trigger_columns: [:password_hash], label: :user_password_updated},

But labels can be used in general:

  {MyApp.Accounts.User, :inserted, label: :user_inserted},
  {MyApp.Accounts.User, :updated, label: :user_updated},

It maybe doesn't make as much sense to do it in the above case, but see the examples above using extra_columns for a case that maybe makes more sense. But I could see people wanting to use labels on all of their watchers for the sake of consistency 🤷

Anyway, the main point (sorry for taking so long to get there), is that when you have a message for a label like from the above the label represents the configuration of the schema, update_type, extra_columns, etc... and I think it doesn't make sense to have both the label and the update_type as part of the message because the label (which needs to be unique across watchers), labels all of those details at once. So instead of:

def handle_info({:inserted, user_inserted, %{id: id}}, socket) do
def handle_info({:updated, user_updated, %{id: id}}, socket) do

I think that, when using labels, the update_type shouldn't be sent:

def handle_info({user_inserted, %{id: id}}, socket) do
def handle_info({user_updated, %{id: id}}, socket) do

However... That means that in the more "normal" case, you might have a three-element tuple:

def handle_info({:inserted, MyApp.Accounts.User, %{id: id}}, socket) do

So I would propose changing this PR so that the first element is either a tuple with schema/update_type (note the flipping in order from the way messages are currently formed!) OR a label:

def handle_info({{MyApp.Accounts.User, :inserted}, %{id: id}}, socket) do
# OR, if the `label` option is used:
def handle_info({:user_inserted, %{id: id}}, socket) do

---

Just noticed this bit from you:

This might remove the necessity for a 'label' notion -- and it would also align the order of values in the message with what's used in the configuration: right now, in the configuration, you would do

watchers: [
     {MyApp.Accounts.User, :inserted}
]

but in the message, the order of schema name and action is reversed:

def handle_info({:inserted, MyApp.Accounts.User, %{id: id}}, socket) do

I.e. whatever users put into the configuration, they'll get the same thing in the message.

Yeah, I agree that it should be the same way around. That was an oversight on my part 😅 That's what I was aiming for with this PR + the additional change I'm proposing.

[cheerfulstoic]

Decided to use tuples for subscribe and for message and switched the order to have the schema first. Changed the README to use aliases so that the double { would be easier to read and understand.