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

Fix appending issues #87

Merged
merged 10 commits into from
Dec 16, 2021
Merged

Fix appending issues #87

merged 10 commits into from
Dec 16, 2021

Conversation

fsteeg
Copy link
Member

@fsteeg fsteeg commented Dec 6, 2021

Will resolve #82 and #80.

@fsteeg fsteeg changed the title Fix using $append as last path segment Fix appending issues Dec 7, 2021
@fsteeg fsteeg linked an issue Dec 9, 2021 that may be closed by this pull request
@fsteeg fsteeg requested a review from blackwinter December 9, 2021 13:20
Copy link
Member

@blackwinter blackwinter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from explicit array marking and still somewhat confusing list semantics (merging), 👍

@fsteeg
Copy link
Member Author

fsteeg commented Dec 9, 2021

Thanks for the review! Addressed your feedback in adb0693.

@TobiasNx TobiasNx requested a review from blackwinter December 16, 2021 07:53
@blackwinter
Copy link
Member

blackwinter commented Dec 16, 2021

Test complexAppendWithArrayOfObjects() still fails:

org.mockito.exceptions.verification.VerificationInOrderFailure: 
Verification in order failure
Wanted but not invoked:
streamReceiver.startEntity("4");
-> at org.metafacture.metafix.MetafixRecordTest.lambda$complexAppendWithArrayOfObjects$31(MetafixRecordTest.java:466)
Wanted anywhere AFTER following interaction:
streamReceiver.endEntity();

Fix string:

copy_field('others', 'animals[].$append')
move_field('fictional', 'animals[].$append')
add_field('animals[].$append.animal', 'earthworm')
[Mockito] Interactions of: streamReceiver
     1. streamReceiver.startRecord("1");
      -> at org.metafacture.metafix.Metafix.endRecord(Metafix.java:146)
     2. streamReceiver.startEntity("animals[]");
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:164)
     3. streamReceiver.startEntity("1");
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:164)
     4. streamReceiver.literal("animal", "dog");
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:169)
     5. streamReceiver.endEntity();
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:166)
     6. streamReceiver.startEntity("2");
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:164)
     7. streamReceiver.literal("animal", "cat");
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:169)
     8. streamReceiver.endEntity();
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:166)
     9. streamReceiver.startEntity("3");
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:164)
     10. streamReceiver.literal(
        "animal",
        "earthworm"
    );
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:169)
     11. streamReceiver.endEntity();
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:166)
     12. streamReceiver.endEntity();
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:166)
     13. streamReceiver.startEntity("others");
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:164)
     14. streamReceiver.literal("animal", "human");
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:169)
     15. streamReceiver.endEntity();
      -> at org.metafacture.metafix.Metafix.lambda$emit$0(Metafix.java:166)
     16. streamReceiver.endRecord();
      -> at org.metafacture.metafix.Metafix.endRecord(Metafix.java:149)

Is the test wrong?

UPDATE: See also #92.

@fsteeg
Copy link
Member Author

fsteeg commented Dec 16, 2021

Is the test wrong?

No, it does not work yet for (indexed) arrays of objects (only strings).

To keep changes small and focused, I'd say we disable the test and work on that in #92.

Copy link
Member

@blackwinter blackwinter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep changes small and focused, I'd say we disable the test and work on that in #92.

Okay.

@blackwinter blackwinter assigned fsteeg and unassigned blackwinter Dec 16, 2021
@fsteeg fsteeg merged commit e2d561e into master Dec 16, 2021
@blackwinter blackwinter deleted the 82-append branch December 17, 2021 15:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Array of strings handling not working Array handling with and without set_array
2 participants