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

one_to_many mapping - repeat source value based on count #2276

Closed
rkorytkowski opened this issue Aug 10, 2020 · 10 comments · Fixed by #2287
Closed

one_to_many mapping - repeat source value based on count #2276

rkorytkowski opened this issue Aug 10, 2020 · 10 comments · Fixed by #2287

Comments

@rkorytkowski
Copy link
Contributor

rkorytkowski commented Aug 10, 2020

Initial PR: #2049

from PR: "one_to_many mapping - repeat source value based on count of fieldPath and do the target transformation."

@rkorytkowski
Copy link
Contributor Author

@igarashitm so I was looking into getting rid off the dropdown. The purpose of it is to point to an array to get its length and determine how many times the source value should be repeated. It's not for selecting the source value itself.

If we get rid of the dropdown, we need a different way to determine how many times to repeat the source value. I was looking into reading that info from the target side, but it may not be available at the point of setting the target field. Is that right? The target array final length could be determined after processing of all other fields.

I'm leaning towards keeping the dropdown and limiting listed fields to those of collection type. I would also include another parameter for specifying a count as an integer. The user would have to specify one or another.

Any thoughts on how to best approach it?

@shanthi4647
Copy link
Collaborator

I agree.. i didn't see a way to repeat source field based on target array size, as we process source fields first and then move to processing target. so went for dropdown of source fields

@igarashitm
Copy link
Member

Sounds like it's a limited version of #781 where it only uses one of single collection length at source side, correct? so like /foo<a>/bar<b> => /foo<a> while our default index mapping is /foo<a>/bar<b> => /foo<a*b>. I don't like to introduce a separate UI for this specific asynmmetric option. So if we really want to achieve it we need generic #781 I would say.
Another option in the meantime is to do it with Conditional Mapping. It should be easier than #781 - the length transformation we already have could participate to get the specific collection length.

@rkorytkowski
Copy link
Contributor Author

rkorytkowski commented Aug 10, 2020

It's a bit different. It's /foo/bar -> /foo<a>/bar, where you would basically put the value of bar "a" times and the target array length is determined based on some source array length or given as a constant.

@igarashitm
Copy link
Member

Ah OK, then it's just length(/some/array<a>) which is used as a parameter for that repeat action, should be possible with Conditional Mapping I guess?

@igarashitm
Copy link
Member

igarashitm commented Aug 10, 2020

Yet another option is, at the runtime side, internally group these kind of repeating mapping depends on target array size, postpone them until other mapping finishes and then repeat for each target item that is already established by other mappings. An idea that the target array size is determined by the other mapping. It would need some core refactoring to manage mapping order.

@igarashitm
Copy link
Member

igarashitm commented Aug 10, 2020

The reason why I don't like to introduce this field dropdown is that, it then have to provide dynamic variable for transformation parameter which is not currently suported. If we go this route, we have to design generic dynamic transformation parameter support ahead of it. I would vote to go with either Conditional Mapping and/or the postponing strategy above until we really want that generic dynamic transformation variable support apart from Conditional Mapping.

@rkorytkowski
Copy link
Contributor Author

So the proposed solution is to introduce a repeat action, which could be called from a conditional mapping as follows to achieve what you intend:

repeat(/field, count(/array<>/field))

@igarashitm
Copy link
Member

igarashitm commented Aug 10, 2020

Sounds good, note that we already have a length() transformation
https://github.com/atlasmap/atlasmap/blob/master/lib/core/src/main/java/io/atlasmap/actions/ObjectFieldActions.java#L85
We might have to relax some "complex object is not supported" trap around the transformation processing.

@rkorytkowski
Copy link
Contributor Author

@shanthi4647, an updated fix for nested action calls are in the following commits:
rkorytkowski@0c9b333
rkorytkowski@71b33bd

TODO:

  1. Rename action to "repeat"
  2. Cleanup json test files to only include items relevant for the test
  3. Add tests for some edge cases e.g. repeat(3, field), repeat(1, field), repeat(3, field_nested_in_a_few_collections)
  4. Remove UI changes
  5. Squash all commits and create a PR

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

Successfully merging a pull request may close this issue.

3 participants