Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some children types might be overwritten during transforming to path rules #20

Merged
merged 3 commits into from
Mar 24, 2020
Merged

Fix some children types might be overwritten during transforming to path rules #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
73b270a
Fix some children types might be overwrited bug in type transforming …
Mar 18, 2020
4b497f5
Add test case to address the problem
Mar 18, 2020
d5a9fd9
Merge branch 'master' into personal/yufei/Fix-type-rule-bug
moria97 Mar 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion ...Fhir.Anonymizer.Core.UnitTests/AnonymizerConfigurations/ResourceAnonymizerContextTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public void GivenATypeRule_WhenParseRule_TransformedPathRuleShouldBeReturned()

Assert.Contains("Patient.address", context.PathSet);
Assert.Contains("Patient.name", context.PathSet);
Assert.Contains("Patient.name.period.start", context.PathSet);
Copy link
Contributor Author

@moria97 moria97 Mar 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case would not pass in master code as we treat all name nodes as the same structure and transform type rules based on Patient.name[0] but omits Patient.name[1].

Assert.Contains("Patient.address.period.start", context.PathSet);
Assert.Contains("Patient.identifier.period.start", context.PathSet);
}
Expand Down Expand Up @@ -115,7 +116,14 @@ private static ElementNode TestPatientElementNode()
""Peter"",
""James""
]
}
},
{
""use"": ""nickname"",
""text"": ""Nick"",
""period"": {
""start"":""2017-01-01""
}
},
],
""address"": [
{
Expand Down
11 changes: 5 additions & 6 deletions src/Fhir.Anonymizer.Core/AnonymizationConfigurations/ResourceAnonymizerContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ public static ResourceAnonymizerContext Create(ElementNode root, AnonymizerConfi
if (typeRules != null && typeRules.Any())
{
var rulePaths = rules.Select(rule => rule.Path).ToHashSet();
TransformTypeRulesToPathRules(root, typeRules, rules, rulePaths);
TransformTypeRulesToPathRules(root, typeRules, rules, rulePaths, new HashSet<string>());
}

return new ResourceAnonymizerContext(rules);
}

private static void TransformTypeRulesToPathRules(ElementNode node, Dictionary<string, string> typeRules, List<AnonymizerRule> rules, HashSet<string> rulePaths)
private static void TransformTypeRulesToPathRules(ElementNode node, Dictionary<string, string> typeRules, List<AnonymizerRule> rules, HashSet<string> rulePaths, HashSet<string> generatedTypePaths)
{
if (node.IsContainedNode() || node.IsEntryNode())
{
Expand All @@ -44,18 +44,17 @@ private static void TransformTypeRulesToPathRules(ElementNode node, Dictionary<s
return;
}

if (typeRules.ContainsKey(node.InstanceType))
if (!generatedTypePaths.Contains(path) && typeRules.ContainsKey(node.InstanceType))
BoyaWu10 marked this conversation as resolved.
Show resolved Hide resolved
{
var rule = new AnonymizerRule(path, typeRules[node.InstanceType], AnonymizerRuleType.TypeRule, node.InstanceType);

rules.Add(rule);
rulePaths.Add(rule.Path);
generatedTypePaths.Add(rule.Path);
}

var children = node.Children().Cast<ElementNode>();
foreach (var child in children)
{
TransformTypeRulesToPathRules(child, typeRules, rules, rulePaths);
TransformTypeRulesToPathRules(child, typeRules, rules, rulePaths, generatedTypePaths);
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/Fhir.Anonymizer.Core/AnonymizerEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ public ElementNode AnonymizeResourceNode(ElementNode root)
var resourceId = root.GetNodeId();
foreach (var rule in resourceContext.RuleList)
{
var pathCompileExpression = new FhirPathCompiler().Compile($"{rule.Path}");
var matchedNodes = pathCompileExpression(root, EvaluationContext.CreateDefault())
.Cast<ElementNode>();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me sync up with you on Teams to understand the details here.

var matchedNodes = root.Select(rule.Path).Cast<ElementNode>();

_logger.LogDebug(rule.Type == AnonymizerRuleType.PathRule ?
$"Path {rule.Source} matches {matchedNodes.Count()} nodes in resource ID {resourceId}." :
Expand Down