danielgerlag / workflow-core

Lightweight workflow engine for .NET Standard
MIT License
5.39k stars 1.2k forks source link

Deserialisation in the ToWorkflowInstance() duplicates List items due to not set ObjectCreationHandling = ObjectCreationHandling.Replace in the JsonSerializerSettings #1270

Open michalkrzych opened 5 months ago

michalkrzych commented 5 months ago

Hello. I was trying out the Workflow Core library recently and found a bug in the deserialisation in the ToWorkflowInstance() method when using SqlServer as the persistence (which uses the EntityFrameworkPersistenceProvider under the hood).

Basically,

result.Data = JsonConvert.DeserializeObject(instance.Data, SerializerSettings);

is duplicating List items due a JsonSerializerSetting, ObjectCreationHandling = ObjectCreationHandling.Replace not set. This in turn, causes all workflow steps to duplicate the items in lists.

More about the ObjectCreationHandling setting here and it's the same problem as described here.

To reproduce:

  1. you need to configure your own SQL persistance
  2. then go into ToWorkflowInstance() and before deserialising assign instance.Data some sample json which contains a list with items, you can use mine, ex.
instance.Data = "{\"$type\":\"AtlasWorkflowEngine.Services.Workflows.TestWorkflow.TestData, AtlasWorkflowEngine.Services\",\"ApprovalStages\":{\"$type\":\"System.Collections.Generic.List`1[[AtlasWorkflowEngine.Services.Workflows.TestWorkflow.TestStage, AtlasWorkflowEngine.Services]], System.Private.CoreLib\",\"$values\":[{\"$type\":\"AtlasWorkflowEngine.Services.Workflows.TestWorkflow.TestStage, AtlasWorkflowEngine.Services\",\"StageOrder\":1,\"StageName\":\"Stage 1.1\",\"ApprovedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[\"test1@test.com\"]},\"RejectedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[]},\"ResponseTimes\":{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib],[System.DateTime, System.Private.CoreLib]], System.Private.CoreLib\"},\"RequiredApprovers\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[\"test1@test.com\",\"test2@test.com\"]},\"ApprovalEventId\":\"4c3d8dbb-f0f1-4b9f-b4a8-39d70f213c69\",\"RequireAllApprovers\":true,\"StageIsApproved\":false,\"StageIsRejected\":false},{\"$type\":\"AtlasWorkflowEngine.Services.Workflows.TestWorkflow.TestStage, AtlasWorkflowEngine.Services\",\"StageOrder\":1,\"StageName\":\"Stage 1.2\",\"ApprovedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[]},\"RejectedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[]},\"ResponseTimes\":{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib],[System.DateTime, System.Private.CoreLib]], System.Private.CoreLib\"},\"RequiredApprovers\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[\"test1@test.com\",\"test2@test.com\",\"test3@test.com\"]},\"ApprovalEventId\":\"acb565b7-26cc-47d8-ad54-c0dbcbbe64e8\",\"RequireAllApprovers\":false,\"StageIsApproved\":false,\"StageIsRejected\":false},{\"$type\":\"AtlasWorkflowEngine.Services.Workflows.TestWorkflow.TestStage, AtlasWorkflowEngine.Services\",\"StageOrder\":2,\"StageName\":\"Stage 2\",\"ApprovedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[]},\"RejectedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[]},\"ResponseTimes\":{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib],[System.DateTime, System.Private.CoreLib]], System.Private.CoreLib\"},\"RequiredApprovers\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[\"test1@test.com\"]},\"ApprovalEventId\":\"4e28761f-2544-4f64-9f88-38278c2828ed\",\"RequireAllApprovers\":true,\"StageIsApproved\":false,\"StageIsRejected\":false}]},\"CurrentStage\":{\"$type\":\"AtlasWorkflowEngine.Services.Workflows.TestWorkflow.TestStage, AtlasWorkflowEngine.Services\",\"StageOrder\":1,\"StageName\":\"Stage 1.1\",\"ApprovedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[\"test1@test.com\"]},\"RejectedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[]},\"ResponseTimes\":{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib],[System.DateTime, System.Private.CoreLib]], System.Private.CoreLib\"},\"RequiredApprovers\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[\"test1@test.com\",\"test2@test.com\"]},\"ApprovalEventId\":\"4c3d8dbb-f0f1-4b9f-b4a8-39d70f213c69\",\"RequireAllApprovers\":true,\"StageIsApproved\":false,\"StageIsRejected\":false},\"CurrentStages\":{\"$type\":\"System.Collections.Generic.List`1[[AtlasWorkflowEngine.Services.Workflows.TestWorkflow.TestStage, AtlasWorkflowEngine.Services]], System.Private.CoreLib\",\"$values\":[{\"$type\":\"AtlasWorkflowEngine.Services.Workflows.TestWorkflow.TestStage, AtlasWorkflowEngine.Services\",\"StageOrder\":1,\"StageName\":\"Stage 1.1\",\"ApprovedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[\"test1@test.com\"]},\"RejectedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[]},\"ResponseTimes\":{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib],[System.DateTime, System.Private.CoreLib]], System.Private.CoreLib\"},\"RequiredApprovers\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[\"test1@test.com\",\"test2@test.com\"]},\"ApprovalEventId\":\"4c3d8dbb-f0f1-4b9f-b4a8-39d70f213c69\",\"RequireAllApprovers\":true,\"StageIsApproved\":false,\"StageIsRejected\":false},{\"$type\":\"AtlasWorkflowEngine.Services.Workflows.TestWorkflow.TestStage, AtlasWorkflowEngine.Services\",\"StageOrder\":1,\"StageName\":\"Stage 1.2\",\"ApprovedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[]},\"RejectedBy\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[]},\"ResponseTimes\":{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, System.Private.CoreLib],[System.DateTime, System.Private.CoreLib]], System.Private.CoreLib\"},\"RequiredApprovers\":{\"$type\":\"System.Collections.Generic.List`1[[System.String, System.Private.CoreLib]], System.Private.CoreLib\",\"$values\":[\"test1@test.com\",\"test2@test.com\",\"test3@test.com\"]},\"ApprovalEventId\":\"acb565b7-26cc-47d8-ad54-c0dbcbbe64e8\",\"RequireAllApprovers\":false,\"StageIsApproved\":false,\"StageIsRejected\":false}]}}";
  1. Step over and you will see the duplicated items in lists. If you use the json I provided above, the expected count for the ApprovedBy is 1 and RequiredApprovers 2, however the counts are getting doubled each time the ToWorkflowInstance() is being called.

image

I believe the fix is to simply set the ObjectCreationHandling = ObjectCreationHandling.Replace like this:

private static JsonSerializerSettings SerializerSettings = new JsonSerializerSettings { 
    TypeNameHandling = TypeNameHandling.All,
    ObjectCreationHandling = ObjectCreationHandling.Replace
};

but as I have a very limited knowledge and understanding of this library I kindly ask for your assistance in evaluating the impacts and/or possible side effects this might have on the rest of the library.

I'm happy to create a pull request for this "bugfix" and cross reference with this thread this afternoon.