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

Release Metafacture core 5.7.0 #502

Closed
10 tasks done
dr0i opened this issue Oct 9, 2023 · 6 comments
Closed
10 tasks done

Release Metafacture core 5.7.0 #502

dr0i opened this issue Oct 9, 2023 · 6 comments

Comments

@dr0i
Copy link
Member

dr0i commented Oct 9, 2023

Follows up #487.
See #491.

Blocked by #507.

@dr0i
Copy link
Member Author

dr0i commented Nov 17, 2023

Based on this PR is the sonatype SNAPSHOT release 5.7.0-rc2-SNAPSHOT and github packages 5.7.0-rc2 . See wiki/Maintainer-Guideline how to use it. Please test it and check mark:

  • lobid-resources
  • limetrans
  • fix
  • playground

@blackwinter
Copy link
Member

fix:

MetafixMethodTest > shouldUriEncodePathSegment() FAILED
    org.mockito.exceptions.verification.VerificationInOrderFailure: 
    Verification in order failure
    Wanted but not invoked:
    streamReceiver.literal(
        "id",
        "slash%2F990223521400206441%3ADE%2DA96%3A61%20TYD%2016%283%29%23%21"
    );
    -> at org.metafacture.metafix.MetafixMethodTest.lambda$shouldUriEncodePathSegment$318(MetafixMethodTest.java:4023)
    Wanted anywhere AFTER following interaction:
    streamReceiver.startRecord("1");
    -> at org.metafacture.metafix.Metafix.endRecord(Metafix.java:248)
        at org.metafacture.metafix.MetafixMethodTest.lambda$shouldUriEncodePathSegment$318(MetafixMethodTest.java:4023)
        at org.metafacture.metafix.MetafixTestHelpers.lambda$assertFix$0(MetafixTestHelpers.java:113)
        at org.metafacture.metafix.MetafixTestHelpers.assertFix(MetafixTestHelpers.java:138)
        at org.metafacture.metafix.MetafixTestHelpers.assertFix(MetafixTestHelpers.java:113)
        at org.metafacture.metafix.MetafixMethodTest.shouldUriEncodePathSegment(MetafixMethodTest.java:4013)

@dr0i
Copy link
Member Author

dr0i commented Nov 17, 2023

Re fix test error: There are two options:

  1. we change the test to assert on slash%2F990223521400206441%3ADE-A96%3A61+TYD+16%283%29%23%21 (note the + for whitespace in comparison to %20) or
  2. we set urlEncode.setPlusForSpace(false) as default in fix (in contradiction to the default in mf-core)

I would vote for 1.

@blackwinter
Copy link
Member

  1. This would be a behaviour change. We should discuss it with @TobiasNx and @fsteeg first.
  2. This is already the case. So why doesn't it work (anymore)?

In any case, I still think we should make it configurable.

@dr0i
Copy link
Member Author

dr0i commented Nov 20, 2023

(Was a bit late at Friday, so here the now more insights):
Note also the difference re - . We discussed this. So it was expected to break things in metafacture-fix.

We deliberately made a metafacture-fix release based on the old metafacture-core 5.6.0 which doesn't include the bad URLEncode because we knew that that we would change the behaviour.

And you can already setSafeChars in metafacture-core. We would have to add the feature to metafacture-fix.

@dr0i
Copy link
Member Author

dr0i commented Nov 27, 2023

All is done, closing.

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

No branches or pull requests

4 participants