Skip to content
This repository has been archived by the owner on Jan 27, 2025. It is now read-only.

Consolidate path traversal. #102

Closed
blackwinter opened this issue Dec 23, 2021 · 3 comments · Fixed by #170
Closed

Consolidate path traversal. #102

blackwinter opened this issue Dec 23, 2021 · 3 comments · Fixed by #170
Assignees
@fsteeg

Comments

@blackwinter
Copy link
Member

Also, transformFields() and find()/findNested() are basically performing the same traversal operations. There may be room for consolidation.

Originally posted by @blackwinter in #97 (comment)

For now, I'm getting a bit uneasy about the divergence of object traversal between find()/insert() and transformFields(). I should probably work on that next.

Originally posted by @blackwinter in #98 (review)

Right, this is also where I'm currently at in #95, where certain things only work in a bind if we first do e.g. an upcase (which calls transformFields).

Originally posted by @fsteeg in #98 (comment)
@blackwinter blackwinter self-assigned this Dec 23, 2021
@blackwinter
Copy link
Member Author

In addition to keeping our internal code consistent, we should probably also adopt a model similar to Catmandu's with designated getters/setters/creators/etc.

Currently, we can't properly deal with wildcards when inserting/transforming values (see also #92 and #100). This needs to be fixed somehow, quite possibly by introducing an abstract path traversal model.

