JsonTransform transforms template

[nilvon9wo]

We are using JUST templates to transform messages coming out of Salesforce to abstractions which better suit our business needs and mask the Salesforce implementation details. There is a lot of abstraction within this process so we don't need to hardcode either the input or output models nor even the transformations as we can store JUST templates in a Mongo database.

When dealing with Salesforce, one must bulkify everything (which is probably a good rule of thumb for most development), which exposes a bug in how JUST.Transform(JObject transformer, JToken input) seems to be working.

I can execute this method:

	public Message CreateOutboundMessage(SubscriptionMessage message, ServiceBusDispatchSalesforceSubscriptionSettings settings)
	{
		JObject jsonPayload = JObject.FromObject(message);
		JToken transformedPayload = settings.JustTemplate != null
			? _jsonTransformer.Transform(settings.JustTemplate, jsonPayload)
			: jsonPayload;
		Message outboundMessage = new()
		{
			BusDomain = settings.AzureDomain,
			EntityPath = settings.AzureTopic,
			Body = Encoding.UTF8.GetBytes(transformedPayload.ToString(Formatting.None)),
			ContentType = "application/json"
		};
		return AddCustomProperties(outboundMessage, jsonPayload, settings);
	}

and the output is correct.

However, when multiple messages are processed, only the first message is correct and the rest are identical to the first.
Inspecting with the debugger reveals that when _jsonTransformer.Transform(settings.JustTemplate, jsonPayload) is executed, the value of settings.JustTemplate is modified such that all subsequent invocations will receive the same value.

We should be able to fix this in by cloning the value of settings.JustTemplate before consuming a clone of it.

[nilvon9wo]

Tried to create a PR to fix this issue, but for some reason either GitHub or this project will not allow me to push my branch.

Anyway, a simple fix would be to change the method at line 106 of JsonTransformer to:

        public JToken Transform(JObject transformer, JToken input)
        {
            // We must clone the transformer or the input templated is mutated preventing 
            // it from being applied correctly to subsequent inputs.
            // (An alternative solution is to refactor to otherwise prevent this seemingly undesirable mutation.)
            if((transformer?.DeepClone()) is JObject transformerTemplate)
            {
                Context.Input = input;
                var parentToken = (JToken)transformerTemplate;
                RecursiveEvaluate(ref parentToken, null, null);
                return parentToken;
            }
            else
            {
                throw new InvalidOperationException("Transformer could not be cloned successfully.");
            }
        }

Yes, this would break things that depend on the flawed behaviour, but I can't imagine any scenario where that behaviour is desirable... but then I can't imagine how this would not have been caught and fixed earlier.

There may be better solutions, such as preventing the mutation somewhere else.
And perhaps it is not desirable to throw an error if the clone is not also a JObject, but this should only happen if the input was null which seems inherently invalid and is likely to cause an error later, so I don't think we should let it slide.

[Courela]

Transformer object is used to execute the transformation, only if transformer and input are strings this doesn't happen (transformer string is converter into an object/array).

[nilvon9wo]

@Courela,
I'm not clear on the nature of your comment.

Is it meant only as an observation that the other methods which take and transform strings to objects work as expected?
That would be the case because the original strings are not mutated and would be fed in their original form with each call.

It is only when we directly call this public method that we should expect the problem (and it is useful and therefore desirable that this method should be public).

[Courela]

It's just an observation on how it works.
I agree that there should be a public method that takes JTokens, and since it returns a value, the arguments should not be modified.

For creating a PR you must fork this project, commit your changes there and then do a cross-fork PR on this project.

[nilvon9wo]

@Courela, why a fork instead of a simple branch?
What do I need to know about doing cross-fork PRs -- I've never done one (or heard of one)?

I'll try to find time to look at this within the next couple weeks but since I have a work-around, it's not high priority for me at the moment.

[Courela]

I don't know, I started to make PRs this way, I'm not the owner of the project, I just started using it and try to improve it.

[nilvon9wo]

I finally made a PR for this:

#273