Absence of constraints on key Umbraco Forms tables

[LennardF1989]

We're currently running into an issue where saving forms to the database appears to have an issue where one Key (UFForms table), can be added to multiple rows (with a different Id). This causes the "GetByUserId" on de FormsSecurityController to fail on the SingleOrDefault lines. We're not quite sure yet how it is triggered, that will be subject to a different issue.

Upon investigation, I ran into the following thread dating from 2017 (almost 3 years back): https://our.umbraco.com/forum/umbraco-forms/82442-umbraco-forms-user-security-error

Jeffrey points out to check for duplicate entries on the UFUserFormSecurity table, he even mentioned an old tracker issue:
https://issues.umbraco.org/issue/CON-1243#

It appears this has never been fixed and is still potentially causing issues, in particular for UFForms and UFUserFormSecurity. After analyzing these tables I noticed there are no constraints on these tables, aside from a PK on UFForms.

In light of "better safe than sorry", because a duplicate entry takes down the whole Umbraco Forms module, I propose:

• To give the Key-field on UFForms a Unique Index
• To set the Id-field on UFUserFormSecurity to a Primary Key
• To give the combination of the Form and User-field on UFUserFormSecurity a Unique Index
• To set the User-field on UFUserSecurity to a Primary Key

I'm actually fairly surprised this hasn't been done already, especially the missing Primary Keys. Something is very wrong with the installation process, as these are cleanly installed non-upgraded version of Forms in Umbraco 8. Or someone decided not to give these tables (at least) a Primary Key, while the application does kind of expect it. Either way is bad news :P

---

This item has been added to our backlog AB#10752

[LennardF1989]

The issue when it comes to saving is already being discussed here: #402 - so I'm not going to make a new issue for that. Mind you this issue is about why these tables don't have constraints to prevent the issue from happening in the first place!

[nul800sebastiaan]

Thanks @LennardF1989 - This is indeed seems like the right to do. Just FYI, It will likely take a little while before we can get to it.

[ronaldbarendse]

I've indeed manually added some of these contraints/indexes to prevent completely breaking a site in case duplicate rows popped up, see this comment: #402 (comment).

Great to see this is getting fixed in the upcoming Forms 8.7 release 😉🎉

[AndyButland]

Thanks - and yes, I'd seen that and made sure they were included in the schema updates.

[nul800sebastiaan]

@ronaldbarendse We'd love it if you could test if the upgrade to 8.7-RC works for you despite the existing modifications to your tables. Would be good to know since other people will have copied your SQL updates as well!

[ronaldbarendse]

@nul800sebastiaan I can confirm the migration works without logging warnings/errors locally and all health checks pass! Next up is restoring a live database backup and test if that also works without issues 👍🏻

[ronaldbarendse]

And even the live database (which included the manually added indexes from #402 (comment)) only logged some information messages (adding the missing constraints) and all health checks pass 🎉 @AndyButland Great work on this!

[AndyButland]

Good to hear, thanks for checking on this @ronaldbarendse.

[robertjf]

@AndyButland I know this issue is closed, but I just discovered on a live Cloud website that the Primary key on table 'UFUserSecurity' is missing according to the Health Check - this is on Umbraco 8.13.1 with Forms 8.7.3.

I'm wondering whether a create index failed, as I checked the table and found a duplication..

[AndyButland]

Hi @robertjf - yes, unfortunately it is possible that the create primary key failed, which is partly why when introducing these we also added the healthcheck to flag the issue. With 8.7 we added a migration to create some keys and indexes that should ideally have been on the tables from the start, but weren't. Unfortunately though, if in the time without in this case a primary key, a duplicate value has been added to the table, we can't add the key constraint without first removing the duplicate. And deleting data, even if likely there in error, isn't something we want to do in an automatic migration - so that particular schema update is skipped and logged.

There's more details on this here, including SQL script version of the migrations and some examples of how to find and remove duplicate records, so if and when you are able to resolve the duplicate issue, you can apply the constraint directly to the database using a SQL schema update.

[robertjf]

Thanks @AndyButland - it was pretty straightforward to resolve the issue. Having that health check certainly helped!