@blackwinter blackwinter mentioned this issue Jan 24, 2022
Closed
@fsteeg fsteeg self-assigned this Jan 31, 2022
fsteeg added a commit that referenced this issue Feb 1, 2022
@fsteeg
Add updated tests from 65e1de9 (see #95, #102)
248d18d
fsteeg added a commit that referenced this issue Feb 1, 2022
@fsteeg @blackwinter
Fix tests based on feedback on 248d18d (see #95, #102)
ca2ffb8 
Co-authored-by: Jens Wille <[email protected]>
fsteeg added a commit that referenced this issue Feb 1, 2022
@fsteeg
Extract find* and transform* methods from Value.Array (#102)
2a14c70 
To a `Path` class that in the end should become something like
https://metacpan.org/pod/Catmandu::Path::simple
fsteeg added a commit that referenced this issue Feb 1, 2022
@fsteeg
Extract find* and transform* methods from Value.Hash (#102)
7bd9854
fsteeg added a commit that referenced this issue Feb 3, 2022
@fsteeg
Extract find* and transform* methods from Value.Array (#102)
4be871b 
To a `Path` class that in the end should become something like
https://metacpan.org/pod/Catmandu::Path::simple
fsteeg added a commit that referenced this issue Feb 3, 2022
@fsteeg
Extract find* and transform* methods from Value.Hash (#102)
5cbe20a
fsteeg added a commit that referenced this issue Feb 3, 2022
@fsteeg
Rename Path to FixPath as suggested in feedback on 2a14c70 (#102)
821308f
@fsteeg fsteeg mentioned this issue Feb 3, 2022
Open
@TobiasNx TobiasNx moved this to Working in Metafacture Feb 3, 2022
This was referenced Feb 3, 2022
Closed
Closed
Closed
Closed
Closed
Closed
@blackwinter blackwinter removed their assignment Feb 3, 2022
@TobiasNx TobiasNx mentioned this issue Jan 27, 2025
Open
fsteeg added a commit that referenced this issue Feb 4, 2022
@fsteeg
Extract insert* and removeNested logic from Value (#102)
1b96789
fsteeg added a commit that referenced this issue Feb 4, 2022
@fsteeg
Order FixPath members by visibility, unify naming (#102)
ef84bc8
fsteeg added a commit that referenced this issue Feb 4, 2022
@fsteeg
Some visibility and consistency tweaks (#102)
e663354
fsteeg added a commit that referenced this issue Feb 4, 2022
@fsteeg
Restrict visibility and improve naming consistency (#102)
f85cc3e
@fsteeg fsteeg mentioned this issue Feb 4, 2022
Merged
@fsteeg
Copy link
Member

fsteeg commented Feb 4, 2022

Merging #133 would unblock work on #113 & #116 (which will touch the same code). Note (mostly to self): continue by making the second FixPath constructor private, work on Hash.appendValue / modifyFields (as mentioned in our meeting, I'll only be able to continue here at the end of next week).

@fsteeg fsteeg moved this from Working to Ready in Metafacture Feb 4, 2022
TobiasNx added a commit that referenced this issue Mar 22, 2022
@TobiasNx
Add new replace_all error (#102)
aa585b0
TobiasNx added a commit that referenced this issue Mar 22, 2022
@TobiasNx
Add new replace_all error (#102)
80ad4ef
TobiasNx added a commit that referenced this issue Mar 22, 2022
@TobiasNx
Third lookup error #102
4055a43
TobiasNx added a commit that referenced this issue Mar 22, 2022
@TobiasNx
Split+Trim+Move error #102
c1003b3
TobiasNx added a commit that referenced this issue Mar 22, 2022
@TobiasNx
Copy_field with index error #102
872d0f2
TobiasNx added a commit that referenced this issue Mar 22, 2022
@TobiasNx
Conform tests to editorconfig #102
55f2ba8
TobiasNx added a commit that referenced this issue Mar 22, 2022
@TobiasNx
Add test variant and fix tests #102
d199059
fsteeg added a commit that referenced this issue Mar 22, 2022
@fsteeg
Update passing integration tests (#102, #149, #170)
e027e1c
TobiasNx added a commit that referenced this issue Mar 23, 2022
@TobiasNx
Add integration test for replace with last Wildcard (#102)
389826b
@TobiasNx TobiasNx mentioned this issue Mar 23, 2022
Merged
TobiasNx added a commit that referenced this issue Mar 24, 2022
@TobiasNx
Add integration test for add_field with $last wildcard
25c2c7b 
#102

Behaviour seems to have changed #102 breaks and master ingores
if there is no object in the array and you use to `add_field` with
$last wildcard
@TobiasNx TobiasNx mentioned this issue Mar 24, 2022
Merged
TobiasNx added a commit that referenced this issue Mar 24, 2022
@TobiasNx
Merge pull request #204 from metafacture/102-oersiIntegration
6c5403c 
Add integration test for replace with last Wildcard (#102)
fsteeg added a commit that referenced this issue Mar 25, 2022
@fsteeg
Only create new path if old value's path doesn't match (#102, #170)
64d8d98
blackwinter added a commit that referenced this issue Mar 25, 2022
@blackwinter
Add unit test for record transformation. (#102, #170)
8cfe1a8
fsteeg added a commit that referenced this issue Mar 25, 2022
@fsteeg
Fix nested removal from array (#102, #113, #170)
e2cf74c
blackwinter added a commit that referenced this issue Mar 25, 2022
@blackwinter
Add unit test for record transformation. (#102, #170)
2378455
fsteeg added a commit that referenced this issue Mar 29, 2022
@fsteeg
Preserve order in Record#transform (#102, #170)
f210dd3 
Mark tests covering faulty usage as TODOs
fsteeg added a commit that referenced this issue Mar 29, 2022
@fsteeg
Explicitly set path in record test (#102, #170)
a4d557e
fsteeg added a commit that referenced this issue Mar 31, 2022
@fsteeg
Delete reversed null results after ordered transform (#102, #170)
ec9f8ef
@TobiasNx TobiasNx mentioned this issue Mar 31, 2022
Merged
fsteeg added a commit that referenced this issue Mar 31, 2022
@fsteeg
Mark failing strictness tests as TODOs (#102, #170)
40ab054
fsteeg added a commit that referenced this issue Apr 1, 2022
@fsteeg
Update path for repeated fields to indexed array (#102, #170)
4324378
fsteeg added a commit that referenced this issue Apr 1, 2022
@fsteeg
Update path when adding, rename on copy/move field (#102, #170)
ca1cba1
fsteeg added a commit that referenced this issue Apr 4, 2022
@fsteeg
Set correct path after inserting transformed value (#102, #170)
049c709
fsteeg added a commit that referenced this issue Apr 5, 2022
@fsteeg
Replace matchType.ifString.orElseThrow with asString (#102, #170)
31cf8e9
fsteeg added a commit that referenced this issue Apr 8, 2022
@fsteeg
Improve * wildcard behavior, additional tests (#102, #170)
6779544
Repository owner moved this from Working to Done in Metafacture Apr 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@fsteeg fsteeg

Labels
None yet
Projects
Metafacture
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Consolidate path traversal metafacture/metafacture-fix
3 participants
@blackwinter @fsteeg @TobiasNx