Search code examples
c#asp.net-core.net-corejson-patch

Validate JsonPatchDocument request operations before applying it in .NET Core


I need to validate the JsonPatchDocument which is coming in the request before applying it to my object.

public async Task<IActionResult> UpdateUserByIdAsync([Required][StringLength(40)] string id, [Required][FromBody] JsonPatchDocument<UserPatch> patchDocument)

I only need to validate the operations part like - If there is an operation for replace, and the path/value variable is null/empty then it should return 400 bad request. Similarly for other operations like move, copy, add etc.

I am currently using the switch case as below :

    foreach (var operation in patchDocument.Operations)
    {
        var op = operation.op;
        var path = operation.path;
        var value = operation.value;
        var from = operation.from;
        switch (op)
        {
            case "add":
            case "replace":
            case "test":
                if (string.IsNullOrEmpty(path) || value == null)
                {
                    return this.BadRequest(this.ModelState);
                }
                break;
            case "copy":
            case "move":
                if (string.IsNullOrEmpty(path) || string.IsNullOrEmpty(from))
                {
                    return this.BadRequest(this.ModelState);
                }
                break;
            case "remove":
                if (string.IsNullOrEmpty(path))
                {
                    return this.BadRequest(this.ModelState);
                }
                break;
            default:
                return this.BadRequest(this.ModelState);
        }
    }

But in the pipeline I have codeclimate configured which breaks the pipeline if any method is having cognitive complexity more than 20, I thought changing to if-else would solve the problem but the complexity is still at 28.

Cognitive Complexity

I need some approach which can validate the request and the complexity reduces.

Edit: I updated the method as below

private static bool ValidatePatchRequest(JsonPatchDocument<UserPatch> patchDocument)
{
    var validationResult = false;

    foreach (var item in patchDocument.Operations)
    {
        if (string.IsNullOrEmpty(item.op))
        {
            validationResult = false;
            break;
        }

        if (item.op == "add" || item.op == "test" || item.op == "replace")
        {
            if (string.IsNullOrEmpty(item.path) || item.value == null)
            {
                validationResult = false;
                break;
            }
        }

        if (item.op == "copy" || item.op == "move")
        {
            if (string.IsNullOrEmpty(item.path) || item.from == null)
            {
                validationResult = false;
                break;
            }
        }

        if (item.op == "remove")
        {
            if (string.IsNullOrEmpty(item.path))
            {
                validationResult = false;
                break;
            }
        }

        validationResult = true;
    }

    return validationResult;
}

Below is the error I am getting during build pipeline execution.

[{"engine_name":"structure","fingerprint":"7558ff4c7ee81fda7b1f****","categories":["Complexity"],"check_name":"method_complexity","content":{"body":"# Cognitive Complexity\nCognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.\n\n### A method's cognitive complexity is based on a few simple rules:\n* Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one\n* Code is considered more complex for each "break in the linear flow of the code"\n* Code is considered more complex when "flow breaking structures are nested"\n\n### Further reading\n* Cognitive Complexity docs\n* Cognitive Complexity: A new way of measuring understandability\n"},"description":"Method ValidatePatchRequest has a Cognitive Complexity of 28 (exceeds 20 allowed). Consider refactoring.","location":{"path":"****Controller.cs","lines":{"begin":233,"end":276}},"other_locations":[],"remediation_points":950000,"severity":"minor","type":"issue"}, {"name":"csharp.parse.succeeded","type":"measurement","value":15,"engine_name":"structure"}, {"name":"csharp.parse.succeeded","type":"measurement","value":15,"engine_name":"duplication"}]


Solution

  • In case someone comes across the same issue, this is how I resolved it. I changed the method as below and it reduced the cognitive complexity less than 20 and the pipeline was successful.

    private static bool ValidatePatchRequest(JsonPatchDocument<UserPatch> patchDocument)
    {
        var validationResult = false;
        foreach (var item in patchDocument.Operations)
        {
            if (string.IsNullOrEmpty(item.op))
            {
                validationResult = false;
                break;
            }
    
            if (string.IsNullOrEmpty(item.path))
            {
                validationResult = false;
                break;
            }
    
            if ((item.op == "add" || item.op == "test" || item.op == "replace") && item.value == null)
            {
                validationResult = false;
                break;
            }
    
            if ((item.op == "copy" || item.op == "move") && item.from == null)
            {
                validationResult = false;
                break;
            }
    
            validationResult = true;
        }
    
        return validationResult;
    }