Change DB schema of NotificationFilterPreferences

[surli]

Hi everyone,

I’m opening this brainstorming related to the work I’m currently doing to use a LiveData with filters and sort for notification filter preferences. And as you will see it might also be related to some future work such as Loading....

While working on LiveData and on complex query on the notification filter preferences table, I noticed that some decisions were made for that table that I think should be changed for the long run. IMO there’s 2 different problems.

1. Notification formats

Right now we have two possible notification formats in XWiki: mail and alert. But, we could have more in the future and one example could be Loading.... Problem is that right now the two formats are hardcoded in the table schema: we have explicitely email and alert to enable the two formats.

IMO we only need a single column for that where we would put the formats of the preference. Which is what we do for the event types of the notification in the same table for example.
By using a single column we have several advantage: it’s easier to extend, of course, but also it’s easier to perform filtering and sorting. Right now we have to manipulate two columns for that.
Also note that we could use here same design as for filterType for the format: to bind it to an Enum, so that the content of the column is always a supported format in XS and not anything. Honestly I don’t know if we want that or not.

2. Scope

This one is the most problematic one for me: right now the schema defines 4 different columns: user, pageOnly, page, wiki. And the idea is that for each entry, only one of this column is used, and based on that column the code is guessing the actual scope which is used: e.g. if there’s a data in pageOnly then we know it’s a preference for a single page, if there’s no data in pageOnly but a data in page then it’s a preference for a space, etc.

There’s plenty of problems with this design: since it’s hardcoded, it’s impossible to extend it; it’s really complicated to use filters / sorting since we have to manipulate 4 columns each time; it’s error prone since we have to ensure that we always use same priority on the columns and we don’t put values in multiple columns, etc. I would be very curious to know why this design was chosen back then.

So my proposal here would be to simplify all this with 2 columns: a column scope which would have a single value like user, wiki, page, space, but it could have other values in the future if we want to extend usage of the filters (e.g. for a tag filter preference we could have a scope tag). And another column for which I don’t have a good name yet, maybe something like entity which would actually contain the value the scope refers to: a page reference, a wiki reference, a user reference, or even a tag name in the future.

So there’s now different questions:

1. What do you think about the proposals above?
2. Knowing that we consider the DB as an API, what can we do to improve the situation? Would that be ok to break the API here? Personally I think that this table is really very technical and chances that anybody is actually directly querying it is very low.

[tmortagne]

@surli

IMO we only need a single column for that where we would put the formats of the preference.

It’s not very clear to me how you enable both email and alert for a filter preference in your proposal.

@surli

So my proposal here would be to simplify all this with 2 columns: a column scope which would have a single value like user, wiki, page, space, but it could have other values in the future if we want to extend usage of the filters (e.g. for a tag filter preference we could have a scope tag). And another column for which I don’t have a good name yet, maybe something like entity which would actually contain the value the scope refers to: a page reference, a wiki reference, a user reference, or even a tag name in the future.

Do you need to have two columns for a specific reason ? Filtering by scope ? Because you can express that in a single value using the prefixed entity reference syntax.

[surli]

@tmortagne

@surli

IMO we only need a single column for that where we would put the formats of the preference.

It’s not very clear to me how you enable both email and alert for a filter preference in your proposal.

The idea is to allow putting multiple values in the column like we do for the event types for example. So a column with a String type which would contain alert,email if both formats are allowed. We could even say that the column is empty if all formats are supported, like we do for event types.

@tmortagne

@surli

So my proposal here would be to simplify all this with 2 columns: a column scope which would have a single value like user, wiki, page, space, but it could have other values in the future if we want to extend usage of the filters (e.g. for a tag filter preference we could have a scope tag). And another column for which I don’t have a good name yet, maybe something like entity which would actually contain the value the scope refers to: a page reference, a wiki reference, a user reference, or even a tag name in the future.

Do you need to have two columns for a specific reason ? Filtering by scope ? Because you can express that in a single value using the prefixed entity reference syntax.

The scope column is needed at the very least to differentiate a filter targeting a page and a filter targeting a user since the reference are both pages.

[tmortagne]

@surli

differentiate a filter targeting a page and a filter targeting a user

Indeed, I read a bit too fast the list of scopes you indicated and forgot the follow user use case.

[vmassol]

@surli

The idea is to allow putting multiple values in the column like we do for the event types for example. So a column with a String type which would contain alert,email if both formats are allowed. We could even say that the column is empty if all formats are supported, like we do for event types.

Small note: That’s not very semantic and it’d mean using a LIKE operation for filtering which is far from the most performant solution.

@surli

And another column for which I don’t have a good name yet, maybe something like entity

If we want to be generic, like be able to have tags, then entity is not the right word since tags are not entities. So scope and scopeValue or scopeTarget for ex.

@surli

Knowing that we consider the DB as an API, what can we do to improve the situation? Would that be ok to break the API here? Personally I think that this table is really very technical and chances that anybody is actually directly querying it is very low.

Ideally a migration but idk if it’s used or not. Could be good to ask advanced users of XWiki (like XWiki SAS for example).

+0 in general

[surli]

@vmassol

@surli

The idea is to allow putting multiple values in the column like we do for the event types for example. So a column with a String type which would contain alert,email if both formats are allowed. We could even say that the column is empty if all formats are supported, like we do for event types.

Small note: That’s not very semantic and it’d mean using a LIKE operation for filtering which is far from the most performant solution.

Not sure what you mean by “not very semantic”, I’m taking better solution that allow extensibility. Regarding filtering and perf I agree with you, now to be honest we almost never filter on those values: right now filtering on those is only performed in the livedata. So the impact on a running instance would be null.

@vmassol

@surli

And another column for which I don’t have a good name yet, maybe something like entity

If we want to be generic, like be able to have tags, then entity is not the right word since tags are not entities. So scope and scopeValue or scopeTarget for ex.

scopeValue would be good indeed, thanks.

@vmassol

@surli

Knowing that we consider the DB as an API, what can we do to improve the situation? Would that be ok to break the API here? Personally I think that this table is really very technical and chances that anybody is actually directly querying it is very low.

Ideally a migration but idk if it’s used or not. Could be good to ask advanced users of XWiki (like XWiki SAS for example).

We will need a migration for sure: the table is used internally. I’ll ping XWiki SAS about that but I think I already know the answer

[vmassol]

@surli

Not sure what you mean by “not very semantic”

I meant that a string is a black box and if you need to parse it, then it’s less semantic than having the various options available without parsing.