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

Inconsistent behaviour with asterisk-wildcard in array path #601

Open
TobiasNx opened this issue Feb 1, 2022 · 13 comments
Open

Inconsistent behaviour with asterisk-wildcard in array path #601

TobiasNx opened this issue Feb 1, 2022 · 13 comments

Comments

@TobiasNx
Copy link
Contributor

TobiasNx commented Feb 1, 2022

Since we do not have a ticket for that here. The asterisk wildcard in array paths seems to work sometimes, sometimes not and sometimes it breaks. I have set up a test with multiple usages of the *-wildcard and separated between working, breaking and no-doers. I am not sure if this is due to inconsistent array handling or due to the function. But I think it is good to collect them here:

https://github.com/TobiasNx/fix-FunctionalReview-Testing/blob/a8f314ce0c416073873553166eb9116e41a95cb7/data/testing/asteriskArray

Following functions work, also with arrays in arrays:
prepend
append
replace_all

copy_field works with single asterisk but not with arrays in arrays.

Following do nothing:
split_field
sort_field
add_field

Following break with asterisk:
reverse
sum
rename

@blackwinter
Copy link
Member

There's a pattern w.r.t. how they're implemented internally:

  • append, prepend, replace_all: Record.transformFields()
  • rename, reverse, sort_field, split_field, sum: Record.transformField() [no s]
  • add_field: Record.append()
  • copy_field: Record.copy()

@blackwinter
Copy link
Member

copy_field works with single asterisk but not with arrays in arrays.

You mean it should work without set_array(...)? Otherwise, I don't see a difference between

set_array("TEST_TWO[]")
copy_field("test[].*", "TEST_TWO[].$append")

and

set_array("TEST_3[]") # sic!
copy_field("test[].*", "TEST_THREE[].$append")

@blackwinter
Copy link
Member

blackwinter commented Feb 2, 2022

Oh, I guess you meant

set_array("TEST_4[]")
copy_field("nestedTest[].*.test[].*", "TEST_4[].$append")

So this is the expected output for TEST_THREE then? (There is no TEST_THREE in expected.json.)

"TEST_THREE" : [ {
  "1" : "One",
  "2" : "Two",
  "3" : "Three"
} ]

TobiasNx referenced this issue in TobiasNx/fix-FunctionalReview-Testing Feb 2, 2022
@TobiasNx
Copy link
Contributor Author

TobiasNx commented Feb 2, 2022

No actually this is an error in my fix. I corrected it TEST_3 is connected to the reverse handling that is commented out:

https://github.com/TobiasNx/fix-FunctionalReview-Testing/blob/764464e662ad0cae2b176e3b6087784709dde8b9/data/testing/asteriskArray/asteriskArray.fix#L27-L28

@TobiasNx
Copy link
Contributor Author

TobiasNx commented Feb 2, 2022

@blackwinter
Copy link
Member

Okay. But Test_4 is a typo again, it should be TEST_4.

@TobiasNx
Copy link
Contributor Author

TobiasNx commented Feb 2, 2022

fixed (why are my code snippets not working?)

@blackwinter
Copy link
Member

why are my code snippets not working?

Which code snippets?

@blackwinter
Copy link
Member

Turned all examples into unit tests.

@fsteeg
Copy link
Member

fsteeg commented Feb 3, 2022

Will revisit after metafacture/metafacture-fix#102.

fsteeg referenced this issue in metafacture/metafacture-fix Feb 28, 2022
For cases where the findIn path does not work with insertInto
(like `title-?` or `some.*.nested.*`), use a new Value#path field
TobiasNx referenced this issue in metafacture/metafacture-fix Mar 1, 2022
TobiasNx referenced this issue in metafacture/metafacture-fix Mar 2, 2022
TobiasNx referenced this issue in metafacture/metafacture-fix Mar 2, 2022
and for the functions that are discussed in #121
TobiasNx referenced this issue in metafacture/metafacture-fix Mar 2, 2022
and for the functions that are discussed in #121
fsteeg referenced this issue in metafacture/metafacture-fix Mar 4, 2022
fsteeg referenced this issue in metafacture/metafacture-fix May 5, 2022
@TobiasNx
Copy link
Contributor Author

TobiasNx commented Jan 8, 2025

@blackwinter should we split this issue in single tickets?

@blackwinter
Copy link
Member

No. Only if/when it turns out that the root causes are actually different and not fixable in one go.

@blackwinter blackwinter transferred this issue from metafacture/metafacture-fix Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

3 participants