Skip to content

Commit 1039e75

Browse files
authored
Calculate closures correctly (#309)
* Maybe really fixes closures * fornat * add ai generated tests * fix tests * fix tests * added test with correct number of closures? * closures are self contained. don't increment on attached properties * format * MergeClosure should reuse if exists, not just set * add not null on a method
1 parent 0f8752d commit 1039e75

22 files changed

+619
-140
lines changed

.github/copilot-instructions.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# Coding standards, domain knowledge, and preferences that AI should follow
2+
3+
## C# Coding Standards
4+
5+
- Use the csharpier formatter for formatting C# code.
6+
- Use the .editorconfig file for code style settings.
7+
- Always use `var` when the type is obvious from the right side of the assignment.
8+
- Always add braces for `if`, `else`, `for`, `foreach`, `while`, and `do` statements, even if they are single-line statements.
9+
10+
## Testing
11+
12+
- Use xUnit for unit testing.
13+
- Use FluentAssertions for assertions in tests.
14+
- Use Moq for mocking dependencies in tests.

.github/git-commit-instructions.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Git Commit Instructions
2+
3+
To ensure high-quality and consistent commits, please follow these guidelines:
4+
5+
1. **Format your code**
6+
- Run the `csharpier` formatter on all C# files before committing.
7+
- Ensure your code adheres to the `.editorconfig` settings.
8+
9+
2. **Write clear commit messages**
10+
- Use the present tense ("Add feature" not "Added feature").
11+
- Start with a short summary (max 72 characters), followed by a blank line and a detailed description if necessary.
12+
13+
3. **Test your changes**
14+
- Run all unit tests before committing.
15+
- Add or update xUnit tests as needed.
16+
- Use FluentAssertions for assertions and Moq for mocking in tests.
17+
18+
4. **Review your changes**
19+
- Double-check for accidental debug code or commented-out code.
20+
- Ensure only relevant files are staged.
21+
22+
Thank you for helping maintain code quality!

Speckle.Sdk.slnx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
<File Path="GitVersion.yml" />
1515
<File Path="global.json" />
1616
<File Path="README.md" />
17+
<File Path=".github\copilot-instructions.md" />
18+
<File Path=".github\git-commit-instructions.md" />
1719
</Folder>
1820
<Folder Name="/config/workflows/">
1921
<File Path=".github/workflows/pr.yml" />
@@ -35,4 +37,4 @@
3537
<Project Path="tests/Speckle.Sdk.Tests.Integration/Speckle.Sdk.Tests.Integration.csproj" />
3638
<Project Path="tests/Speckle.Sdk.Tests.Unit/Speckle.Sdk.Tests.Unit.csproj" />
3739
</Folder>
38-
</Solution>
40+
</Solution>

src/Speckle.Sdk/Common/NotNullExtensions.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,22 @@ public static IEnumerable<T> Empty<T>(this IEnumerable<T>? obj)
9595
}
9696
return obj;
9797
}
98+
99+
public static string NotNullOrWhiteSpace(
100+
[NotNull] this string? value,
101+
[CallerArgumentExpression(nameof(value))] string? paramName = null
102+
)
103+
{
104+
if (value is null)
105+
{
106+
throw new ArgumentNullException(paramName ?? "Value is null");
107+
}
108+
109+
if (string.IsNullOrWhiteSpace(value))
110+
{
111+
throw new ArgumentException("Value cannot be empty or whitespace.", paramName);
112+
}
113+
114+
return value;
115+
}
98116
}

src/Speckle.Sdk/SQLite/SQLiteJsonCacheManager.cs

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Text;
22
using Microsoft.Data.Sqlite;
33
using Speckle.InterfaceGenerator;
4+
using Speckle.Sdk.Common;
45
using Speckle.Sdk.Dependencies;
56

67
namespace Speckle.Sdk.SQLite;
@@ -120,7 +121,10 @@ public void DeleteObject(string id) =>
120121
);
121122

122123
//This does an insert or ignores if already exists
123-
public void SaveObject(string id, string json) =>
124+
public void SaveObject(string id, string json)
125+
{
126+
id.NotNullOrWhiteSpace();
127+
json.NotNullOrWhiteSpace();
124128
_pool.Use(
125129
CacheOperation.InsertOrIgnore,
126130
command =>
@@ -130,6 +134,7 @@ public void SaveObject(string id, string json) =>
130134
command.ExecuteNonQuery();
131135
}
132136
);
137+
}
133138

