Refactor: avoid double parsing of mutation string in ACL

[mangalaman93]

This change is 

[martinmr]

Reviewed 3 of 3 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gitlw and @manishrjain)

[gitlw]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gitlw and @manishrjain)

[gitlw]

Thanks for the refactoring!

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gitlw and @manishrjain)

[manishrjain]

I don't get this refactoring. It changes the sequence of mutation handling quite significantly. For e.g., we check for context, then check if the request is empty, only then we try to parse the mutation. This change reorders all of that.

What's the rationale?

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)

[mangalaman93]

Short answer:
We were calling parseMutation already inside authorizeMutation. I have only moved it outside and reused it in both authorizeMutation and doMutate/Mutate functions

Long answer:
Earlier, we were calling parseMutation function two places. Following is the old sequence of operations -
-> Call authorizeMutation
-> Check whether ACL feature is enabled
-> Call parseMutation
-> More code
-> Call doMutate
-> More code (somewhere here we call parseMutation again)

I modified this sequence to -
-> Call parseMutation [This is copied from below]
-> Call authorizeMutation
-> Check whether ACL feature is enabled
-> [Call parseMutation (This is removed in the new code and moved up]
-> More code
-> Call doMutate
-> More code (somewhere here we call parseMutation again)
^ [No need to call parseMutation here any more]

Reviewable status: complete! all files reviewed, all discussions resolved

[manishrjain]

I see.

Reviewed 3 of 3 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved