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

1888-opacLink #1906

Merged
merged 25 commits into from
Sep 29, 2023
Merged

1888-opacLink #1906

merged 25 commits into from
Sep 29, 2023

Conversation

TobiasNx
Copy link
Contributor

@TobiasNx TobiasNx commented Sep 25, 2023

This PR does the following:

  • Refactoring the mapping heldBy so that the ISIL of the IZ is provided, that changes the order of heldBy.isil and heldBy.id
  • Adding a macro for adding a opacLink as sameAs
  • Adding mapping files (tsv) for opacLink tusing a submodule for prod and a local copy for test
  • Adding sameAs for all mapped hasItem-objects

@acka47 do you have a better property than opacLink?


Outdated:
First draft of opac link enrichment mechanism

@blackwinter: my first design runs into an error, could you have a look at it:

~Error-message~ ``` [main] ERROR org.lobid.resources.AlmaMarc21XmlToLobidJsonMetafixTest - Errored when transforming org.metafacture.metafix.FixProcessException: Error while executing Fix expression (at file:/home/tobias/git/lobid-resources/src/main/resources/alma/fix/macros.fix, line 267): split_field("$i.@opacLink","{hbzid}") at org.metafacture.metafix.RecordTransformer.tryRun(RecordTransformer.java:249) at org.metafacture.metafix.RecordTransformer.lambda$transform$2(RecordTransformer.java:125) at java.lang.Iterable.forEach(Iterable.java:75) at org.metafacture.metafix.RecordTransformer.transform(RecordTransformer.java:124) at org.metafacture.metafix.RecordTransformer.lambda$null$14(RecordTransformer.java:183) at org.metafacture.metafix.RecordTransformer.lambda$null$22(RecordTransformer.java:226) at org.metafacture.metafix.RecordTransformer.lambda$null$1(RecordTransformer.java:125) at org.metafacture.metafix.RecordTransformer.tryRun(RecordTransformer.java:237) at org.metafacture.metafix.RecordTransformer.lambda$transform$2(RecordTransformer.java:125) at java.lang.Iterable.forEach(Iterable.java:75) at org.metafacture.metafix.RecordTransformer.transform(RecordTransformer.java:124) at org.metafacture.metafix.RecordTransformer.transform(RecordTransformer.java:116) at org.metafacture.metafix.FixMethod$10.apply(FixMethod.java:155) at org.metafacture.metafix.RecordTransformer.lambda$null$20(RecordTransformer.java:206) at org.metafacture.metafix.RecordTransformer.lambda$null$22(RecordTransformer.java:226) at org.metafacture.metafix.RecordTransformer.lambda$null$1(RecordTransformer.java:125) at org.metafacture.metafix.RecordTransformer.tryRun(RecordTransformer.java:237) at org.metafacture.metafix.RecordTransformer.lambda$transform$2(RecordTransformer.java:125) at java.lang.Iterable.forEach(Iterable.java:75) at org.metafacture.metafix.RecordTransformer.transform(RecordTransformer.java:124) at org.metafacture.metafix.RecordTransformer.lambda$null$17(RecordTransformer.java:197) at org.metafacture.metafix.RecordTransformer.lambda$null$22(RecordTransformer.java:226) at org.metafacture.metafix.RecordTransformer.lambda$null$1(RecordTransformer.java:125) at org.metafacture.metafix.RecordTransformer.tryRun(RecordTransformer.java:237) at org.metafacture.metafix.RecordTransformer.lambda$transform$2(RecordTransformer.java:125) at java.lang.Iterable.forEach(Iterable.java:75) at org.metafacture.metafix.RecordTransformer.transform(RecordTransformer.java:124) at org.metafacture.metafix.FixBind$1.lambda$execute$1(FixBind.java:40) at org.metafacture.metafix.Value.asList(Value.java:190) at org.metafacture.metafix.Value.asList(Value.java:184) at org.metafacture.metafix.FixBind$1.execute(FixBind.java:33) at org.metafacture.metafix.RecordTransformer.lambda$null$4(RecordTransformer.java:138) at org.metafacture.metafix.RecordTransformer.lambda$null$22(RecordTransformer.java:226) at org.metafacture.metafix.RecordTransformer.lambda$null$1(RecordTransformer.java:125) at org.metafacture.metafix.RecordTransformer.tryRun(RecordTransformer.java:237) at org.metafacture.metafix.RecordTransformer.lambda$transform$2(RecordTransformer.java:125) at java.lang.Iterable.forEach(Iterable.java:75) at org.metafacture.metafix.RecordTransformer.transform(RecordTransformer.java:124) at org.metafacture.metafix.RecordTransformer.transform(RecordTransformer.java:116) at org.metafacture.metafix.FixMethod$1.apply(FixMethod.java:60) at org.metafacture.metafix.RecordTransformer.lambda$null$20(RecordTransformer.java:206) at org.metafacture.metafix.RecordTransformer.lambda$null$22(RecordTransformer.java:226) at org.metafacture.metafix.RecordTransformer.lambda$null$1(RecordTransformer.java:125) at org.metafacture.metafix.RecordTransformer.tryRun(RecordTransformer.java:237) at org.metafacture.metafix.RecordTransformer.lambda$transform$2(RecordTransformer.java:125) at java.lang.Iterable.forEach(Iterable.java:75) at org.metafacture.metafix.RecordTransformer.transform(RecordTransformer.java:124) at org.metafacture.metafix.Metafix.endRecord(Metafix.java:222) at org.metafacture.monitoring.StreamBatchLogger.endRecord(StreamBatchLogger.java:129) at org.metafacture.biblio.marc21.MarcXmlHandler.endElement(MarcXmlHandler.java:132) at com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.endElement(AbstractSAXParser.java:609) at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanEndElement(XMLDocumentFragmentScannerImpl.java:1781) at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl$FragmentContentDriver.next(XMLDocumentFragmentScannerImpl.java:2966) at com.sun.org.apache.xerces.internal.impl.XMLDocumentScannerImpl.next(XMLDocumentScannerImpl.java:601) at com.sun.org.apache.xerces.internal.impl.XMLNSDocumentScannerImpl.next(XMLNSDocumentScannerImpl.java:112) at com.sun.org.apache.xerces.internal.impl.XMLDocumentFragmentScannerImpl.scanDocument(XMLDocumentFragmentScannerImpl.java:504) at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:841) at com.sun.org.apache.xerces.internal.parsers.XML11Configuration.parse(XML11Configuration.java:770) at com.sun.org.apache.xerces.internal.parsers.XMLParser.parse(XMLParser.java:141) at com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.parse(AbstractSAXParser.java:1213) at org.metafacture.xml.XmlDecoder.process(XmlDecoder.java:69) at org.metafacture.xml.XmlDecoder.process(XmlDecoder.java:43) at org.metafacture.io.FileOpener.process(FileOpener.java:158) at org.metafacture.io.FileOpener.process(FileOpener.java:41) at org.metafacture.files.DirReader.dir(DirReader.java:99) at org.metafacture.files.DirReader.process(DirReader.java:75) at org.lobid.resources.AlmaMarc21XmlToLobidJsonMetafixTest.transformFile(AlmaMarc21XmlToLobidJsonMetafixTest.java:124) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100) at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63) at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.junit.runners.ParentRunner.run(ParentRunner.java:413) at org.junit.runners.Suite.runChild(Suite.java:128) at org.junit.runners.Suite.runChild(Suite.java:27) at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.junit.runners.ParentRunner.run(ParentRunner.java:413) at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:367) at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:274) at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238) at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:161) at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:290) at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:242) at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:121) Caused by: java.util.regex.PatternSyntaxException: Illegal repetition {hbzid} at java.util.regex.Pattern.error(Pattern.java:1969) at java.util.regex.Pattern.closure(Pattern.java:3171) at java.util.regex.Pattern.sequence(Pattern.java:2148) at java.util.regex.Pattern.expr(Pattern.java:2010) at java.util.regex.Pattern.compile(Pattern.java:1702) at java.util.regex.Pattern.(Pattern.java:1352) at java.util.regex.Pattern.compile(Pattern.java:1028) at org.metafacture.metafix.FixMethod$43.apply(FixMethod.java:595) at org.metafacture.metafix.RecordTransformer.lambda$null$20(RecordTransformer.java:206) at org.metafacture.metafix.RecordTransformer.lambda$null$22(RecordTransformer.java:226) at org.metafacture.metafix.RecordTransformer.lambda$null$1(RecordTransformer.java:125) at org.metafacture.metafix.RecordTransformer.tryRun(RecordTransformer.java:237) ... 103 more Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 9.088 sec <<< FAILURE! - in UnitTests ```

