Skip to content

Commit

Permalink
refactor: remove TaskData.EscapeKey (#776)
Browse files Browse the repository at this point in the history
# Motivation

Remove code that is not necessary anymore to improve maintainability.

# Description

TaskData.EscapeKey was needed to ensure keys provided by users were
valid. This is not needed anymore beacause IDs are GUID generated by
core. This PR removes this unnecessary step.

# Testing

Unit tests were used.

# Impact

Reduced code to maintain.

# Checklist

- [x] My code adheres to the coding and style guidelines of the project.
- [x] I have performed a self-review of my code.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have made corresponding changes to the documentation.
- [x] I have thoroughly tested my modifications and added tests when
necessary.
- [x] Tests pass locally and in the CI.
- [x] I have assessed the performance impact of my modifications.
  • Loading branch information
aneojgurhem authored Oct 17, 2024
2 parents b390d92 + 745bfbe commit 283a890
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 24 deletions.
2 changes: 1 addition & 1 deletion Adaptors/Memory/src/TaskTable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ public Task RemoveRemainingDataDependenciesAsync(ICollection<string> taskIds,
{
var remainingDep = data.RemainingDataDependencies;

foreach (var dep in dependenciesToRemove.Select(TaskData.EscapeKey))
foreach (var dep in dependenciesToRemove)
{
remainingDep.Remove(dep);
}
Expand Down
4 changes: 2 additions & 2 deletions Adaptors/MongoDB/src/TaskTable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,11 @@ public async Task RemoveRemainingDataDependenciesAsync(ICollection<string> taskI
}


var key0 = TaskData.EscapeKey(deps.Current);
var key0 = deps.Current;
var update = new UpdateDefinitionBuilder<TaskData>().Unset(data => data.RemainingDataDependencies[key0]);
while (deps.MoveNext())
{
var key = TaskData.EscapeKey(deps.Current);
var key = deps.Current;
update = update.Unset(data => data.RemainingDataDependencies[key]);
}

Expand Down
20 changes: 1 addition & 19 deletions Common/src/Storage/TaskData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

using ArmoniK.Core.Base.DataStructures;

Expand Down Expand Up @@ -140,7 +139,7 @@ public TaskData(string sessionId,
{
payloadId,
})
.ToDictionary(EscapeKey,
.ToDictionary(s => s,
_ => true),
expectedOutputIds,
taskId,
Expand Down Expand Up @@ -175,23 +174,6 @@ public TaskData(TaskData original,
: this(original)
=> updates.ApplyTo(this);

/// <summary>
/// ResultIds could contain dots (eg: it is the case in htcmock),
/// but MongoDB does not support well dots in keys.
/// This escapes the key to replace dots with something else.
/// Escaped keys are guaranteed to have neither dots nor dollars
/// </summary>
/// <param name="key">Key string</param>
/// <returns>Escaped key</returns>
public static string EscapeKey(string key)
=> new StringBuilder(key).Replace("@",
"@at@")
.Replace(".",
"@dot@")
.Replace("$",
"@dollar@")
.ToString();

/// <summary>
/// Conversion operator from <see cref="TaskData" /> to <see cref="Application" />
/// </summary>
Expand Down
4 changes: 2 additions & 2 deletions Common/tests/TestBase/TaskTableTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2151,8 +2151,8 @@ public async Task RemoveRemainingDataDependenciesShouldSucceed()
{
var taskId = Guid.NewGuid()
.ToString();
var dd1 = "dependency.1";
var dd2 = "dependency.2";
var dd1 = "dependency1";
var dd2 = "dependency2";

await TaskTable!.CreateTasks(new[]
{
Expand Down

0 comments on commit 283a890

Please sign in to comment.