Added JSON mapping plan implementation by joshika39 · Pull Request #14 · kougen/idou

joshika39 · 2025-12-28T16:39:45Z

No description provided.

Copilot

Pull request overview

This PR adds a JSON-based mapping plan implementation that allows entity types to be mapped from source to target names using configuration loaded from a JSON file. The implementation extends the existing mapping infrastructure by adding a new JsonMappingPlan class and introducing an IsValid property to the IMappingPlan interface.

Key changes:

New JsonMappingPlan class that loads mappings from JSON files with case-insensitive matching
Added IsValid property to IMappingPlan interface to indicate mapping plan validity
Added Newtonsoft.Json dependency for JSON deserialization

Reviewed changes

Copilot reviewed 11 out of 15 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
idou.Core/idou.Core.csproj	Added Newtonsoft.Json 13.0.4 package reference
idou.Core/Mapping/JsonMappingPlan.cs	Implements IMappingPlan to load and apply entity type mappings from JSON files
idou.Core/Mapping/JsonMapping.cs	Data model representing a single source-to-target mapping entry
idou.Core/Mapping/IMappingPlan.cs	Added IsValid property to the interface
idou.Core/Mapping/DefaultMappingPlan.cs	Implemented IsValid property (always returns true)
idou.Core.Tests/idou.Core.Tests.csproj	Configured test asset JSON files to be copied to output directory
idou.Core.Tests/Mapping/assets/valid.json	Test fixture with a sample mapping configuration
idou.Core.Tests/Mapping/assets/empty.json	Test fixture with empty mappings array
idou.Core.Tests/Mapping/JsonMappingPlanTests.cs	Unit tests for JsonMappingPlan functionality
idou.Core.Tests/Engine/ChangePipelineTests.cs	Updated RecordingMappingPlan to implement new IsValid property
docs/flow.puml	Added PlantUML diagram documenting sync job execution flow
.gitignore	Replaced minimal ignore rules with comprehensive .NET/IDE gitignore patterns
.idea/.idea.idou/.idea/vcs.xml	JetBrains Rider VCS configuration
.idea/.idea.idou/.idea/indexLayout.xml	JetBrains Rider index layout configuration
.idea/.idea.idou/.idea/.gitignore	JetBrains Rider ignore patterns

Files not reviewed (3)

.idea/.idea.idou/.idea/.gitignore: Language not supported
.idea/.idea.idou/.idea/indexLayout.xml: Language not supported
.idea/.idea.idou/.idea/vcs.xml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

idou.Core.Tests/Mapping/JsonMappingPlanTests.cs

.idea/.idea.idou/.idea/vcs.xml

Copilot · 2025-12-28T16:43:49Z

idou.Core/Mapping/JsonMapping.cs

+    public string Source { get; set; }
+    public string Target { get; set; }


The Source and Target properties are nullable reference types but should not be null for valid mappings. Consider adding null-forgiving operators, making them required properties, or adding validation to ensure they're not null during deserialization. Currently, if the JSON contains null values for these properties, a NullReferenceException will occur when JsonMappingPlan tries to use them in line 19 (ToDictionary) or line 27 (new EntityType).

Suggested change

public string Source { get; set; }

public string Target { get; set; }

public required string Source { get; set; } = null!;

public required string Target { get; set; } = null!;

idou.Core/Mapping/JsonMappingPlan.cs

Copilot · 2025-12-28T16:43:50Z

idou.Core.Tests/Mapping/JsonMappingPlanTests.cs

+using idou.Core.Domain;
+using idou.Core.Mapping;
+
+namespace idou.Core.Tests.Mapping;
+
+public class JsonMappingPlanTests
+{
+    [Fact]
+    public void MappingPlanIsValid_WhenTheFileExistsAndHasMappings()
+    {
+        var plan = new JsonMappingPlan("Mapping/assets/valid.json");
+        Assert.True(plan.IsValid);
+    }
+
+    [Fact]
+    public void Throws_WhenTheFileDoesNotExist()
+    {
+        Assert.Throws<FileNotFoundException>(() => new JsonMappingPlan("Mapping/assets/does-not-exist.json"));
+    }
+
+    [Fact]
+    public void MappingPlanIsInvalid_WhenTheMappingsAreEmpty()
+    {
+        var plan = new JsonMappingPlan("Mapping/assets/empty.json");
+        Assert.False(plan.IsValid);
+    }
+
+    [Fact]
+    public void MapsTheEntityType_WhenTheMappingExists()
+    {
+        var plan = new JsonMappingPlan("Mapping/assets/valid.json");
+
+        var sourceType = new EntityType("users");
+        var mapped = plan.MapEntityType(sourceType);
+
+        Assert.True(plan.IsValid);
+        Assert.Equal("accounts", mapped.Name);
+    }
+
+    [Fact]
+    public void DoesNotMapTheEntityType_WhenNoMappingExists()
+    {
+        var plan = new JsonMappingPlan("Mapping/assets/valid.json");
+
+        var sourceType = new EntityType("unknown");
+        var mapped = plan.MapEntityType(sourceType);
+
+        Assert.Equal(sourceType.Name, mapped.Name);
+    }
+}