@blackwinter
Copy link
Member

Caused by: java.util.regex.PatternSyntaxException: Illegal repetition

The pattern in split_field is interpreted as a regular expression. You need to escape the curly braces.

@blackwinter blackwinter assigned TobiasNx and unassigned blackwinter Sep 25, 2023
@TobiasNx
Copy link
Contributor Author

TobiasNx commented Sep 25, 2023

@blackwinter thanks.
There seems to be a problem with copy_field (but also paste) a copied and manipulated field (here @iz) within a list bind.
The copy of the copied field is not manipulated (not turned into a ISIL), therefore the new lookup for the opac link does not work.

This seems to be a fix issue. The last commit highlights the problem: 4291810

@blackwinter
Copy link
Member

You're calling the opacLink macro too early; @iz contains the MMS ID at that point, not the ISIL.

Also, all maps are referencing the hbzId.tsv.

@TobiasNx
Copy link
Contributor Author

@jens thanks a lot.
I am getting closer. Could you help me with the opacLinks based on zdbId they are not mapped properly.

src/main/resources/alma/fix/macros.fix Outdated Show resolved Hide resolved
src/main/resources/alma/fix/macros.fix Outdated Show resolved Hide resolved
@TobiasNx TobiasNx marked this pull request as ready for review September 27, 2023 12:02
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.