134139
//This does an insert or replaces if already exists
135140
public void UpdateObject(string id, string json) =>
@@ -148,29 +153,45 @@ public void SaveObjects(IEnumerable<(string id, string json)> items) =>
148153
CacheOperation.BulkInsertOrIgnore,
149154
cmd =>
150155
{
151-
CreateBulkInsert(cmd, items);
152-
return cmd.ExecuteNonQuery();
156+
if (CreateBulkInsert(cmd, items))
157+
{
158+
cmd.ExecuteNonQuery();
159+
}
153160
}
154161
);
155162

156-
private void CreateBulkInsert(SqliteCommand cmd, IEnumerable<(string id, string json)> items)
163+
private bool CreateBulkInsert(SqliteCommand cmd, IEnumerable<(string id, string json)> items)
157164
{
158165
StringBuilder sb = Pools.StringBuilders.Get();
159-
sb.AppendLine(CacheDbCommands.Commands[(int)CacheOperation.BulkInsertOrIgnore]);
160-
int i = 0;
161-
foreach (var (id, json) in items)
166+
try
162167
{
163-
sb.Append($"(@key{i}, @value{i}),");
164-
cmd.Parameters.AddWithValue($"@key{i}", id);
165-
cmd.Parameters.AddWithValue($"@value{i}", json);
166-
i++;
167-
}
168-
sb.Remove(sb.Length - 1, 1);
169-
sb.Append(';');
168+
sb.AppendLine(CacheDbCommands.Commands[(int)CacheOperation.BulkInsertOrIgnore]);
169+
int i = 0;
170+
foreach (var (id, json) in items)
171+
{
172+
sb.Append($"(@key{i}, @value{i}),");
173+
cmd.Parameters.AddWithValue($"@key{i}", id);
174+
cmd.Parameters.AddWithValue($"@value{i}", json);
175+
i++;
176+
}
177+
178+
if (i == 0)
179+
{
180+
return false;
181+
}
182+
183+
sb.Remove(sb.Length - 1, 1);
184+
sb.Append(';');
170185
#pragma warning disable CA2100
171-
cmd.CommandText = sb.ToString();
186+
cmd.CommandText = sb.ToString();
172187
#pragma warning restore CA2100
173-
Pools.StringBuilders.Return(sb);
188+
}
189+
finally
190+
{
191+
Pools.StringBuilders.Return(sb);
192+
}
193+
194+
return true;
174195
}
175196

176197
public bool HasObject(string objectId) =>
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
namespace Speckle.Sdk.Serialisation.V2.Send;
2+
3+
public static class ClosureMath
4+
{
5+
public static void IncrementClosures(this Dictionary<Id, int> current, IEnumerable<KeyValuePair<Id, int>> child)
6+
{
7+
foreach (var closure in child)
8+
{
9+
if (current.TryGetValue(closure.Key, out var count))
10+
{
11+
current[closure.Key] = Math.Max(closure.Value, count) + 1;
12+
}
13+
else
14+
{
15+
current[closure.Key] = closure.Value + 1;
16+
}
17+
}
18+
}
19+
20+
public static void MergeClosures(this Dictionary<Id, int> current, IEnumerable<KeyValuePair<Id, int>> child)
21+
{
22+
foreach (var closure in child)
23+
{
24+
if (current.TryGetValue(closure.Key, out var count))
25+
{
26+
current[closure.Key] = Math.Max(closure.Value, count);
27+
}
28+
else
29+
{
30+
current[closure.Key] = closure.Value;
31+
}
32+
}
33+
}
34+
35+
public static void IncrementClosure(this Dictionary<Id, int> current, Id id)
36+
{
37+
if (current.TryGetValue(id, out var count))
38+
{
39+
current[id] = count + 1;
40+
}
41+
else
42+
{
43+
current[id] = 1;
44+
}
45+
}
46+
47+
public static void MergeClosure(this Dictionary<Id, int> current, Id id)
48+
{
49+
if (current.TryGetValue(id, out var count))
50+
{
51+
current[id] = count;
52+
}
53+
else
54+
{
55+
current[id] = 1;
56+
}
57+
}
58+
}

0 commit comments

Comments
 (0)