The MapRecord method lacks test coverage. While MapEntityType is tested, there are no tests verifying that MapRecord correctly creates a new EntityRecord with mapped type and key, or that it properly copies the Attributes dictionary. Consider adding a test that verifies MapRecord behavior, similar to the existing tests for MapEntityType.

Copilot · 2025-12-28T16:43:50Z

idou.Core/Mapping/JsonMappingPlan.cs

+            File.ReadAllText(filePath)
+        ) ?? throw new InvalidOperationException("Invalid mapping file");
+
+        _bySource = mappings.ToDictionary(m => m.Source, StringComparer.OrdinalIgnoreCase);


If the JSON file contains duplicate source values (considering case-insensitive comparison due to StringComparer.OrdinalIgnoreCase), the ToDictionary call will throw an ArgumentException. Consider either validating for duplicates and providing a clearer error message, or handling this case explicitly. There's also no test coverage for this scenario.

Copilot · 2025-12-28T16:43:50Z

idou.Core/Mapping/JsonMappingPlan.cs

+        var mappings = JsonConvert.DeserializeObject<List<JsonMapping>>(
+            File.ReadAllText(filePath)
+        ) ?? throw new InvalidOperationException("Invalid mapping file");


If the JSON file contains malformed JSON, JsonConvert.DeserializeObject will throw a JsonException that isn't caught or handled. Consider adding try-catch to provide a more user-friendly error message, or document that callers should expect JsonException in addition to FileNotFoundException and InvalidOperationException.

Suggested change

var mappings = JsonConvert.DeserializeObject<List<JsonMapping>>(

File.ReadAllText(filePath)

) ?? throw new InvalidOperationException("Invalid mapping file");

List<JsonMapping> mappings;

try

{

mappings = JsonConvert.DeserializeObject<List<JsonMapping>>(

File.ReadAllText(filePath)

) ?? throw new InvalidOperationException("Invalid mapping file");

}

catch (JsonException ex)

{

throw new InvalidOperationException("Invalid mapping file", ex);

}

github-actions · 2025-12-28T19:01:32Z

Test Results

25 tests +16 25 ✅ +16 0s ⏱️ ±0s
1 suites ± 0 0 💤 ± 0
1 files ± 0 0 ❌ ± 0

Results for commit 5998bd4. ± Comparison against base commit ecc0349.

♻️ This comment has been updated with latest results.

github-actions · 2025-12-28T22:21:21Z

Package	Line Rate	Branch Rate	Complexity	Health
idou.Core	100%	83%	56	✔
Summary	100% (92 / 92)	83% (15 / 18)	56	✔

joshika39 added 6 commits December 28, 2025 16:19

added newtonsoft.json 13.0.4 to the Core

9f3ab7b

added a valid flag to the IMappingPlan

776c6c7

started implementing json mappings

da47045

added flow diagram

ca25705

working inital JSON mapping plan

5415f78

added idea configs and excluded copilot settings

d3b1f36

joshika39 requested a review from Copilot December 28, 2025 16:40

Copilot started reviewing on behalf of joshika39 December 28, 2025 16:40 View session

Copilot AI reviewed Dec 28, 2025

View reviewed changes

Merge branch 'master' into feature/json-mapping-plan

02882d2

joshika39 added 10 commits December 28, 2025 20:08

removed the two user specific files from git

440ea46

improved the tests

cbef203

added tests for JsonMapping

c690f63

maxed out test coverage

6e4fd04

excluded the tests

05e599e

fixed the exclusion

fc21892

added more tests

96ef4f7

removed more unused files

9a5e094

added moq to tests and dedicated tests for ChangeEvent

d99f7a4

finished the tests

5998bd4

joshika39 merged commit a0ab535 into master Dec 28, 2025
2 checks passed

joshika39 deleted the feature/json-mapping-plan branch December 28, 2025 22:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added JSON mapping plan implementation#14

Added JSON mapping plan implementation#14
joshika39 merged 17 commits intomasterfrom
feature/json-mapping-plan

joshika39 commented Dec 28, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

Copilot AI Dec 28, 2025

Uh oh!

github-actions bot commented Dec 28, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Dec 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		public string Source { get; set; }
		public string Target { get; set; }

Conversation

joshika39 commented Dec 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions bot commented Dec 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Dec 28, 2025 •

edited

Loading