Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

nilvon9wo
Copy link

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
Copy link
Contributor

Courela commented May 18, 2023

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

@nilvon9wo
Copy link
Author

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
Copy link
Contributor

Courela commented Jun 15, 2023

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
Copy link
Author

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
Copy link
Contributor

Courela commented Jun 18, 2023

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants