1

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"}]

Amir Suhel
  • 33
  • 4
  • Move validation into separate method. – Guru Stron Mar 17 '23 at 06:26
  • Initially I had validation in controller, but currently it is in separate method and still the same problem. – Amir Suhel Mar 17 '23 at 06:34
  • Can you please add the error from the build tool? And actual method which it complains about? – Guru Stron Mar 17 '23 at 06:42
  • "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"}, – Amir Suhel Mar 17 '23 at 06:58

1 Answers1

0

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;
}
Amir Suhel
  • 33
  • 4
  • As it’s currently written, your answer is unclear. Please [edit] to add additional details that will help others understand how this addresses the question asked. You can find more information on how to write good answers [in the help center](/help/how-to-answer). – Community Mar 22 '23 at 16:01