Skip to content

Commit ebd228c

Browse files
AndyButlandCopilot
andauthored
Ensure tag operations are case insensitive on insert across database types (#19439)
* Ensure tag operations are case insensitve on insert across database types. * Ensure tags provided in a single property are case insensitively distinct when saving the tags and relationships. * Update src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TagRepository.cs Co-authored-by: Copilot <[email protected]> * Handle case sensitivity on insert with tag groups too. --------- Co-authored-by: Copilot <[email protected]>
1 parent 4b83a74 commit ebd228c

File tree

2 files changed

+104
-6
lines changed

2 files changed

+104
-6
lines changed

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TagRepository.cs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,13 @@ public void Assign(int contentId, int propertyTypeId, IEnumerable<ITag> tags, bo
128128
var group = SqlSyntax.GetQuotedColumnName("group");
129129

130130
// insert tags
131+
// - Note we are checking in the subquery for the existence of the tag, so we don't insert duplicates, using a case-insensitive comparison (the
132+
// LOWER keyword is consistent across SQLite and SQLServer). This ensures consistent behavior across databases as by default, SQLServer will
133+
// perform a case-insensitive comparison, while SQLite will not.
131134
var sql1 = $@"INSERT INTO cmsTags (tag, {group}, languageId)
132135
SELECT tagSet.tag, tagSet.{group}, tagSet.languageId
133136
FROM {tagSetSql}
134-
LEFT OUTER JOIN cmsTags ON (tagSet.tag = cmsTags.tag AND tagSet.{group} = cmsTags.{group} AND COALESCE(tagSet.languageId, -1) = COALESCE(cmsTags.languageId, -1))
137+
LEFT OUTER JOIN cmsTags ON (LOWER(tagSet.tag) = LOWER(cmsTags.tag) AND LOWER(tagSet.{group}) = LOWER(cmsTags.{group}) AND COALESCE(tagSet.languageId, -1) = COALESCE(cmsTags.languageId, -1))
135138
WHERE cmsTags.id IS NULL";
136139

137140
Database.Execute(sql1);
@@ -142,7 +145,7 @@ LEFT OUTER JOIN cmsTags ON (tagSet.tag = cmsTags.tag AND tagSet.{group} = cmsTag
142145
FROM (
143146
SELECT t.Id
144147
FROM {tagSetSql}
145-
INNER JOIN cmsTags as t ON (tagSet.tag = t.tag AND tagSet.{group} = t.{group} AND COALESCE(tagSet.languageId, -1) = COALESCE(t.languageId, -1))
148+
INNER JOIN cmsTags as t ON (LOWER(tagSet.tag) = LOWER(t.tag) AND LOWER(tagSet.{group}) = LOWER(t.{group}) AND COALESCE(tagSet.languageId, -1) = COALESCE(t.languageId, -1))
146149
) AS tagSet2
147150
LEFT OUTER JOIN cmsTagRelationship r ON (tagSet2.id = r.tagId AND r.nodeId = {contentId} AND r.propertyTypeID = {propertyTypeId})
148151
WHERE r.tagId IS NULL";
@@ -245,14 +248,18 @@ private class TagComparer : IEqualityComparer<ITag>
245248
{
246249
public bool Equals(ITag? x, ITag? y) =>
247250
ReferenceEquals(x, y) // takes care of both being null
248-
|| (x != null && y != null && x.Text == y.Text && x.Group == y.Group && x.LanguageId == y.LanguageId);
251+
|| (x != null &&
252+
y != null &&
253+
string.Equals(x.Text, y.Text, StringComparison.OrdinalIgnoreCase) &&
254+
string.Equals(x.Group, y.Group, StringComparison.OrdinalIgnoreCase) &&
255+
x.LanguageId == y.LanguageId);
249256

250257
public int GetHashCode(ITag obj)
251258
{
252259
unchecked
253260
{
254-
var h = obj.Text.GetHashCode();
255-
h = (h * 397) ^ obj.Group.GetHashCode();
261+
var h = StringComparer.OrdinalIgnoreCase.GetHashCode(obj.Text);
262+
h = (h * 397) ^ StringComparer.OrdinalIgnoreCase.GetHashCode(obj.Group);
256263
h = (h * 397) ^ (obj.LanguageId?.GetHashCode() ?? 0);
257264
return h;
258265
}

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/TagRepositoryTest.cs

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Umbraco.
22
// See LICENSE for more details.
33

4-
using System.Linq;
54
using Microsoft.Extensions.Logging;
65
using NUnit.Framework;
76
using Umbraco.Cms.Core.Cache;
@@ -1047,6 +1046,98 @@ public void Can_Get_Tagged_Entities_For_Tag()
10471046
}
10481047
}
10491048

1049+
[Test]
1050+
public void Can_Create_Tag_Relations_With_Mixed_Casing_For_Tag()
1051+
{
1052+
var provider = ScopeProvider;
1053+
using (var scope = ScopeProvider.CreateScope())
1054+
{
1055+
(IContentType contentType, IContent content1, IContent content2) = CreateContentForCreateTagTests();
1056+
1057+
var repository = CreateRepository(provider);
1058+
1059+
// Note two tags are applied, but they differ only in case for the tag.
1060+
Tag[] tags1 = { new() { Text = "tag1", Group = "test" }, new() { Text = "Tag1", Group = "test" } };
1061+
repository.Assign(
1062+
content1.Id,
1063+
contentType.PropertyTypes.First().Id,
1064+
tags1,
1065+
false);
1066+
1067+
// Note the casing is different from the tag in tags1, but both should be considered equivalent.
1068+
Tag[] tags2 = { new() { Text = "TAG1", Group = "test" } };
1069+
repository.Assign(
1070+
content2.Id,
1071+
contentType.PropertyTypes.First().Id,
1072+
tags2,
1073+
false);
1074+
1075+
// Only one tag should have been saved.
1076+
var tagCount = scope.Database.ExecuteScalar<int>(
1077+
"SELECT COUNT(*) FROM cmsTags WHERE [group] = 'test'");
1078+
Assert.AreEqual(1, tagCount);
1079+
1080+
// Both content items should be found as tagged by the tag, even though one was assigned with the tag differing in case.
1081+
Assert.AreEqual(2, repository.GetTaggedEntitiesByTag(TaggableObjectTypes.Content, "tag1").Count());
1082+
}
1083+
}
1084+
1085+
[Test]
1086+
public void Can_Create_Tag_Relations_With_Mixed_Casing_For_Group()
1087+
{
1088+
var provider = ScopeProvider;
1089+
using (var scope = ScopeProvider.CreateScope())
1090+
{
1091+
(IContentType contentType, IContent content1, IContent content2) = CreateContentForCreateTagTests();
1092+
1093+
var repository = CreateRepository(provider);
1094+
1095+
// Note two tags are applied, but they differ only in case for the group.
1096+
Tag[] tags1 = { new() { Text = "tag1", Group = "group1" }, new() { Text = "tag1", Group = "Group1" } };
1097+
repository.Assign(
1098+
content1.Id,
1099+
contentType.PropertyTypes.First().Id,
1100+
tags1,
1101+
false);
1102+
1103+
// Note the casing is different from the group in tags1, but both should be considered equivalent.
1104+
Tag[] tags2 = { new() { Text = "tag1", Group = "GROUP1" } };
1105+
repository.Assign(
1106+
content2.Id,
1107+
contentType.PropertyTypes.First().Id,
1108+
tags2,
1109+
false);
1110+
1111+
// Only one tag/group should have been saved.
1112+
var tagCount = scope.Database.ExecuteScalar<int>(
1113+
"SELECT COUNT(*) FROM cmsTags WHERE [tag] = 'tag1'");
1114+
Assert.AreEqual(1, tagCount);
1115+
1116+
var groupCount = scope.Database.ExecuteScalar<int>(
1117+
"SELECT COUNT(*) FROM cmsTags WHERE [group] = 'group1'");
1118+
Assert.AreEqual(1, groupCount);
1119+
1120+
// Both content items should be found as tagged by the tag, even though one was assigned with the group differing in case.
1121+
Assert.AreEqual(2, repository.GetTaggedEntitiesByTagGroup(TaggableObjectTypes.Content, "group1").Count());
1122+
}
1123+
}
1124+
1125+
private (IContentType ContentType, IContent Content1, IContent Content2) CreateContentForCreateTagTests()
1126+
{
1127+
var template = TemplateBuilder.CreateTextPageTemplate();
1128+
FileService.SaveTemplate(template);
1129+
1130+
var contentType = ContentTypeBuilder.CreateSimpleContentType("test", "Test", defaultTemplateId: template.Id);
1131+
ContentTypeRepository.Save(contentType);
1132+
1133+
var content1 = ContentBuilder.CreateSimpleContent(contentType);
1134+
var content2 = ContentBuilder.CreateSimpleContent(contentType);
1135+
DocumentRepository.Save(content1);
1136+
DocumentRepository.Save(content2);
1137+
1138+
return (contentType, content1, content2);
1139+
}
1140+
10501141
private TagRepository CreateRepository(IScopeProvider provider) =>
10511142
new((IScopeAccessor)provider, AppCaches.Disabled, LoggerFactory.CreateLogger<TagRepository>());
10521143
}

0 commit comments

Comments
 (0)