Case-insensitive tag names with SQLite provider

[lewishazell]

Prerequisites

• I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #13981

Description

I've fixed the case-sensitivity issues with tags in SQLite by using LIKE instead of = when querying and joining tags.

This could be fixed through collations but would require either database-specific SQL or database-specific schema changes. I liked this way because it works everywhere and doesn't need large changes in the database setup.

To test:

• Create an Umbraco site and add a content template with variants enabled and a tag property editor.
• Create one page with tag "Umbraco" and another with tag "umbraco" instead.
• Both pages should save and publish.
• Test SQLite and SQL Server to be sure.

I have also added an integration test, though this only tests the in-memory SQLite database.

[github-actions]

Hi there @lewishazell, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[mikecp]

Thanks @lewishazell for the fix, I'll have to try this out 😉
Cheers!

[mikecp]

Hi @lewishazell ,

I have tested your update and it works all fine 👍
So let's merge this and remove tags CaSE senSiTiviTY 😁

CHeers and thanks again for the fix!

[lewishazell]

Great, thanks @mikecp! Glad to help! 😄

[nul800sebastiaan]

Cherry picked for 10.6 in bb7752c

[nikolajlauridsen]

Hey @lewishazell, unfortunately, I've had to revert this pull request since it does not work on SQL Server (thankfully, you had added tests (Big H5YR), we'd love another PR however which also works with SQL Server 😄

[lewishazell]

@nikolajlauridsen Bummer! I was sure I tested on SQL Server. Maybe I set it up incorrectly.

I'll see what I can do :)

[nikolajlauridsen]

Yeah always a bummer to have something reverted, and sorry that I don't have any more info as to why right now 😅

I just saw that the tests failed while building the RC that's gonna be released today, so didn't have much time 😄

But I see why it wasn't caught earlier, it's because PRs only run the integration tests against SQLite, and not SQL Server, so they didn't run until I was making the release. When testing locally however you can actually make the tests use LocalDB, if you go into the Umbraco.Tests.Integration project and open the appsettings.Tests.json you can then set the key Tests:Database:DatabaseType to "LocalDb" it'll run with local DB 😄

[lewishazell]

@nikolajlauridsen Sorry for my tardiness on this, been busy! Thanks for the LocalDB tip 😄

I've stepped through this and it seems the issue actually lies with the behaviour of SQLite combined with the assertion in the test being faulty.

If tags are cAsE iNsEnSiTiVe, we should expect repository.GetTaggedEntitiesByTag(TaggableObjectTypes.Content, "tag1").Count() to return 2, where I've asserted 1. Both content1 and content2 should be tagged with "tag1" even though it is assigned as "TAG1" in content2. SQLite returning 1 means it has failed to tag content2 and this isn't what we're looking for.

I think case insensitive comparison for tag names on line 145 of TagRepository.cs would fix this.

I'm also leaning more towards using lower(tagSet.tag) = lower(cmsTags.tag) just in case LIKE introduces any unintended side-effects. For example 'abc' LIKE 'a_c' would be true in SQL Server. I'm not sure which characters are allowed in tags, but this seems... bad.

Then the integration test should ensure:

• Only one tag is created
• Both nodes have the created tag

I take it this will be a new PR?