Looks good, thanks!

src/main/resources/alma/fix/macros.fix Show resolved Hide resolved
src/main/resources/alma/fix/item.fix Outdated Show resolved Hide resolved
src/main/resources/alma/fix/item.fix Outdated Show resolved Hide resolved
@dr0i dr0i removed their request for review September 28, 2023 13:30
Co-authored-by: Jens Wille <ww@blackwinter.de>
@TobiasNx
Copy link
Contributor Author

There is one last change @blackwinter

@TobiasNx TobiasNx requested review from dr0i and removed request for acka47 September 28, 2023 14:23
Copy link
Member

@dr0i dr0i left a comment

Choose a reason for hiding this comment

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

putting context.jsonld is not necessary I think? Please remove it. It brings conflicts, as discussed . There is hope so - the ordering of the context!

@TobiasNx
Copy link
Contributor Author

putting context.jsonld is not necessary I think? Please remove it. It brings conflicts, as discussed . There is hope so - the ordering of the context!

Since there are so many commits I am not able to kick the changes for this specific file out. @blackwinter or @dr0i are you able to help.

@blackwinter
Copy link
Member

I'm not exactly sure what conflicts you're talking about, but does 7c6fb57 help?

@dr0i dr0i merged commit 03720ef into master Sep 29, 2023
1 check passed
@dr0i
Copy link
Member

dr0i commented Sep 29, 2023

thx @blackwinter!

@dr0i dr0i deleted the 1888-opacLink branch September 29, 2023 06:35
@dr0i
Copy link
Member

dr0i commented Sep 29, 2023

Will be deployed next Monday.

@blackwinter
Copy link
Member

This didn't work as expected. E.g. 990366624790206441:

"hasItem" : [ {
  // ...
  "seeAlso" : [ "DE-Hag4HT020469610" ],
  "id" : "http://lobid.org/items/990366624790206441:DE-Hag4:5378132900006461#!"
}, {
  // ...
  "seeAlso" : [ "DE-575HT020469610" ],
  "id" : "http://lobid.org/items/990366624790206441:DE-575:53102917670006480#!"
} ],

I couldn't find any link that was added correctly.

@dr0i
Copy link
Member

dr0i commented Oct 2, 2023

Right, that was because gitsubmodules (since #1902 ) didnt't update the isil2opac (hbz/lookup-tables@19c5676). I've restarted the indexing, should be deployed tomorrow.

@blackwinter
Copy link
Member

Caused by missing file map: lookup() calls map.get() which tries FileMap.init() leading to an exception every time. This then leaves the looked up value untouched even though delete: "true" was specified.

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

Successfully merging this pull request may close these issues.

3 participants