Port changes from IQSS XOAI fork to v4

[poikilotherm]

At https://github.com/IQSS/dataverse/tree/develop/local_lib/com/lyncode lives a local fork of an old version of this library.

We need to compare these sources to an old Lyncode release and look if and how to incorporate these changes. (Maybe things got fixed or refactored making the change obsolete...)

Apparently this is tricky: we have source JARs for data provider and service provider, but we did not add our common source JAR. Decompilation reveals the code, but comparison gets much harder this way! (And the context aka why sth was changed is lost...)

[poikilotherm]

XOAI Data Provider

No changes!

XOAI Service Provider

diff --color -ur lyncode-svc-prov/com/lyncode/xoai/serviceprovider/handler/ListSetsHandler.java iqss-svc-prov/com/lyncode/xoai/serviceprovider/handler/ListSetsHandler.java
--- lyncode-svc-prov/com/lyncode/xoai/serviceprovider/handler/ListSetsHandler.java	2014-08-01 23:08:24.000000000 +0200
+++ iqss-svc-prov/com/lyncode/xoai/serviceprovider/handler/ListSetsHandler.java	2016-05-19 12:24:16.000000000 +0200
@@ -43,6 +43,7 @@
 import com.lyncode.xoai.serviceprovider.lazy.Source;
 import com.lyncode.xoai.serviceprovider.model.Context;
 import com.lyncode.xoai.serviceprovider.parsers.ListSetsParser;
+import java.io.IOException;
 
 public class ListSetsHandler implements Source<Set> {
     private Context context;
@@ -82,6 +83,24 @@
                         resumptionToken = text;
                 } else ended = true;
             } else ended = true;
+            /* This appears to be a bug in 4.1.0: the handler should be 
+             * closing the stream here, similarly to the ListIdentifierHandle, 
+             * etc. Without closing it, if there is a resumption token and 
+             * we need to make another call - you will get an exception 
+             * "Invalid use of BasicClientConnManager: connection still allocated.
+             * Make sure to release the connection before allocating another one."
+             * Also note, that I ignore the IOException if one is thrown on an 
+             * attempt to close the stream (unlike ListIdentifierHandler - which 
+             * then proceeds to throw an InvalidOAIResponse). If there is 
+             * something seriously bad with the connection, to the point that it
+             * prevents us from making the next call, it will surely result in 
+             * an exception then. -- L.A. May 2016.
+            */
+            try {
+                stream.close();
+            } catch (IOException ioex) {
+                // ignore!
+            }
             return sets;
         } catch (XmlReaderException e) {
             throw new InvalidOAIResponse(e);
diff --color -ur lyncode-svc-prov/com/lyncode/xoai/serviceprovider/parsers/HeaderParser.java iqss-svc-prov/com/lyncode/xoai/serviceprovider/parsers/HeaderParser.java
--- lyncode-svc-prov/com/lyncode/xoai/serviceprovider/parsers/HeaderParser.java	2014-08-01 23:08:24.000000000 +0200
+++ iqss-svc-prov/com/lyncode/xoai/serviceprovider/parsers/HeaderParser.java	2016-05-10 13:44:42.000000000 +0200
@@ -40,8 +40,13 @@
         header.withIdentifier(reader.getText());
         reader.next(elementName(localPart(equalTo("datestamp")))).next(text());
         header.withDatestamp(reader.get(dateParser()));
+        /* The 4.0-4.1 implementation was buggy, in that it made the 
+         * setspec tag mandatory. This was fixed in 4.1.1, as follows:
+         * (by just removing the 2 fixed lines below:)
         reader.next(setSpecElement()).next(text());
         header.withSetSpec(reader.getText());
+        */
+        
         while (reader.next(endOfHeader(), setSpecElement()).current(setSpecElement()))
             header.withSetSpec(reader.next(text()).getText());
         return header;
diff --color -ur lyncode-svc-prov/META-INF/MANIFEST.MF iqss-svc-prov/META-INF/MANIFEST.MF
--- lyncode-svc-prov/META-INF/MANIFEST.MF	2014-08-01 23:11:18.000000000 +0200
+++ iqss-svc-prov/META-INF/MANIFEST.MF	2016-05-19 12:24:38.000000000 +0200
@@ -1,6 +1,6 @@
 Manifest-Version: 1.0
 Archiver-Version: Plexus Archiver
+Built-By: landreev
 Created-By: Apache Maven
-Built-By: jmelo
-Build-Jdk: 1.8.0_05
+Build-Jdk: 1.8.0_66

[poikilotherm]

Decompilated the common JAR via CLI, sources attached.

java -cp fernflower.jar org.jetbrains.java.decompiler.main.decompiler.ConsoleDecompiler -hdc=0 -dgs=1 -rsy=1 -rbr=1 -lit=1 -nls=1 -mpm=60 "$(pwd)/xoai-common-4.1.0-header-patch.jar" "$(pwd)/iqss-common"

xoai-common-4.1.0-header-patch-decompiles-sources.zip

[poikilotherm]

After very careful examination of the decompiled classes in xoai-common: there hadn't been any changes by @landreev to those classes.

[poikilotherm]

@landreev pushed the changes from long time ago also to https://github.com/landreev/xoai. This would be a good source to validate above findings.

As he added some other cleanups, it would be good to look into those, too. Compare branches

[landreev]

I will make a PR (into branch-5.0), and will link it to this issue.

[landreev]

The fixes DSpace eventually made in serviceprovider/handler/ListSetsHandler.java and serviceprovider/parsers/HeaderParser.java (the diffs under "XOAI Service Provider" above), for the bugs that had been fixed in the old IQSS fork, appear to be perfectly acceptable. So there shouldn't be any need to port those fixes.
But there were a couple of small, but necessary changes in xoai-common and xoai-data-provider as well, so I'm making a PR for those.

[landreev]

@poikilotherm Is it ok if I try to finish porting the xoai-service-provider module? (or would I be interfering with your efforts?)

[poikilotherm]

Hey @landreev plz let me finish #14 which I had mostly in my pipeline locally (found time to commit today 🥳 ).

This should create a solid JDK 11+ base to look at and incorporate any fixes, changes, tweaks etc. This would be the first version sendable to Maven Central as a snapshot, so we can test within Dataverse code, too.

[landreev]

OK, no problem. I started on it a little bit - but didn't spend too much time on it (had to spend most of my time today on various support/systems things). But I will add some comments here on how we changed the XOAI model of serving metadata in order to use the library with Dataverse.
Thank you again.