WorkMaze / JUST.net

JUST - JSON Under Simple Transformation (XSLT equivalent for JSON).
MIT License
173 stars 54 forks source link

Bug/issue 263/prevent transform from modifying template parameter #273

Open nilvon9wo opened 1 year ago

nilvon9wo commented 1 year ago

As described in the bug ticket, the Transform method mutates the transformer parameter so if/when this value is reused, the latter values returned are incorrectly matched to the first input, instead of later input.

By cloning the transformer before doing anything with it, it is the clone which gets mutated and the original transformer remains safe to reuse.

Courela commented 1 year ago

I guess you can't see this... image

nilvon9wo commented 1 year ago

Sorry for delayed response, but this isn't a large priority for me, so I hardly look here. I understand your concern about this being a breaking change, but I'm not clear what you mean by EvaluationMode.

If you explain what you mean, I'll try to fix it sometime, though it may be more efficient (though less educational) if you simply take over the branch (I promise not to be offended.)

Courela commented 1 year ago

There's this enum, EvaluationMode EvaluationMode This tells some sections of the code to behave differently according to selected options, or stick to default behavior. My suggestion is for this to be optional, able to be chosen by an EvaluationMode, thus maintaining default behavior as it is.

Something like this:

    [Flags]
    public enum EvaluationMode : short
    {
        FallbackToDefault = 1,
        AddOrReplaceProperties = 2,
        Strict = 4,
        CloneTransformer = 8,
    }
            Context.Input = input;
            JToken parentToken;
            if ((Context.EvaluationMode & Context.EvaluationMode.CloneTransformer) == EvaluationMode.CloneTransformer)
            {
                if((transformer?.DeepClone()) is JObject transformerTemplate)
                {
                    parentToken = (JToken)transformerTemplate;
                }
                else
                {
                    throw new InvalidOperationException("Transformer could not be cloned successfully.");
                }
            }
            else
            {
                parentToken = (JToken)transformer;
            }
            RecursiveEvaluate(ref parentToken, null, null, input);
            return parentToken;

Then when creating JsonTransformer:

new JsonTransformer(new JUSTContext { EvaluationMode = EvaluationMode.CloneTransformer | EvaluationMode.<more options if needed> | EvaluationMode.<more options if needed> })
nilvon9wo commented 1 year ago

I see. This is something like a feature flag. That makes sense as it allows us to make the change without risk that we will break existing uses.

Is there a plan for eventually deprecating the flags and folding these features in?

Courela commented 1 year ago

Yes, like a feature flag. I'm just a contributer, I've started to be more involved in this project because I used it at work, and we were always adding new features as we needed. If there's a plan, I"m not aware of.

nilvon9wo commented 1 year ago

@Courela, I've added the Evaluation Mode flag.