metafacture / metafacture-core

Core package of the Metafacture tool suite for metadata processing.
https://metafacture.org
Apache License 2.0
69 stars 34 forks source link

Release Metafacture core 5.7.0 #502

Closed dr0i closed 7 months ago

dr0i commented 9 months ago

Follows up https://github.com/metafacture/metafacture-core/issues/487. See https://github.com/metafacture/metafacture-core/issues/491.

Blocked by https://github.com/metafacture/metafacture-core/pull/507.

dr0i commented 7 months ago

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:

blackwinter commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago
  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 commented 7 months ago

(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 commented 7 months ago

All is done, closing.