From f21b11723d157500692bdf4a023a10b09aca71aa Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 21 Dec 2021 18:33:50 +0100 Subject: [PATCH 01/56] refactor(solr): move schema.xml and script to schema folder, delete static solrconfig.xml #7662 --- conf/solr/8.11.1/readme.md | 1 - conf/solr/8.11.1/solrconfig.xml | 1410 ----------------- conf/solr/{8.11.1 => schema}/schema.xml | 0 conf/solr/{8.11.1 => schema}/update-fields.sh | 0 4 files changed, 1411 deletions(-) delete mode 100644 conf/solr/8.11.1/readme.md delete mode 100644 conf/solr/8.11.1/solrconfig.xml rename conf/solr/{8.11.1 => schema}/schema.xml (100%) rename conf/solr/{8.11.1 => schema}/update-fields.sh (100%) diff --git a/conf/solr/8.11.1/readme.md b/conf/solr/8.11.1/readme.md deleted file mode 100644 index 4457cf9a7df..00000000000 --- a/conf/solr/8.11.1/readme.md +++ /dev/null @@ -1 +0,0 @@ -Please see the dev guide for what to do with Solr config files. \ No newline at end of file diff --git a/conf/solr/8.11.1/solrconfig.xml b/conf/solr/8.11.1/solrconfig.xml deleted file mode 100644 index 3e4e5adc7b6..00000000000 --- a/conf/solr/8.11.1/solrconfig.xml +++ /dev/null @@ -1,1410 +0,0 @@ - - - - - - - - - 7.3.0 - - - - - - - - - - - - - - - - - - - - ${solr.data.dir:} - - - - - - - - - - - - - - - - - - - - - - - - - - - - - ${solr.lock.type:native} - - - - - - - - - - - - - - - - - - - - - - - - - - - - - ${solr.ulog.dir:} - ${solr.ulog.numVersionBuckets:65536} - - - - - ${solr.autoCommit.maxTime:15000} - false - - - - - - ${solr.autoSoftCommit.maxTime:-1} - - - - - - - - - - - - - - 1024 - - - - - - - - - - - - - - - - - - - - - - - - true - - - - - - 20 - - - 200 - - - - - - - - - - - - - - - - false - - - - - - - - - - - - - - - - - - - - - - explicit - 10 - edismax - 0.075 - - dvName^400 - authorName^180 - dvSubject^190 - dvDescription^180 - dvAffiliation^170 - title^130 - subject^120 - keyword^110 - topicClassValue^100 - dsDescriptionValue^90 - authorAffiliation^80 - publicationCitation^60 - producerName^50 - fileName^30 - fileDescription^30 - variableLabel^20 - variableName^10 - _text_^1.0 - - - dvName^200 - authorName^100 - dvSubject^100 - dvDescription^100 - dvAffiliation^100 - title^75 - subject^75 - keyword^75 - topicClassValue^75 - dsDescriptionValue^75 - authorAffiliation^75 - publicationCitation^75 - producerName^75 - - - - isHarvested:false^25000 - - - - - - - - - - - - - - - - - - explicit - json - true - - - - - - - - explicit - - - - - - _text_ - - - - - - - true - ignored_ - _text_ - - - - - - - - - text_general - - - - - - default - _text_ - solr.DirectSolrSpellChecker - - internal - - 0.5 - - 2 - - 1 - - 5 - - 4 - - 0.01 - - - - - - - - - - - - default - on - true - 10 - 5 - 5 - true - true - 10 - 5 - - - spellcheck - - - - - - - - - - true - - - tvComponent - - - - - - - - - - - - true - false - - - terms - - - - - - - - string - - - - - - explicit - - - elevator - - - - - - - - - - - 100 - - - - - - - - 70 - - 0.5 - - [-\w ,/\n\"']{20,200} - - - - - - - ]]> - ]]> - - - - - - - - - - - - - - - - - - - - - - - - ,, - ,, - ,, - ,, - ,]]> - ]]> - - - - - - 10 - .,!? - - - - - - - WORD - - - en - US - - - - - - - - - - - - - - [^\w-\.] - _ - - - - - - - yyyy-MM-dd'T'HH:mm:ss.SSSZ - yyyy-MM-dd'T'HH:mm:ss,SSSZ - yyyy-MM-dd'T'HH:mm:ss.SSS - yyyy-MM-dd'T'HH:mm:ss,SSS - yyyy-MM-dd'T'HH:mm:ssZ - yyyy-MM-dd'T'HH:mm:ss - yyyy-MM-dd'T'HH:mmZ - yyyy-MM-dd'T'HH:mm - yyyy-MM-dd HH:mm:ss.SSSZ - yyyy-MM-dd HH:mm:ss,SSSZ - yyyy-MM-dd HH:mm:ss.SSS - yyyy-MM-dd HH:mm:ss,SSS - yyyy-MM-dd HH:mm:ssZ - yyyy-MM-dd HH:mm:ss - yyyy-MM-dd HH:mmZ - yyyy-MM-dd HH:mm - yyyy-MM-dd - - - - - - - - - - - - - - - - - - - - - - - - - - - - - text/plain; charset=UTF-8 - - - - - ${velocity.template.base.dir:} - ${velocity.solr.resource.loader.enabled:true} - ${velocity.params.resource.loader.enabled:false} - - - - - 5 - - - - - - - - - - - - - - diff --git a/conf/solr/8.11.1/schema.xml b/conf/solr/schema/schema.xml similarity index 100% rename from conf/solr/8.11.1/schema.xml rename to conf/solr/schema/schema.xml diff --git a/conf/solr/8.11.1/update-fields.sh b/conf/solr/schema/update-fields.sh similarity index 100% rename from conf/solr/8.11.1/update-fields.sh rename to conf/solr/schema/update-fields.sh From 60a263d2220f15f4a9bf366f71d86135d95ddf56 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 21 Dec 2021 18:34:37 +0100 Subject: [PATCH 02/56] feat(solr): add XSLT scripts to edit solrconfig.xml with our changes Dataverse specific changes #7662 --- conf/solr/config/disable-schemaless.xslt | 27 ++++++++ conf/solr/config/search-boosting.xslt | 72 +++++++++++++++++++++ conf/solr/config/static-schema-factory.xslt | 21 ++++++ 3 files changed, 120 insertions(+) create mode 100644 conf/solr/config/disable-schemaless.xslt create mode 100644 conf/solr/config/search-boosting.xslt create mode 100644 conf/solr/config/static-schema-factory.xslt diff --git a/conf/solr/config/disable-schemaless.xslt b/conf/solr/config/disable-schemaless.xslt new file mode 100644 index 00000000000..d2b5e32904a --- /dev/null +++ b/conf/solr/config/disable-schemaless.xslt @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/conf/solr/config/search-boosting.xslt b/conf/solr/config/search-boosting.xslt new file mode 100644 index 00000000000..915c98ce600 --- /dev/null +++ b/conf/solr/config/search-boosting.xslt @@ -0,0 +1,72 @@ + + + + + + + + + + + + + + + + + + + edismax + 0.075 + + dvName^400 + authorName^180 + dvSubject^190 + dvDescription^180 + dvAffiliation^170 + title^130 + subject^120 + keyword^110 + topicClassValue^100 + dsDescriptionValue^90 + authorAffiliation^80 + publicationCitation^60 + producerName^50 + fileName^30 + fileDescription^30 + variableLabel^20 + variableName^10 + _text_^1.0 + + + dvName^200 + authorName^100 + dvSubject^100 + dvDescription^100 + dvAffiliation^100 + title^75 + subject^75 + keyword^75 + topicClassValue^75 + dsDescriptionValue^75 + authorAffiliation^75 + publicationCitation^75 + producerName^75 + + + + isHarvested:false^25000 + + + + \ No newline at end of file diff --git a/conf/solr/config/static-schema-factory.xslt b/conf/solr/config/static-schema-factory.xslt new file mode 100644 index 00000000000..1ed44f2e403 --- /dev/null +++ b/conf/solr/config/static-schema-factory.xslt @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file From c5d3a09cfebf3d3873500c0cb1d7aee4f98debc9 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 23 Dec 2021 11:47:32 +0100 Subject: [PATCH 03/56] feat(solr): make schema factory XSLT idempotent #7662 --- conf/solr/config/static-schema-factory.xslt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/conf/solr/config/static-schema-factory.xslt b/conf/solr/config/static-schema-factory.xslt index 1ed44f2e403..e3b9c34d437 100644 --- a/conf/solr/config/static-schema-factory.xslt +++ b/conf/solr/config/static-schema-factory.xslt @@ -11,9 +11,11 @@ - + - + + + From 4bbe1e9a3f0167916546207b033845dfa1c958b2 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 23 Dec 2021 13:07:26 +0100 Subject: [PATCH 04/56] feat(solr): make search boosting XSLT idempotent #7662 --- conf/solr/config/search-boosting.xslt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/conf/solr/config/search-boosting.xslt b/conf/solr/config/search-boosting.xslt index 915c98ce600..86750328d24 100644 --- a/conf/solr/config/search-boosting.xslt +++ b/conf/solr/config/search-boosting.xslt @@ -14,7 +14,8 @@ - + + edismax 0.075 From 1a24a5008fdb057eca86de44d022c23afcfb1ba7 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 23 Dec 2021 13:10:35 +0100 Subject: [PATCH 05/56] fix(solr): adapt pathes in shellspec tests for update-fields.sh #7662 --- tests/shell/spec/update_fields_spec.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/shell/spec/update_fields_spec.sh b/tests/shell/spec/update_fields_spec.sh index e77121672dd..f1e98708df5 100644 --- a/tests/shell/spec/update_fields_spec.sh +++ b/tests/shell/spec/update_fields_spec.sh @@ -1,16 +1,16 @@ #shellcheck shell=sh update_fields() { - ../../conf/solr/8.11.1/update-fields.sh "$@" + ../../conf/solr/schema/update-fields.sh "$@" } Describe "Update fields command" Describe "can operate on upstream data" - copyUpstreamSchema() { cp ../../conf/solr/8.11.1/schema.xml data/solr/upstream-schema.xml; } + copyUpstreamSchema() { cp ../../conf/solr/schema/schema.xml data/solr/upstream-schema.xml; } AfterAll 'copyUpstreamSchema' - Path schema-xml="../../conf/solr/8.11.1/schema.xml" + Path schema-xml="../../conf/solr/schema/schema.xml" It "needs upstream schema.xml" The path schema-xml should be exist End From a6fdaa8e4b4376f97fc93d663f754cfa4ccaae46 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 23 Dec 2021 15:34:07 +0100 Subject: [PATCH 06/56] docs(solr): make Sphinx read Solr version from Maven pom.xml #7662 --- doc/sphinx-guides/source/conf.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/sphinx-guides/source/conf.py b/doc/sphinx-guides/source/conf.py index 42988690329..03f9e51ffb5 100755 --- a/doc/sphinx-guides/source/conf.py +++ b/doc/sphinx-guides/source/conf.py @@ -18,6 +18,9 @@ sys.path.insert(0, os.path.abspath('../../')) import sphinx_bootstrap_theme +import xml.etree.ElementTree as et +pom = et.parse("../../../pom.xml") +ns = {"mvn": "http://maven.apache.org/POM/4.0.0"} # Activate the theme. # html_theme = 'bootstrap' @@ -433,4 +436,5 @@ rst_prolog = """ .. |toctitle| replace:: Contents: .. |anotherSub| replace:: Yes, there can be multiple. -""" +.. |solr_version| replace:: {solr_version} +""".format(solr_version=pom.find("./mvn:properties/mvn:solr.version", ns).text) From 3641f600b7dc4631bb4d331f8a38377102670615 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 23 Dec 2021 15:35:01 +0100 Subject: [PATCH 07/56] docs(solr): use Sphinx substitution for Solr version in installation guide #7662 --- .../source/installation/prerequisites.rst | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/doc/sphinx-guides/source/installation/prerequisites.rst b/doc/sphinx-guides/source/installation/prerequisites.rst index ac8beb0f284..e8650bc335c 100644 --- a/doc/sphinx-guides/source/installation/prerequisites.rst +++ b/doc/sphinx-guides/source/installation/prerequisites.rst @@ -157,34 +157,37 @@ The Dataverse Software search index is powered by Solr. Supported Versions ================== -The Dataverse Software has been tested with Solr version 8.11.1. Future releases in the 8.x series are likely to be compatible; however, this cannot be confirmed until they are officially tested. Major releases above 8.x (e.g. 9.x) are not supported. +The Dataverse Software has been tested with Solr version |solr_version|. Future releases in the 8.x series are likely to be compatible; however, this cannot be confirmed until they are officially tested. Major releases above 8.x (e.g. 9.x) are not supported. Installing Solr =============== -You should not run Solr as root. Create a user called ``solr`` and a directory to install Solr into:: +You should not run Solr as root. Create a user called ``solr`` and a directory to install Solr into: +.. parsed-literal:: useradd solr mkdir /usr/local/solr chown solr:solr /usr/local/solr -Become the ``solr`` user and then download and configure Solr:: +Become the ``solr`` user and then download and configure Solr: +.. parsed-literal:: su - solr cd /usr/local/solr - wget https://archive.apache.org/dist/lucene/solr/8.11.1/solr-8.11.1.tgz - tar xvzf solr-8.11.1.tgz - cd solr-8.11.1 + wget https://archive.apache.org/dist/lucene/solr/|solr_version|/solr-|solr_version|.tgz + tar xvzf solr-|solr_version|.tgz + cd solr-|solr_version| cp -r server/solr/configsets/_default server/solr/collection1 -You should already have a "dvinstall.zip" file that you downloaded from https://github.com/IQSS/dataverse/releases . Unzip it into ``/tmp``. Then copy the files into place:: +You should already have a "dvinstall.zip" file that you downloaded from https://github.com/IQSS/dataverse/releases . Unzip it into ``/tmp``. Then copy the files into place: - cp /tmp/dvinstall/schema*.xml /usr/local/solr/solr-8.11.1/server/solr/collection1/conf - cp /tmp/dvinstall/solrconfig.xml /usr/local/solr/solr-8.11.1/server/solr/collection1/conf +.. parsed-literal:: + cp /tmp/dvinstall/schema*.xml /usr/local/solr/solr-|solr_version|/server/solr/collection1/conf + cp /tmp/dvinstall/solrconfig.xml /usr/local/solr/solr-|solr_version|/server/solr/collection1/conf Note: The Dataverse Project team has customized Solr to boost results that come from certain indexed elements inside the Dataverse installation, for example prioritizing results from Dataverse collections over Datasets. If you would like to remove this, edit your ``solrconfig.xml`` and remove the ```` element and its contents. If you have ideas about how this boosting could be improved, feel free to contact us through our Google Group https://groups.google.com/forum/#!forum/dataverse-dev . -A Dataverse installation requires a change to the ``jetty.xml`` file that ships with Solr. Edit ``/usr/local/solr/solr-8.11.1/server/etc/jetty.xml`` , increasing ``requestHeaderSize`` from ``8192`` to ``102400`` +A Dataverse installation requires a change to the ``jetty.xml`` file that ships with Solr. Edit ``/usr/local/solr/*/server/etc/jetty.xml`` , increasing ``requestHeaderSize`` from ``8192`` to ``102400`` Solr will warn about needing to increase the number of file descriptors and max processes in a production environment but will still run with defaults. We have increased these values to the recommended levels by adding ulimit -n 65000 to the init script, and the following to ``/etc/security/limits.conf``:: @@ -201,9 +204,11 @@ Solr launches asynchronously and attempts to use the ``lsof`` binary to watch fo # yum install lsof -Finally, you need to tell Solr to create the core "collection1" on startup:: +Finally, you need to tell Solr to create the core "collection1" on startup: - echo "name=collection1" > /usr/local/solr/solr-8.11.1/server/solr/collection1/core.properties +.. parsed-literal:: + + echo "name=collection1" > /usr/local/solr/solr-|solr_version|/server/solr/collection1/core.properties Solr Init Script ================ From 69f66f566ff2b76ef8b143182e9df521ccda5b60 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 23 Dec 2021 15:38:24 +0100 Subject: [PATCH 08/56] docs(metadata): fix update-fields.sh include path #7662 --- doc/sphinx-guides/source/admin/metadatacustomization.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/sphinx-guides/source/admin/metadatacustomization.rst b/doc/sphinx-guides/source/admin/metadatacustomization.rst index 9fb10099f22..da9f8b75193 100644 --- a/doc/sphinx-guides/source/admin/metadatacustomization.rst +++ b/doc/sphinx-guides/source/admin/metadatacustomization.rst @@ -644,7 +644,7 @@ the Solr schema configuration, including any enabled metadata schemas: ``curl "http://localhost:8080/api/admin/index/solr/schema"`` -You can use :download:`update-fields.sh <../../../../conf/solr/8.11.1/update-fields.sh>` to easily add these to the +You can use :download:`update-fields.sh <../../../../conf/solr/schema/update-fields.sh>` to easily add these to the Solr schema you installed for your Dataverse installation. The script needs a target XML file containing your Solr schema. (See the :doc:`/installation/prerequisites/` section of From 2d506dbb5041fe6ff9a88d124fc26a930aa34e6e Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 3 Jan 2022 15:06:44 +0100 Subject: [PATCH 09/56] feat(solr): add Makefile to create Dataverse ConfigSet #7662 Simple Makefile to download Solr, extract the default configset and create a Dataverse flavored one. - Uses Maven to find the Solr distribution version to download. - Uses xsltproc to apply our XSLT transformations to sorlconfig.xml - Replaces the managed-schema with the static one we provide - Zips the configset to make it distributable as artifact --- conf/solr/.gitignore | 1 + conf/solr/Makefile | 80 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 conf/solr/.gitignore create mode 100644 conf/solr/Makefile diff --git a/conf/solr/.gitignore b/conf/solr/.gitignore new file mode 100644 index 00000000000..53c37a16608 --- /dev/null +++ b/conf/solr/.gitignore @@ -0,0 +1 @@ +dist \ No newline at end of file diff --git a/conf/solr/Makefile b/conf/solr/Makefile new file mode 100644 index 00000000000..83045b7d707 --- /dev/null +++ b/conf/solr/Makefile @@ -0,0 +1,80 @@ +DIST_DIR := ./dist +DOWNLOAD_DIR := $(DIST_DIR)/download +EXTRACT_DIR := $(DIST_DIR)/extract +POM_LOCATION := ../../pom.xml +XSLT_LOCATION := ./config +XSLT_FILES := $(wildcard $(XSLT_LOCATION)/*.xslt) +SOLR_VERS_FILE := $(DIST_DIR)/solr-version +SOLR_TARGZ := $(DOWNLOAD_DIR)/solr.tar.gz +SOLR_DEFAULT_CS := $(EXTRACT_DIR)/_default +SOLR_CONFIGSET := $(DIST_DIR)/dataverse_cs +SOLR_CFG_SRC := $(SOLR_DEFAULT_CS)/conf/solrconfig.xml +SOLR_CFG_PATCHED := $(SOLR_CONFIGSET)/conf/solrconfig.xml +SOLR_CFG_TMP := $(DIST_DIR)/solrconfig.xml.tmp +SOLR_SCHEMA_DST := $(SOLR_CONFIGSET)/conf/schema.xml +SOLR_SCHEMA_SRC := ./schema/schema.xml +SOLR_CONFIGSET_ZIP := $(DIST_DIR)/solr-configset.zip + +.PHONY: all clean clean-all get-solr prepare-solr patch-solr package + +all: patch-solr + +clean: + rm -rf $(EXTRACT_DIR) $(SOLR_CONFIGSET) $(SOLR_CONFIGSET_ZIP) $(SOLR_VERS_FILE) + +clean-all: + rm -rf $(DIST_DIR) + +get-solr: $(DOWNLOAD_DIR) $(SOLR_TARGZ) + +prepare-solr: $(EXTRACT_DIR) get-solr $(SOLR_CONFIGSET) + +patch-solr: prepare-solr $(SOLR_CFG_PATCHED) $(SOLR_SCHEMA_DST) + +package: patch-solr $(SOLR_CONFIGSET_ZIP) + +$(SOLR_CONFIGSET_ZIP): $(SOLR_CFG_PATCHED) $(SOLR_SCHEMA_DST) + $(info Zipping the Dataverse Solr ConfigSet for distribution) + cd "$(SOLR_CONFIGSET)" && zip -9Tru "$(abspath $(SOLR_CONFIGSET_ZIP))" * + +$(SOLR_SCHEMA_DST): $(SOLR_SCHEMA_SRC) + $(info Delete managed-schema and copy static schema into Dataverse ConfigSet) + -rm $(SOLR_CONFIGSET)/conf/managed-schema + cp "$(SOLR_SCHEMA_SRC)" "$(SOLR_SCHEMA_DST)" + +$(SOLR_CFG_PATCHED): $(XSLT_FILES) + $(info Dataverse ConfigSet: Copy default solrconfig.xml and patch with XSLT) + cp "$(SOLR_CFG_SRC)" "$(SOLR_CFG_PATCHED)" + for XSLT in $(XSLT_FILES); do xsltproc "$$XSLT" "$(SOLR_CFG_PATCHED)" > "$(SOLR_CFG_TMP)" && mv "$(SOLR_CFG_TMP)" "$(SOLR_CFG_PATCHED)"; done + +$(SOLR_CONFIGSET): $(SOLR_DEFAULT_CS) + $(info Copy default ConfigSet into a Dataverse flavored one) + [ -d "$(SOLR_CONFIGSET)" ] && rm -rf "$(SOLR_CONFIGSET)" || true + cp -a "$(SOLR_DEFAULT_CS)" "$(SOLR_CONFIGSET)" + rm "$(SOLR_CFG_PATCHED)" # initialy delete the config file - we will add a patched one in the next step + +$(SOLR_DEFAULT_CS): $(SOLR_TARGZ) $(SOLR_VERS_FILE) + $(info Extracting default ConfigSet from distribution) + $(eval SOLR_VERSION := $(shell cat "$(SOLR_VERS_FILE)")) + $(eval SOLR_TARGZ_SUBPATH := solr-$(SOLR_VERSION)/server/solr/configsets/_default) + tar -xmzf "$(SOLR_TARGZ)" -C "$(EXTRACT_DIR)" "$(SOLR_TARGZ_SUBPATH)" + cp -r "$(EXTRACT_DIR)/$(SOLR_TARGZ_SUBPATH)" "$(EXTRACT_DIR)" + rm -rf "$(EXTRACT_DIR)/solr-$(SOLR_VERSION)" + +$(SOLR_TARGZ): $(SOLR_VERS_FILE) + $(info Downloading Solr binary distribution) + $(eval SOLR_VERSION := $(shell cat "$(SOLR_VERS_FILE)")) + $(eval SOLR_DOWNLOAD_URL := https://archive.apache.org/dist/lucene/solr/$(SOLR_VERSION)/solr-$(SOLR_VERSION).tgz) + curl -fsS "$(SOLR_DOWNLOAD_URL)" -o "$(SOLR_TARGZ)" + +$(SOLR_VERS_FILE): $(POM_LOCATION) + $(info Extracting Solr version from Maven POM) + [ -d "$(DIST_DIR)" ] || mkdir -p "$(DIST_DIR)" + mvn -f "$(POM_LOCATION)" help:evaluate -Dexpression=solr.version -q -DforceStdout > "$(SOLR_VERS_FILE)" + +$(DOWNLOAD_DIR): + [ -d "$(DOWNLOAD_DIR)" ] || mkdir -p "$(DOWNLOAD_DIR)" + +$(EXTRACT_DIR): + [ -d "$(EXTRACT_DIR)" ] || mkdir -p "$(EXTRACT_DIR)" + From 76a9a999c16f1621108f2d79b4cd583023cdf597 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 3 Jan 2022 17:19:50 +0100 Subject: [PATCH 10/56] fix(solr): make XSLTs output XML proc line & comments #7662 --- conf/solr/config/disable-schemaless.xslt | 3 +-- conf/solr/config/search-boosting.xslt | 29 +++++++++------------ conf/solr/config/static-schema-factory.xslt | 3 +-- 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/conf/solr/config/disable-schemaless.xslt b/conf/solr/config/disable-schemaless.xslt index d2b5e32904a..a9a9da12229 100644 --- a/conf/solr/config/disable-schemaless.xslt +++ b/conf/solr/config/disable-schemaless.xslt @@ -1,7 +1,6 @@ - - + diff --git a/conf/solr/config/search-boosting.xslt b/conf/solr/config/search-boosting.xslt index 86750328d24..2d4e8d39987 100644 --- a/conf/solr/config/search-boosting.xslt +++ b/conf/solr/config/search-boosting.xslt @@ -1,7 +1,6 @@ - - + @@ -13,20 +12,16 @@ - - - - + + + This boosting configuration has been + first introduced in 2015, see https://github.com/IQSS/dataverse/issues/1928#issuecomment-91651853, + been re-introduced in 2018 for Solr 7.2.1 update, see https://github.com/IQSS/dataverse/issues/4158, + and finally evolved to the current state later in 2018 https://github.com/IQSS/dataverse/issues/4938 + (merged with https://github.com/IQSS/dataverse/commit/3843e5366845d55c327cdb252dd9b4e4125b9b88) + + Since then, this has not been touched again (2021-12-21). + edismax 0.075 @@ -64,7 +59,7 @@ publicationCitation^75 producerName^75 - + Even though this number is huge it only seems to apply a boost of ~1.5x to final result -MAD 4.9.3 isHarvested:false^25000 diff --git a/conf/solr/config/static-schema-factory.xslt b/conf/solr/config/static-schema-factory.xslt index e3b9c34d437..f154965b30a 100644 --- a/conf/solr/config/static-schema-factory.xslt +++ b/conf/solr/config/static-schema-factory.xslt @@ -1,7 +1,6 @@ - - + From ebb7ddfad8926caeaf36fa8cf92c1cfe028ac56b Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 28 Feb 2022 20:59:42 +0100 Subject: [PATCH 11/56] refactor(deps): move SolrJ dependency to parent POM #7662 As the SolrJ dependency is going to be used within more than the WAR module, it is getting moved to the parent POM. In the same go, the dependency is versioned with a property "solr.version", that makes it easy to switch to a different Solr version. The property has been present before, but now is updated to 8.11.1 to reflect the latest upstream changes. The Sphinx guide conf.py is also updated to retrieve the property from the parent POM instead of the WAR POM. --- doc/sphinx-guides/source/conf.py | 2 +- modules/dataverse-parent/pom.xml | 7 ++++++- pom.xml | 1 - 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/doc/sphinx-guides/source/conf.py b/doc/sphinx-guides/source/conf.py index 03f9e51ffb5..eabb77e5323 100755 --- a/doc/sphinx-guides/source/conf.py +++ b/doc/sphinx-guides/source/conf.py @@ -19,7 +19,7 @@ import sphinx_bootstrap_theme import xml.etree.ElementTree as et -pom = et.parse("../../../pom.xml") +pom = et.parse("../../../modules/dataverse-parent/pom.xml") ns = {"mvn": "http://maven.apache.org/POM/4.0.0"} # Activate the theme. diff --git a/modules/dataverse-parent/pom.xml b/modules/dataverse-parent/pom.xml index f20dd34f5f6..6b4b66e3cc7 100644 --- a/modules/dataverse-parent/pom.xml +++ b/modules/dataverse-parent/pom.xml @@ -51,6 +51,11 @@ postgresql ${postgresql.version} + + org.apache.solr + solr-solrj + ${solr.version} + commons-logging commons-logging @@ -148,7 +153,7 @@ 5.2021.5 42.3.3 - 8.8.1 + 8.11.1 1.11.762 0.157.0 diff --git a/pom.xml b/pom.xml index 7663bb3e6e6..4c574f6ace5 100644 --- a/pom.xml +++ b/pom.xml @@ -240,7 +240,6 @@ org.apache.solr solr-solrj - 8.11.1 colt From 17555c86bd6f1b4867a9594224cb0761bfde2b03 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 28 Feb 2022 21:01:09 +0100 Subject: [PATCH 12/56] refactor(deps): move JUnit Jupiter dependency to parent POM #7662 As the JUnit Jupiter dependency is going to be used within more than the WAR module, it is getting moved to the parent POM under dependencyManagement control. --- modules/dataverse-parent/pom.xml | 6 ++++++ pom.xml | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/dataverse-parent/pom.xml b/modules/dataverse-parent/pom.xml index 6b4b66e3cc7..f7036f87537 100644 --- a/modules/dataverse-parent/pom.xml +++ b/modules/dataverse-parent/pom.xml @@ -129,6 +129,12 @@ import pom + + + org.junit.jupiter + junit-jupiter + ${junit.jupiter.version} + diff --git a/pom.xml b/pom.xml index 4c574f6ace5..ac50373eb08 100644 --- a/pom.xml +++ b/pom.xml @@ -507,7 +507,6 @@ org.junit.jupiter junit-jupiter - ${junit.jupiter.version} test From a7ffb29b1803f3fc0276d42a9a32d8a9cc55c291 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 28 Feb 2022 21:05:55 +0100 Subject: [PATCH 13/56] refactor(solr): move XSLT and Schema to solr-configset module #7662 This introduces a few files of the future Maven module to carry all of the Solr configuration by moving them from conf/solr to modules/solr-configset/src/resources. --- .../solr-configset/src/main/resources}/schema.xml | 0 .../src/main/resources/solrconfig-xslt/01-disable-schemaless.xslt | 0 .../src/main/resources/solrconfig-xslt/02-search-boosting.xslt | 0 .../main/resources/solrconfig-xslt/03-static-schema-factory.xslt | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename {conf/solr/schema => modules/solr-configset/src/main/resources}/schema.xml (100%) rename conf/solr/config/disable-schemaless.xslt => modules/solr-configset/src/main/resources/solrconfig-xslt/01-disable-schemaless.xslt (100%) rename conf/solr/config/search-boosting.xslt => modules/solr-configset/src/main/resources/solrconfig-xslt/02-search-boosting.xslt (100%) rename conf/solr/config/static-schema-factory.xslt => modules/solr-configset/src/main/resources/solrconfig-xslt/03-static-schema-factory.xslt (100%) diff --git a/conf/solr/schema/schema.xml b/modules/solr-configset/src/main/resources/schema.xml similarity index 100% rename from conf/solr/schema/schema.xml rename to modules/solr-configset/src/main/resources/schema.xml diff --git a/conf/solr/config/disable-schemaless.xslt b/modules/solr-configset/src/main/resources/solrconfig-xslt/01-disable-schemaless.xslt similarity index 100% rename from conf/solr/config/disable-schemaless.xslt rename to modules/solr-configset/src/main/resources/solrconfig-xslt/01-disable-schemaless.xslt diff --git a/conf/solr/config/search-boosting.xslt b/modules/solr-configset/src/main/resources/solrconfig-xslt/02-search-boosting.xslt similarity index 100% rename from conf/solr/config/search-boosting.xslt rename to modules/solr-configset/src/main/resources/solrconfig-xslt/02-search-boosting.xslt diff --git a/conf/solr/config/static-schema-factory.xslt b/modules/solr-configset/src/main/resources/solrconfig-xslt/03-static-schema-factory.xslt similarity index 100% rename from conf/solr/config/static-schema-factory.xslt rename to modules/solr-configset/src/main/resources/solrconfig-xslt/03-static-schema-factory.xslt From 9954ba1c646b22b8272d6814147c33c906201bad Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 28 Feb 2022 21:28:11 +0100 Subject: [PATCH 14/56] refactor(solr): move update-fields.sh to solr-configset module #7662 This commit moves the update script from conf/solr to the new Maven submodule hosting the Solr configuration bits. Also changes the Shellspec scripts, Makefile and Sphinx guides to reflect the move. --- doc/sphinx-guides/source/admin/metadatacustomization.rst | 2 +- .../solr-configset/src/main/scripts}/update-fields.sh | 0 scripts/installer/Makefile | 4 ++-- tests/shell/spec/update_fields_spec.sh | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) rename {conf/solr/schema => modules/solr-configset/src/main/scripts}/update-fields.sh (100%) diff --git a/doc/sphinx-guides/source/admin/metadatacustomization.rst b/doc/sphinx-guides/source/admin/metadatacustomization.rst index da9f8b75193..210e0940afd 100644 --- a/doc/sphinx-guides/source/admin/metadatacustomization.rst +++ b/doc/sphinx-guides/source/admin/metadatacustomization.rst @@ -644,7 +644,7 @@ the Solr schema configuration, including any enabled metadata schemas: ``curl "http://localhost:8080/api/admin/index/solr/schema"`` -You can use :download:`update-fields.sh <../../../../conf/solr/schema/update-fields.sh>` to easily add these to the +You can use :download:`update-fields.sh <../../../../modules/solr-configset/src/main/scripts/update-fields.sh>` to easily add these to the Solr schema you installed for your Dataverse installation. The script needs a target XML file containing your Solr schema. (See the :doc:`/installation/prerequisites/` section of diff --git a/conf/solr/schema/update-fields.sh b/modules/solr-configset/src/main/scripts/update-fields.sh similarity index 100% rename from conf/solr/schema/update-fields.sh rename to modules/solr-configset/src/main/scripts/update-fields.sh diff --git a/scripts/installer/Makefile b/scripts/installer/Makefile index 1722f208175..4783b01daea 100644 --- a/scripts/installer/Makefile +++ b/scripts/installer/Makefile @@ -56,9 +56,9 @@ ${JHOVE_SCHEMA}: ../../conf/jhove/jhoveConfig.xsd ${INSTALLER_ZIP_DIR} @echo copying jhove schema file /bin/cp ../../conf/jhove/jhoveConfig.xsd ${INSTALLER_ZIP_DIR} -${SOLR_SCHEMA}: ../../conf/solr/8.11.1/schema.xml ../../conf/solr/8.11.1/update-fields.sh ${INSTALLER_ZIP_DIR} +${SOLR_SCHEMA}: ../../conf/solr/8.11.1/schema.xml ../../modules/solr-configset/src/main/scripts/update-fields.sh ${INSTALLER_ZIP_DIR} @echo copying Solr schema file - /bin/cp ../../conf/solr/8.11.1/schema.xml ../../conf/solr/8.11.1/update-fields.sh ${INSTALLER_ZIP_DIR} + /bin/cp ../../conf/solr/8.11.1/schema.xml ../../modules/solr-configset/src/main/scripts/update-fields.sh ${INSTALLER_ZIP_DIR} ${SOLR_CONFIG}: ../../conf/solr/8.11.1/solrconfig.xml ${INSTALLER_ZIP_DIR} @echo copying Solr config file diff --git a/tests/shell/spec/update_fields_spec.sh b/tests/shell/spec/update_fields_spec.sh index f1e98708df5..be86444d8a6 100644 --- a/tests/shell/spec/update_fields_spec.sh +++ b/tests/shell/spec/update_fields_spec.sh @@ -1,16 +1,16 @@ #shellcheck shell=sh update_fields() { - ../../conf/solr/schema/update-fields.sh "$@" + ../../modules/solr-configset/src/main/scripts/update-fields.sh "$@" } Describe "Update fields command" Describe "can operate on upstream data" - copyUpstreamSchema() { cp ../../conf/solr/schema/schema.xml data/solr/upstream-schema.xml; } + copyUpstreamSchema() { cp ../../modules/solr-configset/src/main/resources/schema.xml data/solr/upstream-schema.xml; } AfterAll 'copyUpstreamSchema' - Path schema-xml="../../conf/solr/schema/schema.xml" + Path schema-xml="../../modules/solr-configset/src/main/resources/schema.xml" It "needs upstream schema.xml" The path schema-xml should be exist End From eb2be9c5daaa808a8683986cd717f2216530333c Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 28 Feb 2022 22:20:31 +0100 Subject: [PATCH 15/56] feat!(solr): create Maven module solr-configset #7662 With this commit, a new Maven submodule is introduced: creating a distributable configset to create Solr cores from. The configset consists of a customized core configuration, a schema and more to create indices from incoming Solr docs. As the solrconfig.xml file depends on the Solr/Lucene version in use under the hood and requires careful tweaking for usage within a Dataverse installation, it gets created by a compilation process involving version controlled XSLT files. The schema.xml file is version controlled and depends on the application code. A Solr release version according to the parent POM defined Solr version is downloaded and cached with the local Maven repository. The template for out Dataverse flavored configset is extracted from it. With the resulting configset an embedded Solr instance is started and a core gets created with it to ensure basic functionality of a compile Solr configset and schema.xml. BREAKING CHANGE: :warning: changes Solr configuration method from replacing single files to creation from a template --- modules/dataverse-parent/pom.xml | 1 + modules/solr-configset/pom.xml | 212 +++++++++++++++ .../solr-configset/src/assembly/default.xml | 19 ++ .../solrconfig-xslt/99-fix-special-chars.xslt | 15 ++ .../main/scripts/CompileSolrConfigSet.java | 246 ++++++++++++++++++ .../src/test/java/Constants.java | 3 + .../src/test/java/EmbeddedSolrConfigIT.java | 106 ++++++++ 7 files changed, 602 insertions(+) create mode 100644 modules/solr-configset/pom.xml create mode 100644 modules/solr-configset/src/assembly/default.xml create mode 100644 modules/solr-configset/src/main/resources/solrconfig-xslt/99-fix-special-chars.xslt create mode 100644 modules/solr-configset/src/main/scripts/CompileSolrConfigSet.java create mode 100644 modules/solr-configset/src/test/java/Constants.java create mode 100644 modules/solr-configset/src/test/java/EmbeddedSolrConfigIT.java diff --git a/modules/dataverse-parent/pom.xml b/modules/dataverse-parent/pom.xml index f7036f87537..5ebb0d93930 100644 --- a/modules/dataverse-parent/pom.xml +++ b/modules/dataverse-parent/pom.xml @@ -13,6 +13,7 @@ ../../pom.xml ../../scripts/zipdownload + ../solr-configset + pom + + + + ${project.build.directory}/dataverse + + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + test-compile + test-compile + + testCompile + + + ${target.java.version} + + ${compilerArgument} + + + + + + + + com.googlecode.maven-download-plugin + download-maven-plugin + 1.6.8 + + + download-solr + + + wget + + + https://archive.apache.org/dist/lucene/solr/${solr.version}/solr-${solr.version}.zip + false + ${project.build.directory} + true + + + + + + + dev.jbang + jbang-maven-plugin + 0.0.7 + + + compile-configset + process-resources + + run + + + + --solr-version="${solr.version}" + --solr-zipfile="${project.build.directory}/solr-${solr.version}.zip" + --solr-configset-targetdir="${solr.configset.directory}" + --solr-schemaxml-sourcepath="${project.resources[0].directory}/schema.xml" + --solr-config-xsltdir="${project.resources[0].directory}/solrconfig-xslt" + + + + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + test-solr-config + test + + test + + + + ${project.build.directory} + + + SolrConfig + + **/*Test.java + **/*IT.java + + + + + + + + maven-assembly-plugin + ${maven-assembly-plugin.version} + + false + ${project.artifactId} + + src/assembly/default.xml + + + + + create-archive + package + + single + + + + + + + + + + + + info.picocli + picocli + 4.6.3 + + + net.sf.saxon + Saxon-HE + 10.6 + + + + org.junit.jupiter + junit-jupiter + test + + + org.apache.solr + solr-solrj + test + + + org.apache.solr + solr-core + ${solr.version} + test + + + + + \ No newline at end of file diff --git a/modules/solr-configset/src/assembly/default.xml b/modules/solr-configset/src/assembly/default.xml new file mode 100644 index 00000000000..7a5e6d16f30 --- /dev/null +++ b/modules/solr-configset/src/assembly/default.xml @@ -0,0 +1,19 @@ + + default + + zip + + false + + + ${solr.configset.directory} + dataverse + + **/* + + + + + diff --git a/modules/solr-configset/src/main/resources/solrconfig-xslt/99-fix-special-chars.xslt b/modules/solr-configset/src/main/resources/solrconfig-xslt/99-fix-special-chars.xslt new file mode 100644 index 00000000000..9d6a3e2ee5e --- /dev/null +++ b/modules/solr-configset/src/main/resources/solrconfig-xslt/99-fix-special-chars.xslt @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/modules/solr-configset/src/main/scripts/CompileSolrConfigSet.java b/modules/solr-configset/src/main/scripts/CompileSolrConfigSet.java new file mode 100644 index 00000000000..b4695f50f53 --- /dev/null +++ b/modules/solr-configset/src/main/scripts/CompileSolrConfigSet.java @@ -0,0 +1,246 @@ +///usr/bin/env jbang "$0" "$@" ; exit $? +//DEPS info.picocli:picocli:4.6.3 +//DEPS net.sf.saxon:Saxon-HE:10.6 + +import net.sf.saxon.s9api.Processor; +import net.sf.saxon.s9api.SaxonApiException; +import net.sf.saxon.s9api.Serializer; +import net.sf.saxon.s9api.Xslt30Transformer; +import net.sf.saxon.s9api.XsltCompiler; +import net.sf.saxon.s9api.XsltExecutable; +import picocli.CommandLine; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +import javax.xml.transform.stream.StreamSource; +import java.io.IOException; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +@Command(name = "CompileSolrConfigSet", + mixinStandardHelpOptions = true, + version = "CompileSolrConfigSet 0.1", + description = "CompileSolrConfigSet made with jbang") +class CompileSolrConfigSet implements Callable { + + /** + * A wrapper for Throwables to create a checked exception that leads to aborting the execution + */ + static final class AbortScriptException extends Throwable { + private AbortScriptException() {} + public AbortScriptException(String msg, Throwable cause) { + super(msg, cause); + } + } + + /** + * Static inner logging wrapper for convenience. + * This is here because we don't want to add more clutter of a logging framework + * to the Maven output where we use this from. + */ + static final class Logger { + public static void log(String message) { + System.out.println(message); + } + public static void log(AbortScriptException ex) { + System.out.println(ex.getMessage()); + ex.getCause().printStackTrace(); + } + } + + @Option(required = true, + names = {"--solr-version"}, + description = "The release version of Solr (must match with the ZIP files content)") + private String solrVersion; + + @Option(required = true, + names = {"--solr-zipfile"}, + description = "Path to local Solr ZIP distribution file") + private Path solrZipFile; + + @Option(required = false, + names = {"--solr-configset-zippath"}, + description = "Path within ZIP to _default configset", + defaultValue = "solr-{{ solr.version }}/server/solr/configsets/_default") + private String solrConfigSetZipPath; + + @Option(required = true, + names = {"--solr-configset-targetdir"}, + description = "Path to the new configset directory") + private String solrConfigSetTargetDir; + + @Option(required = true, + names = {"--solr-schemaxml-sourcepath"}, + description = "Path to the schema.xml to include") + private String solrSchemaXmlSourcePath; + + @Option(required = true, + names = {"--solr-config-xsltdir"}, + description = "Path to the directory with XSLTs to adapt solrconfig.xml") + private String solrConfigXSLTDir; + + public static void main(String... args) { + int exitCode = new CommandLine(new CompileSolrConfigSet()).execute(args); + System.exit(exitCode); + } + + /** + * Business logic routine, calling all the execution steps. + * @return The exit code + */ + @Override + public Integer call() throws Exception { // TODO: remove Exception signature + try { + replaceVariables(); + extractConfigSet(); + replaceSchemaXML(); + applySolrConfigXSLT(); + } catch (AbortScriptException e) { + Logger.log(e); + // this might be nicely refactored with exit codes stored within the exception + return CommandLine.ExitCode.SOFTWARE; + } + return CommandLine.ExitCode.OK; + } + + private void replaceVariables() throws AbortScriptException { + this.solrConfigSetZipPath = this.solrConfigSetZipPath.replaceAll("\\Q{{ solr.version }}\\E", solrVersion); + } + + private void extractConfigSet() throws AbortScriptException { + // Wrap the file system in a try-with-resources statement + // to auto-close it when finished and prevent a memory leak + try (FileSystem zipFileSystem = FileSystems.newFileSystem(solrZipFile, null)) { + Path zipSource = zipFileSystem.getPath(solrConfigSetZipPath); + + // TODO: should we delete the target before copying the new content? (Usually this shouldn't change, but better safe than sorry?) + + Logger.log("Starting to extract Solr _default config set..."); + Files.walkFileTree(zipSource, new SimpleFileVisitor<>() { + // Copy the directory structure (skip existing with the same name) + @Override + public FileVisitResult preVisitDirectory(Path zippedDir, BasicFileAttributes attrs) throws IOException { + // Remove the leading path part from the ZIP file structure, as we don't want it in target + String strippedZipPath = zippedDir.toString().substring(solrConfigSetZipPath.length()); + Path targetDir = Path.of(solrConfigSetTargetDir, strippedZipPath); + + try { + Files.copy(zippedDir, targetDir, StandardCopyOption.COPY_ATTRIBUTES); + } catch (FileAlreadyExistsException e) { + // intentional ignore - simply reuse the existing directory + } + + return FileVisitResult.CONTINUE; + } + + // Copy & replace files already present (which makes any run idempotent by deleting the former run) + @Override + public FileVisitResult visitFile(Path zippedFile, BasicFileAttributes attrs) throws IOException { + String strippedZipPath = zippedFile.toString().substring(solrConfigSetZipPath.length()); + Path targetFile = Path.of(solrConfigSetTargetDir, strippedZipPath); + + Files.copy(zippedFile, targetFile, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); + + return FileVisitResult.CONTINUE; + } + }); + + } catch(IOException e) { + throw new AbortScriptException("Extracting from ZIP file "+solrZipFile+" failed", e); + } + } + + private void replaceSchemaXML() throws AbortScriptException { + Logger.log("Starting to replace the Solr schema..."); + + // Delete "managed-schema" file + try { + Path managedSchema = Path.of(solrConfigSetTargetDir, "conf", "managed-schema"); + Files.deleteIfExists(managedSchema); + } catch (IOException e) { + throw new AbortScriptException("Could not delete managed-schema", e); + } + + // Copy schema.xml in place + try { + Path sourceSchema = Path.of(solrSchemaXmlSourcePath); + Path targetSchema = Path.of(solrConfigSetTargetDir, "conf", "schema.xml"); + Files.copy(sourceSchema, targetSchema, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); + } catch (IOException e) { + throw new AbortScriptException("Could not copy schema.xml", e); + } + } + + private void applySolrConfigXSLT() throws AbortScriptException { + Logger.log("Starting to transform solrconfig.xml..."); + + final Path solrConfig = Path.of(solrConfigSetTargetDir, "conf", "solrconfig.xml"); + final Path xsltDir = Path.of(solrConfigXSLTDir); + + // Find all the XSLT files + final List xsltFiles; + try (Stream walk = Files.walk(xsltDir, 2)) { + xsltFiles = walk.filter(path -> path.toFile().isFile()) + .filter(path -> path.toString().endsWith("xslt")) + .sorted() + .collect(Collectors.toList()); + } catch (IOException e) { + throw new AbortScriptException("Could not walk over XSLT files at " + xsltDir, e); + } + + // Log found XSLT files + Logger.log("Found XSLT files in " + solrConfigXSLTDir + ":"); + for (Path xsltFile : xsltFiles) { + Logger.log(xsltFile.toString().substring(xsltDir.toString().length()+1)); + } + + // Setup the XSLT processor + final Processor processor = new Processor(false); + final XsltCompiler compiler = processor.newXsltCompiler(); + + try { + + // First iteration uses initial solrconfig.xml as input source + StreamSource source = new StreamSource(Files.newInputStream(solrConfig)); + + // For every XSLT, we need to do the transformation and rotate the input source afterwards, + // so we apply the next transformation to the already transformed content. + for (Path xsltFile : xsltFiles) { + final XsltExecutable stylesheet = compiler.compile(new StreamSource(Files.newInputStream(xsltFile))); + + // Prepare a fresh temporary file (which will be deleted when we read it back for the next iteration) + final Path tmpFile = Files.createTempFile(null, null); + Serializer out = processor.newSerializer(Files.newOutputStream(tmpFile)); + + // Actual transformation happens + Xslt30Transformer transformer = stylesheet.load30(); + transformer.transform(source, out); + + // Read back the transformed config and rotate the source. The opening option makes the old temp file + // go away after it has been read. + source = new StreamSource(Files.newInputStream(tmpFile, StandardOpenOption.DELETE_ON_CLOSE)); + } + + // The final transformation still reads back the final result, so we need to push InputStream content somewhere + Files.copy(source.getInputStream(), + solrConfig, + StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + throw new AbortScriptException("Could not complete solrconfig.xml compilation", e); + } catch (SaxonApiException e) { + throw new AbortScriptException("XML transformation failed", e); + } + } +} diff --git a/modules/solr-configset/src/test/java/Constants.java b/modules/solr-configset/src/test/java/Constants.java new file mode 100644 index 00000000000..88a6abb8a24 --- /dev/null +++ b/modules/solr-configset/src/test/java/Constants.java @@ -0,0 +1,3 @@ +public class Constants { + public final static String TAG_CONFIG = "SolrConfig"; +} diff --git a/modules/solr-configset/src/test/java/EmbeddedSolrConfigIT.java b/modules/solr-configset/src/test/java/EmbeddedSolrConfigIT.java new file mode 100644 index 00000000000..00abf2929c0 --- /dev/null +++ b/modules/solr-configset/src/test/java/EmbeddedSolrConfigIT.java @@ -0,0 +1,106 @@ +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer; +import org.apache.solr.client.solrj.request.CoreAdminRequest; +import org.apache.solr.client.solrj.request.CoreStatus; +import org.apache.solr.client.solrj.request.SolrPing; +import org.apache.solr.client.solrj.response.SolrPingResponse; +import org.apache.solr.core.NodeConfig; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Calendar; +import java.util.Comparator; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +@Tag(Constants.TAG_CONFIG) +@DisplayName("Embedded Solr Configuration Integration Test") +public class EmbeddedSolrConfigIT { + + final static String buildDirectory = System.getProperty("buildDirectory"); + final static String[] solrHomeSubPath = {"solr"}; + + static Path solrHome; + static Path solrConfigSets; + + final static String collectionName = "collection1"; + final static String configSetName = "dataverse"; + static SolrClient solrClient; + + @BeforeAll + public static void setUp() throws IOException { + checkAndSetupSolrDirectories(); + + // build a node config, so we can start an embedded server with it + final NodeConfig config = new NodeConfig.NodeConfigBuilder("embeddedSolrServerNode", solrHome) + .setConfigSetBaseDirectory(solrConfigSets.toString()) + .build(); + + // create the server + final EmbeddedSolrServer embeddedSolrServer = new EmbeddedSolrServer(config, collectionName); + solrClient = embeddedSolrServer; + } + + @AfterAll + public static void tearDown() throws IOException { + // delete the solr home (so we can run the test again without mvn clean) + try (Stream walk = Files.walk(solrHome)) { + walk.sorted(Comparator.reverseOrder()) + .map(Path::toFile) + //.peek(System.out::println) + .forEach(File::delete); + } + } + + @Test + @DisplayName("Deploy Solr Core from Dataverse ConfigSet") + public void deployDataverseCore() throws SolrServerException, IOException { + assumeTrue(solrClient != null); + + // create the core from our configset + final CoreAdminRequest.Create createRequest = new CoreAdminRequest.Create(); + createRequest.setCoreName(collectionName); + createRequest.setConfigSet(configSetName); + createRequest.process(solrClient); + + // get the core status + final CoreStatus coreStatus = CoreAdminRequest.getCoreStatus(collectionName, solrClient); + assertNotNull(coreStatus); + assertTrue(coreStatus.getCoreStartTime().before(Calendar.getInstance().getTime())); + + // check ping + final SolrPing ping = new SolrPing(); + SolrPingResponse pingResponse = ping.process(solrClient); + assertNotNull(pingResponse); + assertTrue(pingResponse.toString().contains("status=OK")); + } + + + private static void checkAndSetupSolrDirectories() throws IOException { + assertNotNull(buildDirectory); + final Path buildDir = Path.of(buildDirectory); + assertTrue(buildDir.isAbsolute() && + Files.exists(buildDir) && + Files.isDirectory(buildDir) && + Files.isReadable(buildDir) && + Files.isWritable(buildDir)); + + // create the solr home (might be replaced with a memory fs) + solrHome = Path.of(buildDirectory, solrHomeSubPath); + Files.createDirectories(solrHome); + + // we simply reuse the parent directory from the build as our configsets source directory + solrConfigSets = buildDir; + } +} From 91e1983ceb9a37d5425161eec392d1299a86c0a6 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 28 Feb 2022 22:49:15 +0100 Subject: [PATCH 16/56] refactor(solr): remove conf/solr leftovers #7662 --- conf/solr/.gitignore | 1 - conf/solr/Makefile | 80 ------------------- .../files/etc/systemd/solr.service | 1 + 3 files changed, 1 insertion(+), 81 deletions(-) delete mode 100644 conf/solr/.gitignore delete mode 100644 conf/solr/Makefile diff --git a/conf/solr/.gitignore b/conf/solr/.gitignore deleted file mode 100644 index 53c37a16608..00000000000 --- a/conf/solr/.gitignore +++ /dev/null @@ -1 +0,0 @@ -dist \ No newline at end of file diff --git a/conf/solr/Makefile b/conf/solr/Makefile deleted file mode 100644 index 83045b7d707..00000000000 --- a/conf/solr/Makefile +++ /dev/null @@ -1,80 +0,0 @@ -DIST_DIR := ./dist -DOWNLOAD_DIR := $(DIST_DIR)/download -EXTRACT_DIR := $(DIST_DIR)/extract -POM_LOCATION := ../../pom.xml -XSLT_LOCATION := ./config -XSLT_FILES := $(wildcard $(XSLT_LOCATION)/*.xslt) -SOLR_VERS_FILE := $(DIST_DIR)/solr-version -SOLR_TARGZ := $(DOWNLOAD_DIR)/solr.tar.gz -SOLR_DEFAULT_CS := $(EXTRACT_DIR)/_default -SOLR_CONFIGSET := $(DIST_DIR)/dataverse_cs -SOLR_CFG_SRC := $(SOLR_DEFAULT_CS)/conf/solrconfig.xml -SOLR_CFG_PATCHED := $(SOLR_CONFIGSET)/conf/solrconfig.xml -SOLR_CFG_TMP := $(DIST_DIR)/solrconfig.xml.tmp -SOLR_SCHEMA_DST := $(SOLR_CONFIGSET)/conf/schema.xml -SOLR_SCHEMA_SRC := ./schema/schema.xml -SOLR_CONFIGSET_ZIP := $(DIST_DIR)/solr-configset.zip - -.PHONY: all clean clean-all get-solr prepare-solr patch-solr package - -all: patch-solr - -clean: - rm -rf $(EXTRACT_DIR) $(SOLR_CONFIGSET) $(SOLR_CONFIGSET_ZIP) $(SOLR_VERS_FILE) - -clean-all: - rm -rf $(DIST_DIR) - -get-solr: $(DOWNLOAD_DIR) $(SOLR_TARGZ) - -prepare-solr: $(EXTRACT_DIR) get-solr $(SOLR_CONFIGSET) - -patch-solr: prepare-solr $(SOLR_CFG_PATCHED) $(SOLR_SCHEMA_DST) - -package: patch-solr $(SOLR_CONFIGSET_ZIP) - -$(SOLR_CONFIGSET_ZIP): $(SOLR_CFG_PATCHED) $(SOLR_SCHEMA_DST) - $(info Zipping the Dataverse Solr ConfigSet for distribution) - cd "$(SOLR_CONFIGSET)" && zip -9Tru "$(abspath $(SOLR_CONFIGSET_ZIP))" * - -$(SOLR_SCHEMA_DST): $(SOLR_SCHEMA_SRC) - $(info Delete managed-schema and copy static schema into Dataverse ConfigSet) - -rm $(SOLR_CONFIGSET)/conf/managed-schema - cp "$(SOLR_SCHEMA_SRC)" "$(SOLR_SCHEMA_DST)" - -$(SOLR_CFG_PATCHED): $(XSLT_FILES) - $(info Dataverse ConfigSet: Copy default solrconfig.xml and patch with XSLT) - cp "$(SOLR_CFG_SRC)" "$(SOLR_CFG_PATCHED)" - for XSLT in $(XSLT_FILES); do xsltproc "$$XSLT" "$(SOLR_CFG_PATCHED)" > "$(SOLR_CFG_TMP)" && mv "$(SOLR_CFG_TMP)" "$(SOLR_CFG_PATCHED)"; done - -$(SOLR_CONFIGSET): $(SOLR_DEFAULT_CS) - $(info Copy default ConfigSet into a Dataverse flavored one) - [ -d "$(SOLR_CONFIGSET)" ] && rm -rf "$(SOLR_CONFIGSET)" || true - cp -a "$(SOLR_DEFAULT_CS)" "$(SOLR_CONFIGSET)" - rm "$(SOLR_CFG_PATCHED)" # initialy delete the config file - we will add a patched one in the next step - -$(SOLR_DEFAULT_CS): $(SOLR_TARGZ) $(SOLR_VERS_FILE) - $(info Extracting default ConfigSet from distribution) - $(eval SOLR_VERSION := $(shell cat "$(SOLR_VERS_FILE)")) - $(eval SOLR_TARGZ_SUBPATH := solr-$(SOLR_VERSION)/server/solr/configsets/_default) - tar -xmzf "$(SOLR_TARGZ)" -C "$(EXTRACT_DIR)" "$(SOLR_TARGZ_SUBPATH)" - cp -r "$(EXTRACT_DIR)/$(SOLR_TARGZ_SUBPATH)" "$(EXTRACT_DIR)" - rm -rf "$(EXTRACT_DIR)/solr-$(SOLR_VERSION)" - -$(SOLR_TARGZ): $(SOLR_VERS_FILE) - $(info Downloading Solr binary distribution) - $(eval SOLR_VERSION := $(shell cat "$(SOLR_VERS_FILE)")) - $(eval SOLR_DOWNLOAD_URL := https://archive.apache.org/dist/lucene/solr/$(SOLR_VERSION)/solr-$(SOLR_VERSION).tgz) - curl -fsS "$(SOLR_DOWNLOAD_URL)" -o "$(SOLR_TARGZ)" - -$(SOLR_VERS_FILE): $(POM_LOCATION) - $(info Extracting Solr version from Maven POM) - [ -d "$(DIST_DIR)" ] || mkdir -p "$(DIST_DIR)" - mvn -f "$(POM_LOCATION)" help:evaluate -Dexpression=solr.version -q -DforceStdout > "$(SOLR_VERS_FILE)" - -$(DOWNLOAD_DIR): - [ -d "$(DOWNLOAD_DIR)" ] || mkdir -p "$(DOWNLOAD_DIR)" - -$(EXTRACT_DIR): - [ -d "$(EXTRACT_DIR)" ] || mkdir -p "$(EXTRACT_DIR)" - diff --git a/doc/sphinx-guides/source/_static/installation/files/etc/systemd/solr.service b/doc/sphinx-guides/source/_static/installation/files/etc/systemd/solr.service index d89ee108377..591d2ffb68c 100644 --- a/doc/sphinx-guides/source/_static/installation/files/etc/systemd/solr.service +++ b/doc/sphinx-guides/source/_static/installation/files/etc/systemd/solr.service @@ -8,6 +8,7 @@ Type = forking WorkingDirectory = /usr/local/solr/solr-8.11.1 ExecStart = /usr/local/solr/solr-8.11.1/bin/solr start -m 1g -j "jetty.host=127.0.0.1" ExecStop = /usr/local/solr/solr-8.11.1/bin/solr stop +Environment="SOLR_OPTS=-Dsolr.jetty.request.header.size=65535" LimitNOFILE=65000 LimitNPROC=65000 Restart=on-failure From 1f03925eb644087e4b17f77be6d4f2e4f035d136 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 10 Mar 2022 15:59:05 +0100 Subject: [PATCH 17/56] chore(solr): remove unnecessary Solr plugin #7662 Currently the devs consensus is they don't need to run a Solr instance from Maven without a container. --- modules/solr-configset/pom.xml | 44 ---------------------------------- 1 file changed, 44 deletions(-) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index a4aa0a585be..1220658d581 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -132,42 +132,6 @@ - - @@ -199,14 +163,6 @@ ${solr.version} test - \ No newline at end of file From 1e6edfcc5b7be5ee18acd4868dc5b32b9e1edde7 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 10 Mar 2022 15:59:42 +0100 Subject: [PATCH 18/56] deps(solr): switch all deps to scope provided or test #7662 --- modules/solr-configset/pom.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 1220658d581..8b54f49bd42 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -140,11 +140,13 @@ info.picocli picocli 4.6.3 + provided net.sf.saxon Saxon-HE 10.6 + provided From c5fc8ade50bee80ca8c0956e11de2cedf8a4a694 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 11 Mar 2022 10:38:46 +0100 Subject: [PATCH 19/56] build(solr): move zip assembly into default profile #7662 By using a default profile, the assembly of the ZIP file can be skipped when using a different profile, e.g. to build a container. --- modules/solr-configset/pom.xml | 55 ++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 8b54f49bd42..763b0766828 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -15,7 +15,6 @@ pom - ${project.build.directory}/dataverse @@ -111,27 +110,6 @@ - - - maven-assembly-plugin - ${maven-assembly-plugin.version} - - false - ${project.artifactId} - - src/assembly/default.xml - - - - - create-archive - package - - single - - - - @@ -167,4 +145,37 @@ + + + zip + + true + + + + + maven-assembly-plugin + ${maven-assembly-plugin.version} + + false + ${project.artifactId} + + src/assembly/default.xml + + + + + create-archive + package + + single + + + + + + + + + \ No newline at end of file From 2509beb6f455bf1753654e657cbe688b27c96d91 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 11 Mar 2022 10:43:53 +0100 Subject: [PATCH 20/56] build(parent): add a Maven profile "ct" for container image usage - Introducing the Docker Maven Plugin to build/run containers from Maven - Adding a plugin to retrieve git details, included by default in profile "ct" - Activate with `mvn -Pct ...` --- modules/dataverse-parent/pom.xml | 42 ++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/modules/dataverse-parent/pom.xml b/modules/dataverse-parent/pom.xml index 5ebb0d93930..7f53fb7f7c6 100644 --- a/modules/dataverse-parent/pom.xml +++ b/modules/dataverse-parent/pom.xml @@ -194,6 +194,10 @@ 3.0.0-M5 3.3.0 3.1.2 + + + 0.39.1 + ghcr.io @@ -256,6 +260,11 @@ + + io.fabric8 + docker-maven-plugin + ${fabric8-dmp.version} + @@ -327,4 +336,37 @@ --> + + + ct + + + + + + + io.github.git-commit-id + git-commit-id-maven-plugin + 5.0.0 + + + retrieve-git-details + + revision + + initialize + + + + ${project.basedir}/../../.git + UTC + 8 + false + + + + + + + \ No newline at end of file From 52dcf8d09a58a03fe58459265c672f317c2cd103 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 11 Mar 2022 10:45:42 +0100 Subject: [PATCH 21/56] build(solr): make test compile in all cases #7662 Some plugins seem to mess around with the build order, so making the test compile configuration apply to every execution makes it go away. --- modules/solr-configset/pom.xml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 763b0766828..4db0de6308e 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -30,13 +30,14 @@ testCompile - - ${target.java.version} - - ${compilerArgument} - + + + ${target.java.version} + + ${compilerArgument} + From e418ef9c98c84c2f78a05cd801019dcf14902041 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 11 Mar 2022 10:46:27 +0100 Subject: [PATCH 22/56] build(solr): make packaging type configurable via property #7662 Defaults to POM, as before. --- modules/solr-configset/pom.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 4db0de6308e..06155e72f1c 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -12,10 +12,11 @@ solr-configset - pom + ${solr.packaging.type} ${project.build.directory}/dataverse + pom From 9e1dcee02fd0acf55edb64bf49453a87f99ae87c Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 11 Mar 2022 10:48:02 +0100 Subject: [PATCH 23/56] feat(solr): add a build for Dataverse Solr container images #7662 This will simply grab the upstream Solr image and add our configset as a template for new collections. --- modules/solr-configset/pom.xml | 47 ++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 06155e72f1c..83967e6f62b 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -178,6 +178,53 @@ + + + ct + + docker + + + + + + io.fabric8 + docker-maven-plugin + true + + + + solr + gdcc/solr:${revision} + ${ct.registry} + + solr:${solr.version} + + + ${git.build.time} + ]]> + https://k8s-docs.gdcc.io + https://k8s-docs.gdcc.io + https://github.com/gdcc/dataverse/tree/develop%2Bct/modules/solr-configset + ${project.version} + ${git.commit.id.abbrev} + Global Dataverse Community Consortium + Apache-2.0 + gdcc/solr :: Dataverse-ready Solr + This container image provides a Dataverse-ready Solr Search Index. + + + /opt/solr/server/solr/configsets + ${project.basedir}/src/assembly/default.xml + + + + + + + + + \ No newline at end of file From 0a32c2132588662bea831d37170a768a3e0e0570 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 11 Mar 2022 11:08:18 +0100 Subject: [PATCH 24/56] feat(parent,solr): make configset name configurable #7662 Allow easier reuse of the name for run configurations etc. --- modules/dataverse-parent/pom.xml | 1 + modules/solr-configset/pom.xml | 2 +- modules/solr-configset/src/assembly/default.xml | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/dataverse-parent/pom.xml b/modules/dataverse-parent/pom.xml index 7f53fb7f7c6..054316e1d5f 100644 --- a/modules/dataverse-parent/pom.xml +++ b/modules/dataverse-parent/pom.xml @@ -198,6 +198,7 @@ 0.39.1 ghcr.io + dataverse diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 83967e6f62b..7bd0887c8d7 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -15,7 +15,7 @@ ${solr.packaging.type} - ${project.build.directory}/dataverse + ${project.build.directory}/${solr.configset} pom diff --git a/modules/solr-configset/src/assembly/default.xml b/modules/solr-configset/src/assembly/default.xml index 7a5e6d16f30..87487791d81 100644 --- a/modules/solr-configset/src/assembly/default.xml +++ b/modules/solr-configset/src/assembly/default.xml @@ -9,7 +9,7 @@ ${solr.configset.directory} - dataverse + ${solr.configset} **/* From 5b8a0bc23352d311a330e7e2d45adb2776f7ef93 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 11 Mar 2022 19:11:42 +0100 Subject: [PATCH 25/56] refactor(solr,parent): make configsets target path a reusable property #7662 --- modules/solr-configset/pom.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 7bd0887c8d7..b27b9d9415a 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -183,6 +183,7 @@ ct docker + /opt/solr/server/solr/configsets @@ -214,7 +215,7 @@ This container image provides a Dataverse-ready Solr Search Index. - /opt/solr/server/solr/configsets + ${solr.configsets.path} ${project.basedir}/src/assembly/default.xml From 1aa408e88dd81d550266911639d67dc6d4adb72a Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 11 Mar 2022 19:12:34 +0100 Subject: [PATCH 26/56] feat(solr): tune container image memory defaults and request size #7662 --- modules/solr-configset/pom.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index b27b9d9415a..38c0dde1fcc 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -200,6 +200,14 @@ ${ct.registry} solr:${solr.version} + + + + + -XX:MaxRAMPercentage=80.0 -XX:MinHeapFreeRatio=20 -XX:MaxHeapFreeRatio=40 + + -Dsolr.jetty.request.header.size=65535 + ${git.build.time} From b9140f0fcb0cf25d2580b5d662af4fa3f5fbae2f Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 11 Mar 2022 19:14:34 +0100 Subject: [PATCH 27/56] feat(solr): add Solr container run configuration #7662 --- modules/dataverse-parent/pom.xml | 1 + modules/solr-configset/pom.xml | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/modules/dataverse-parent/pom.xml b/modules/dataverse-parent/pom.xml index 054316e1d5f..021f151b00d 100644 --- a/modules/dataverse-parent/pom.xml +++ b/modules/dataverse-parent/pom.xml @@ -198,6 +198,7 @@ 0.39.1 ghcr.io + collection1 dataverse diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 38c0dde1fcc..f9b3caaaf1b 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -184,6 +184,7 @@ docker /opt/solr/server/solr/configsets + 1073741824 @@ -193,6 +194,7 @@ docker-maven-plugin true + true solr @@ -227,8 +229,38 @@ ${project.basedir}/src/assembly/default.xml + + + + solr-precreate ${solr.collection} ${solr.configsets.path}/${solr.configset} + + + ${solr.memory} + + 8983:8983 + + + + solr-temp-volume:/var/solr + + + + + + solr-temp-volume + local + + tmpfs + tmpfs + size=256m,uid=8939 + + + true + + + From c81066ec2c9ce8c1b93682687b7214a3d623416c Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Sat, 12 Mar 2022 02:21:08 +0100 Subject: [PATCH 28/56] fix(solr): align Solr request size with documentation #7662 SystemD unit file and container image needed a change from 65535 to 102400 to be aligned with the docs recommendations. --- .../source/_static/installation/files/etc/systemd/solr.service | 2 +- modules/solr-configset/pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/sphinx-guides/source/_static/installation/files/etc/systemd/solr.service b/doc/sphinx-guides/source/_static/installation/files/etc/systemd/solr.service index 591d2ffb68c..8bb21e38083 100644 --- a/doc/sphinx-guides/source/_static/installation/files/etc/systemd/solr.service +++ b/doc/sphinx-guides/source/_static/installation/files/etc/systemd/solr.service @@ -8,7 +8,7 @@ Type = forking WorkingDirectory = /usr/local/solr/solr-8.11.1 ExecStart = /usr/local/solr/solr-8.11.1/bin/solr start -m 1g -j "jetty.host=127.0.0.1" ExecStop = /usr/local/solr/solr-8.11.1/bin/solr stop -Environment="SOLR_OPTS=-Dsolr.jetty.request.header.size=65535" +Environment="SOLR_OPTS=-Dsolr.jetty.request.header.size=102400" LimitNOFILE=65000 LimitNPROC=65000 Restart=on-failure diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index f9b3caaaf1b..76530e0d1ca 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -208,7 +208,7 @@ -XX:MaxRAMPercentage=80.0 -XX:MinHeapFreeRatio=20 -XX:MaxHeapFreeRatio=40 - -Dsolr.jetty.request.header.size=65535 + -Dsolr.jetty.request.header.size=102400 From 3dba548085f429b4a0e25cdb0c4a2221a92db6d4 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Sat, 12 Mar 2022 02:23:42 +0100 Subject: [PATCH 29/56] docs(solr): refactor Solr installation guide #7662 Aligning with current good practices from Solr, minor corrections about limits and SystemD, usage of the Dataverse configset. --- .../source/installation/prerequisites.rst | 160 +++++++++++++----- 1 file changed, 118 insertions(+), 42 deletions(-) diff --git a/doc/sphinx-guides/source/installation/prerequisites.rst b/doc/sphinx-guides/source/installation/prerequisites.rst index e8650bc335c..8af4b5651cb 100644 --- a/doc/sphinx-guides/source/installation/prerequisites.rst +++ b/doc/sphinx-guides/source/installation/prerequisites.rst @@ -152,91 +152,164 @@ Configuring Database Access for the Dataverse Installation (and the Dataverse So Solr ---- -The Dataverse Software search index is powered by Solr. +The Dataverse Software search index is powered by `Solr `_. Supported Versions ================== -The Dataverse Software has been tested with Solr version |solr_version|. Future releases in the 8.x series are likely to be compatible; however, this cannot be confirmed until they are officially tested. Major releases above 8.x (e.g. 9.x) are not supported. +The Dataverse Software has been tested with Solr version |solr_version|. Future releases in the 8.x series are likely to +be compatible; however, this cannot be confirmed until they are officially tested. Major releases above 8.x (e.g. 9.x) +are not supported. + +- Releases up to 4.20 supported Solr 7.x.x. +- Releases 5.0 to 5.3 supported Solr 7.7.2. +- Releases 5.4 to 5.9 supported Solr 8.8.1. +- Releases since 5.10 support Solr |solr_version| + Installing Solr =============== -You should not run Solr as root. Create a user called ``solr`` and a directory to install Solr into: +Note: this guide describes setting up a small installation, using the Solr *standalone* mode. For larger installations or +higher availability requirements, please take a look at `Solr Cloud `_ mode. + +Optional Step 0: + +- Solr launches asynchronously and attempts to use the ``lsof`` binary to watch for its own availability. + Installation of this package isn't required but will prevent a warning in the log at startup. (Use ``dnf``, ``yum`` or + ``apt-get`` to install this standard package.) +- Solr 8.x runs on Java 11 (same as your Dataverse installation). Remember to install it when running Solr on a + separated machine. + +**Step 1**: You should **not** run Solr as ``root``! Create a user and group called ``solr`` (as root) and a directory to +install Solr into: .. parsed-literal:: - useradd solr mkdir /usr/local/solr - chown solr:solr /usr/local/solr + groupadd -r --gid 8983 solr + useradd -r --home-dir /usr/local/solr --uid 8983 --gid 8983 solr + chown solr: /usr/local/solr -Become the ``solr`` user and then download and configure Solr: +**Step 2:** Become the ``solr`` user and then download and configure Solr: .. parsed-literal:: - su - solr + sudo -u solr -s cd /usr/local/solr wget https://archive.apache.org/dist/lucene/solr/|solr_version|/solr-|solr_version|.tgz tar xvzf solr-|solr_version|.tgz - cd solr-|solr_version| - cp -r server/solr/configsets/_default server/solr/collection1 + exit + -You should already have a "dvinstall.zip" file that you downloaded from https://github.com/IQSS/dataverse/releases . Unzip it into ``/tmp``. Then copy the files into place: +Solr Init Script +================ + +**Step 3:** Once you installed Solr, you need to add to the init system to start on boot, stop on shutdown etc. Please choose the +right option for your underlying Linux operating system. *It will not be necessary to execute both!* + +SystemD based systems +^^^^^^^^^^^^^^^^^^^^^ + +For systems running systemd (like RedHat or derivatives since 7, Debian since 9, Ubuntu since 15.04), as root, download +:download:`solr.service<../_static/installation/files/etc/systemd/solr.service>` and place it in ``/tmp``. Then start +Solr and configure it to start at boot with the following commands (run as root again): .. parsed-literal:: - cp /tmp/dvinstall/schema*.xml /usr/local/solr/solr-|solr_version|/server/solr/collection1/conf - cp /tmp/dvinstall/solrconfig.xml /usr/local/solr/solr-|solr_version|/server/solr/collection1/conf + cp /tmp/solr.service /etc/systemd/system + systemctl daemon-reload + systemctl start solr.service + systemctl enable solr.service -Note: The Dataverse Project team has customized Solr to boost results that come from certain indexed elements inside the Dataverse installation, for example prioritizing results from Dataverse collections over Datasets. If you would like to remove this, edit your ``solrconfig.xml`` and remove the ```` element and its contents. If you have ideas about how this boosting could be improved, feel free to contact us through our Google Group https://groups.google.com/forum/#!forum/dataverse-dev . +SysVinit based systems +^^^^^^^^^^^^^^^^^^^^^^ -A Dataverse installation requires a change to the ``jetty.xml`` file that ships with Solr. Edit ``/usr/local/solr/*/server/etc/jetty.xml`` , increasing ``requestHeaderSize`` from ``8192`` to ``102400`` +For (older) systems using init.d (like CentOS 6 or Devuan), download this :download:`Solr init script <../_static/installation/files/etc/init.d/solr>` +and place it in ``/tmp``. Then start Solr and configure it to start at boot with the following commands (run as root again): -Solr will warn about needing to increase the number of file descriptors and max processes in a production environment but will still run with defaults. We have increased these values to the recommended levels by adding ulimit -n 65000 to the init script, and the following to ``/etc/security/limits.conf``:: +.. parsed-literal:: + cp /tmp/solr /etc/init.d + service start solr + chkconfig solr on - solr soft nproc 65000 - solr hard nproc 65000 - solr soft nofile 65000 - solr hard nofile 65000 -On operating systems which use systemd such as RHEL/derivative, you may then add a line like LimitNOFILE=65000 for the number of open file descriptors and a line with LimitNPROC=65000 for the max processes to the systemd unit file, or adjust the limits on a running process using the prlimit tool:: +Creating Solr Core +================== - # sudo prlimit --pid pid --nofile=65000:65000 +Solr Cores hold the actual data of your index. They get created from templates called "config sets". We provide a +template that has been tuned carefully for usage within a Dataverse installation and is distributed as a ZIP file. -Solr launches asynchronously and attempts to use the ``lsof`` binary to watch for its own availability. Installation of this package isn't required but will prevent a warning in the log at startup:: +Note: The Dataverse Project team has customized the cores ``solrconfig.xml`` to boost Solr search results that come from +certain indexed elements inside the Dataverse installation, for example prioritizing results from Dataverse collections +over Datasets. If you would like to remove this, edit this file and remove the ```` element and its +contents. If you have ideas about how this boosting could be improved, feel free to contact us through our +`Google Group `_. - # yum install lsof +**Step 4:** If not already done, please download the latest release package ``dvinstall.zip`` at +https://github.com/IQSS/dataverse/releases. -Finally, you need to tell Solr to create the core "collection1" on startup: +**Step 5:** Extract our Solr Dataverse config set from it and unpack the configset directory: .. parsed-literal:: + sudo -u solr -s + cd solr-|solr_version|/server/solr/configsets + unzip path/to/dvinstall.zip solr-configset.zip + unzip solr-configset.zip - echo "name=collection1" > /usr/local/solr/solr-|solr_version|/server/solr/collection1/core.properties +**Step 6:** Create the core within your running Solr instance: -Solr Init Script -================ +.. parsed-literal:: + /usr/local/solr/solr-|solr_version|/bin/solr create -c collection1 -d dataverse -Please choose the right option for your underlying Linux operating system. -It will not be necessary to execute both! -For systems running systemd (like RedHat or derivatives since 7, Debian since 9, Ubuntu since 15.04), as root, download :download:`solr.service<../_static/installation/files/etc/systemd/solr.service>` and place it in ``/tmp``. Then start Solr and configure it to start at boot with the following commands:: +Tuning Solr +=========== - cp /tmp/solr.service /etc/systemd/system - systemctl daemon-reload - systemctl start solr.service - systemctl enable solr.service +The next steps are mostly extracted from the recommendations for +`"Taking Solr to Production" `_. -For systems using init.d (like CentOS 6), download this :download:`Solr init script <../_static/installation/files/etc/init.d/solr>` and place it in ``/tmp``. Then start Solr and configure it to start at boot with the following commands:: +They are mostly necessary for older Linux distributions using System V init systems. If you are using our +SystemD unit file (see above), they may be skipped. - cp /tmp/solr /etc/init.d - service start solr - chkconfig solr on +1. A Dataverse installation requires a change to the ``jetty.xml`` file that ships with Solr. + Edit ``/usr/local/solr/*/server/etc/jetty.xml`` , increasing ``requestHeaderSize`` from ``8192`` to ``102400``. + + Alternative: use ``SOLR_OPTS`` to set the system property (see Solr docs linked above). + +2. Solr will warn about needing to increase the number of file descriptors and max processes in a production environment + but will still run with defaults. We have increased these values to the recommended levels by adding ulimit -n 65000 + to the init script, and the following to ``/etc/security/limits.conf``: + + .. parsed-literal:: + solr soft nproc 65000 + solr hard nproc 65000 + solr soft nofile 65000 + solr hard nofile 65000 + + Note: This is not necessary with SystemD, which ignores these settings (see unit file instead)! + If not using our unit file, you may need to add a line like ``LimitNOFILE=65000`` for the number of open file + descriptors and a line with ``LimitNPROC=65000`` for the max processes to the systemd unit file. + + Alternative: adjust the limits on a running process using the ``prlimit`` tool: + + .. parsed-literal:: + sudo prlimit --pid pid --nofile=65000:65000 Securing Solr ============= -Our sample init script and systemd service file linked above tell Solr to only listen on localhost (127.0.0.1). We strongly recommend that you also use a firewall to block access to the Solr port (8983) from outside networks, for added redundancy. +Our sample init script and systemd service file linked above tell Solr to only listen on localhost (127.0.0.1). We +strongly recommend that you also use a firewall to block access to the Solr port (8983) from outside networks, for +added redundancy. -It is **very important** not to allow direct access to the Solr API from outside networks! Otherwise, any host that can reach the Solr port (8983 by default) can add or delete data, search unpublished data, and even reconfigure Solr. For more information, please see https://lucene.apache.org/solr/guide/7_3/securing-solr.html. A particularly serious security issue that has been identified recently allows a potential intruder to remotely execute arbitrary code on the system. See `RCE in Solr via Velocity Template `_ for more information. +It is **very important** not to allow direct access to the Solr API from outside networks! Otherwise, any host that can +reach the Solr port (8983 by default) can add or delete data, search unpublished data, and even reconfigure Solr. For +more information, please see https://lucene.apache.org/solr/guide/7_3/securing-solr.html. A particularly serious +security issue that has been identified recently allows a potential intruder to remotely execute arbitrary code on the +system. See `RCE in Solr via Velocity Template `_ +for more information. -If you're running your Dataverse installation across multiple service hosts you'll want to remove the jetty.host argument (``-j jetty.host=127.0.0.1``) from the startup command line, but make sure Solr is behind a firewall and only accessible by the Dataverse installation host(s), by specific ip address(es). +If you're running your Dataverse installation across multiple service hosts you'll want to remove the jetty.host +argument (``-j jetty.host=127.0.0.1``) from the startup command line, but make sure Solr is behind a firewall and only +accessible by the Dataverse installation host(s), by specific ip address(es). We additionally recommend that the Solr service account's shell be disabled, as it isn't necessary for daily operation:: @@ -250,7 +323,10 @@ or simply prepend each command you would run as the Solr user with "sudo -u solr # sudo -u solr command -Finally, we would like to reiterate that it is simply never a good idea to run Solr as root! Running the process as a non-privileged user would substantially minimize any potential damage even in the event that the instance is compromised. +Finally, we would like to reiterate that it is simply never a good idea to run Solr as root! Running the process as +non-privileged user would substantially minimize any potential damage even in the event that the instance is compromised. + + jq -- From 4b9832666a78a806bd8bb350935b0a2b949690d8 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Mon, 21 Mar 2022 13:10:31 +0100 Subject: [PATCH 30/56] refactor(solr): rename CompileSolrConfigSet to CompileSolrConfigXML #7662 This script is just about the solrconfig.xml and schema.xml will be changed by another script, so rename this one to reflect the scope better. --- modules/solr-configset/pom.xml | 4 ++-- ...ompileSolrConfigSet.java => CompileSolrConfigXML.java} | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) rename modules/solr-configset/src/main/scripts/{CompileSolrConfigSet.java => CompileSolrConfigXML.java} (98%) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 76530e0d1ca..e318ce85d5f 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -69,7 +69,7 @@ 0.0.7 - compile-configset + compile-solrconfig-xml process-resources run @@ -82,7 +82,7 @@ --solr-schemaxml-sourcepath="${project.resources[0].directory}/schema.xml" --solr-config-xsltdir="${project.resources[0].directory}/solrconfig-xslt" - + diff --git a/modules/solr-configset/src/main/scripts/CompileSolrConfigSet.java b/modules/solr-configset/src/main/scripts/CompileSolrConfigXML.java similarity index 98% rename from modules/solr-configset/src/main/scripts/CompileSolrConfigSet.java rename to modules/solr-configset/src/main/scripts/CompileSolrConfigXML.java index b4695f50f53..f148831db73 100644 --- a/modules/solr-configset/src/main/scripts/CompileSolrConfigSet.java +++ b/modules/solr-configset/src/main/scripts/CompileSolrConfigXML.java @@ -29,11 +29,11 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -@Command(name = "CompileSolrConfigSet", +@Command(name = "CompileSolrConfigXML", mixinStandardHelpOptions = true, - version = "CompileSolrConfigSet 0.1", - description = "CompileSolrConfigSet made with jbang") -class CompileSolrConfigSet implements Callable { + version = "CompileSolrConfigXML 0.1", + description = "CompileSolrConfigXML made with jbang") +class CompileSolrConfigXML implements Callable { /** * A wrapper for Throwables to create a checked exception that leads to aborting the execution From afa2b15b2f9761580a84bde64cec085de8df2b4f Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 1 Apr 2022 07:45:53 +0200 Subject: [PATCH 31/56] fix(solr): make container image group name a variable #7662 --- modules/dataverse-parent/pom.xml | 1 + modules/solr-configset/pom.xml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/dataverse-parent/pom.xml b/modules/dataverse-parent/pom.xml index 723b2ea988b..4539301cbc7 100644 --- a/modules/dataverse-parent/pom.xml +++ b/modules/dataverse-parent/pom.xml @@ -198,6 +198,7 @@ 0.39.1 ghcr.io + gdcc collection1 dataverse diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index e318ce85d5f..71702b795cc 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -198,7 +198,7 @@ solr - gdcc/solr:${revision} + ${ct.orgname}/solr:${revision} ${ct.registry} solr:${solr.version} From fb6dcafc6f190ef188edf976f0106853e9da8f58 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 1 Apr 2022 17:40:20 +0200 Subject: [PATCH 32/56] feat(solr): introduce solrteur JBang app #7662 To bundle more Solr related activities, a new top application class is introduced and adds the former CompileSolrConfigXML as subcommand (to be refactored). --- .../cmd/CompileSolrConfig.java} | 12 ++-- .../src/main/scripts/cli/solrteur.java | 59 +++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) rename modules/solr-configset/src/main/scripts/{CompileSolrConfigXML.java => cli/cmd/CompileSolrConfig.java} (97%) create mode 100644 modules/solr-configset/src/main/scripts/cli/solrteur.java diff --git a/modules/solr-configset/src/main/scripts/CompileSolrConfigXML.java b/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java similarity index 97% rename from modules/solr-configset/src/main/scripts/CompileSolrConfigXML.java rename to modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java index f148831db73..56586648091 100644 --- a/modules/solr-configset/src/main/scripts/CompileSolrConfigXML.java +++ b/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java @@ -1,6 +1,6 @@ ///usr/bin/env jbang "$0" "$@" ; exit $? -//DEPS info.picocli:picocli:4.6.3 //DEPS net.sf.saxon:Saxon-HE:10.6 +package cli.cmd; import net.sf.saxon.s9api.Processor; import net.sf.saxon.s9api.SaxonApiException; @@ -29,11 +29,11 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -@Command(name = "CompileSolrConfigXML", +@Command(name = "CompileSolrConfig", mixinStandardHelpOptions = true, - version = "CompileSolrConfigXML 0.1", - description = "CompileSolrConfigXML made with jbang") -class CompileSolrConfigXML implements Callable { + version = "CompileSolrConfig 0.1", + description = "CompileSolrConfig made with jbang") +public class CompileSolrConfig implements Callable { /** * A wrapper for Throwables to create a checked exception that leads to aborting the execution @@ -92,7 +92,7 @@ public static void log(AbortScriptException ex) { private String solrConfigXSLTDir; public static void main(String... args) { - int exitCode = new CommandLine(new CompileSolrConfigSet()).execute(args); + int exitCode = new CommandLine(new CompileSolrConfig()).execute(args); System.exit(exitCode); } diff --git a/modules/solr-configset/src/main/scripts/cli/solrteur.java b/modules/solr-configset/src/main/scripts/cli/solrteur.java new file mode 100644 index 00000000000..2124269ed3f --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/solrteur.java @@ -0,0 +1,59 @@ +///usr/bin/env jbang "$0" "$@" ; exit $? +// +//SOURCES cmd/*.java +// +//DEPS info.picocli:picocli:4.6.3 + +package cli; + +import picocli.CommandLine; +import picocli.CommandLine.Command; + +import cli.cmd.CompileSolrConfig; + +/** + * This class is the main entry point into the different functions of handling different aspects of + * the Dataverse Solr flavor. + * + * (The name "solrteur" is a remix of the German word "Solarteur" which means "solar technician".) + */ +@Command(name = solrteur.CLI_NAME, + mixinStandardHelpOptions = true, + version = solrteur.CLI_NAME+" "+ solrteur.CLI_VERSION, + description = "Execute different tasks around Dataverse and Solr", + subcommands = { + CompileSolrConfig.class + }) +public class solrteur { + public final static String CLI_NAME = "solrteur"; + public final static String CLI_VERSION = "1.0"; + + /** + * A wrapper for Throwables to create a checked exception that leads to aborting the execution + */ + public static final class AbortScriptException extends Exception { + private AbortScriptException() {} + public AbortScriptException(String msg, Throwable cause) { + super(msg, cause); + } + } + /** + * Static inner logging wrapper for convenience. + * This is here because we don't want to add more clutter of a logging framework + * to the Maven output where we use this from. + */ + public static final class Logger { + public static void log(String message) { + System.out.println(message); + } + public static void log(AbortScriptException ex) { + System.out.println(ex.getMessage()); + ex.getCause().printStackTrace(); + } + } + + public static void main(String... args) { + int exitCode = new CommandLine(new solrteur()).execute(args); + System.exit(exitCode); + } +} From 097053338ac86fbd043ea9f11423a46682a30e80 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 1 Apr 2022 17:49:47 +0200 Subject: [PATCH 33/56] refactor(solr): move CompileSolrConfig to pretty sub #7662 A pretty subcommand is used to compile the solr configset. The functionality will be split up to make the subcommand concentrate on the solrconfig.xml handling. --- modules/solr-configset/pom.xml | 11 +- .../scripts/cli/cmd/CompileSolrConfig.java | 114 +++++++----------- 2 files changed, 51 insertions(+), 74 deletions(-) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 71702b795cc..75604d53508 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -76,13 +76,14 @@ + solrconfig --solr-version="${solr.version}" - --solr-zipfile="${project.build.directory}/solr-${solr.version}.zip" - --solr-configset-targetdir="${solr.configset.directory}" - --solr-schemaxml-sourcepath="${project.resources[0].directory}/schema.xml" - --solr-config-xsltdir="${project.resources[0].directory}/solrconfig-xslt" + --zip="${project.build.directory}/solr-${solr.version}.zip" + --configset="${solr.configset.directory}" + --xslts="${project.resources[0].directory}/solrconfig-xslt" + --schemaxml="${project.resources[0].directory}/schema.xml" - + diff --git a/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java b/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java index 56586648091..7c34305854f 100644 --- a/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java +++ b/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java @@ -1,5 +1,5 @@ -///usr/bin/env jbang "$0" "$@" ; exit $? //DEPS net.sf.saxon:Saxon-HE:10.6 + package cli.cmd; import net.sf.saxon.s9api.Processor; @@ -29,89 +29,65 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -@Command(name = "CompileSolrConfig", - mixinStandardHelpOptions = true, - version = "CompileSolrConfig 0.1", - description = "CompileSolrConfig made with jbang") -public class CompileSolrConfig implements Callable { - - /** - * A wrapper for Throwables to create a checked exception that leads to aborting the execution - */ - static final class AbortScriptException extends Throwable { - private AbortScriptException() {} - public AbortScriptException(String msg, Throwable cause) { - super(msg, cause); - } - } - - /** - * Static inner logging wrapper for convenience. - * This is here because we don't want to add more clutter of a logging framework - * to the Maven output where we use this from. - */ - static final class Logger { - public static void log(String message) { - System.out.println(message); - } - public static void log(AbortScriptException ex) { - System.out.println(ex.getMessage()); - ex.getCause().printStackTrace(); - } - } +import cli.solrteur.AbortScriptException; +import cli.solrteur.Logger; + +@Command(name = "solrconfig", + mixinStandardHelpOptions = true, + usageHelpAutoWidth = true, + showDefaultValues = true, + sortOptions = false, + description = "Compile the solrconfig.xml from a source and XSLT files%n") +public +class CompileSolrConfig implements Callable { @Option(required = true, names = {"--solr-version"}, + paramLabel = "x.y.z", description = "The release version of Solr (must match with the ZIP files content)") private String solrVersion; @Option(required = true, - names = {"--solr-zipfile"}, + names = {"--zip"}, + paramLabel = "", description = "Path to local Solr ZIP distribution file") private Path solrZipFile; @Option(required = false, - names = {"--solr-configset-zippath"}, - description = "Path within ZIP to _default configset", + names = {"--zip-subpath"}, + paramLabel = "", + description = "Relative path within ZIP to _default configset", defaultValue = "solr-{{ solr.version }}/server/solr/configsets/_default") private String solrConfigSetZipPath; @Option(required = true, - names = {"--solr-configset-targetdir"}, - description = "Path to the new configset directory") - private String solrConfigSetTargetDir; + names = {"--xslts"}, + paramLabel = "", + description = "Path to the directory with XSLTs to adapt solrconfig.xml") + private String solrConfigXSLTDir; @Option(required = true, - names = {"--solr-schemaxml-sourcepath"}, + names = {"--schemaxml"}, + paramLabel = "", description = "Path to the schema.xml to include") private String solrSchemaXmlSourcePath; @Option(required = true, - names = {"--solr-config-xsltdir"}, - description = "Path to the directory with XSLTs to adapt solrconfig.xml") - private String solrConfigXSLTDir; - - public static void main(String... args) { - int exitCode = new CommandLine(new CompileSolrConfig()).execute(args); - System.exit(exitCode); - } + names = {"--configset"}, + paramLabel = "", + description = "Path to the new configset directory") + private String solrConfigSetTargetDir; /** * Business logic routine, calling all the execution steps. * @return The exit code */ @Override - public Integer call() throws Exception { // TODO: remove Exception signature - try { - replaceVariables(); - extractConfigSet(); - replaceSchemaXML(); - applySolrConfigXSLT(); - } catch (AbortScriptException e) { - Logger.log(e); - // this might be nicely refactored with exit codes stored within the exception - return CommandLine.ExitCode.SOFTWARE; - } + public Integer call() throws Exception { + replaceVariables(); + extractConfigSet(); + replaceSchemaXML(); + applySolrConfigXSLT(); return CommandLine.ExitCode.OK; } @@ -144,7 +120,7 @@ public FileVisitResult preVisitDirectory(Path zippedDir, BasicFileAttributes att return FileVisitResult.CONTINUE; } - + // Copy & replace files already present (which makes any run idempotent by deleting the former run) @Override public FileVisitResult visitFile(Path zippedFile, BasicFileAttributes attrs) throws IOException { @@ -172,7 +148,7 @@ private void replaceSchemaXML() throws AbortScriptException { } catch (IOException e) { throw new AbortScriptException("Could not delete managed-schema", e); } - + // Copy schema.xml in place try { Path sourceSchema = Path.of(solrSchemaXmlSourcePath); @@ -188,14 +164,14 @@ private void applySolrConfigXSLT() throws AbortScriptException { final Path solrConfig = Path.of(solrConfigSetTargetDir, "conf", "solrconfig.xml"); final Path xsltDir = Path.of(solrConfigXSLTDir); - + // Find all the XSLT files final List xsltFiles; try (Stream walk = Files.walk(xsltDir, 2)) { xsltFiles = walk.filter(path -> path.toFile().isFile()) - .filter(path -> path.toString().endsWith("xslt")) - .sorted() - .collect(Collectors.toList()); + .filter(path -> path.toString().endsWith("xslt")) + .sorted() + .collect(Collectors.toList()); } catch (IOException e) { throw new AbortScriptException("Could not walk over XSLT files at " + xsltDir, e); } @@ -211,28 +187,28 @@ private void applySolrConfigXSLT() throws AbortScriptException { final XsltCompiler compiler = processor.newXsltCompiler(); try { - + // First iteration uses initial solrconfig.xml as input source StreamSource source = new StreamSource(Files.newInputStream(solrConfig)); - + // For every XSLT, we need to do the transformation and rotate the input source afterwards, // so we apply the next transformation to the already transformed content. for (Path xsltFile : xsltFiles) { final XsltExecutable stylesheet = compiler.compile(new StreamSource(Files.newInputStream(xsltFile))); - + // Prepare a fresh temporary file (which will be deleted when we read it back for the next iteration) final Path tmpFile = Files.createTempFile(null, null); Serializer out = processor.newSerializer(Files.newOutputStream(tmpFile)); - + // Actual transformation happens Xslt30Transformer transformer = stylesheet.load30(); transformer.transform(source, out); - + // Read back the transformed config and rotate the source. The opening option makes the old temp file // go away after it has been read. source = new StreamSource(Files.newInputStream(tmpFile, StandardOpenOption.DELETE_ON_CLOSE)); } - + // The final transformation still reads back the final result, so we need to push InputStream content somewhere Files.copy(source.getInputStream(), solrConfig, From 6ce1b5df4f1ecb29e22737491d503ed66415a1fa Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 26 Apr 2022 17:49:14 +0200 Subject: [PATCH 34/56] refactor(solrteur): introduce simple log levels #7662 --- .../main/scripts/cli/cmd/CompileSolrConfig.java | 8 ++++---- .../src/main/scripts/cli/solrteur.java | 15 +++++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java b/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java index 7c34305854f..50b7b1e153e 100644 --- a/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java +++ b/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java @@ -160,9 +160,9 @@ private void replaceSchemaXML() throws AbortScriptException { } private void applySolrConfigXSLT() throws AbortScriptException { - Logger.log("Starting to transform solrconfig.xml..."); + Logger.info("Starting to transform solrconfig.xml..."); - final Path solrConfig = Path.of(solrConfigSetTargetDir, "conf", "solrconfig.xml"); + final Path solrConfig = cliParent.getTargetDir().resolve(Path.of("conf", "solrconfig.xml")); final Path xsltDir = Path.of(solrConfigXSLTDir); // Find all the XSLT files @@ -177,9 +177,9 @@ private void applySolrConfigXSLT() throws AbortScriptException { } // Log found XSLT files - Logger.log("Found XSLT files in " + solrConfigXSLTDir + ":"); + Logger.info("Found XSLT files in " + solrConfigXSLTDir + ":"); for (Path xsltFile : xsltFiles) { - Logger.log(xsltFile.toString().substring(xsltDir.toString().length()+1)); + Logger.info(xsltFile.toString().substring(xsltDir.toString().length()+1)); } // Setup the XSLT processor diff --git a/modules/solr-configset/src/main/scripts/cli/solrteur.java b/modules/solr-configset/src/main/scripts/cli/solrteur.java index 2124269ed3f..12082678dca 100644 --- a/modules/solr-configset/src/main/scripts/cli/solrteur.java +++ b/modules/solr-configset/src/main/scripts/cli/solrteur.java @@ -43,13 +43,24 @@ public AbortScriptException(String msg, Throwable cause) { * to the Maven output where we use this from. */ public static final class Logger { - public static void log(String message) { + static void log(String message) { System.out.println(message); } - public static void log(AbortScriptException ex) { + static void log(AbortScriptException ex) { System.out.println(ex.getMessage()); ex.getCause().printStackTrace(); } + + public static void info(String message) { + if (!quiet) { + log(message); + } + } + public static void info(AbortScriptException ex) { + if (!quiet) { + log(ex); + } + } } public static void main(String... args) { From c0516cc67deb0ec48a16f4ee9002d053a4ef2e6b Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 26 Apr 2022 17:52:04 +0200 Subject: [PATCH 35/56] refactor(solrteur): split up solr config compilation #7662 - Only do the solrconfig.xml handling within the command - Move common options to parent command (solrteur as top level) - Move configset extraction from the Solr ZIP into distinct command to make it independently usable --- .../scripts/cli/cmd/CompileSolrConfig.java | 110 +----------------- .../scripts/cli/cmd/ExtractConfigSet.java | 108 +++++++++++++++++ .../src/main/scripts/cli/solrteur.java | 36 +++++- 3 files changed, 146 insertions(+), 108 deletions(-) create mode 100644 modules/solr-configset/src/main/scripts/cli/cmd/ExtractConfigSet.java diff --git a/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java b/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java index 50b7b1e153e..d4949a2752d 100644 --- a/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java +++ b/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java @@ -10,20 +10,15 @@ import net.sf.saxon.s9api.XsltExecutable; import picocli.CommandLine; import picocli.CommandLine.Command; +import picocli.CommandLine.ParentCommand; import picocli.CommandLine.Option; import javax.xml.transform.stream.StreamSource; import java.io.IOException; -import java.nio.file.FileAlreadyExistsException; -import java.nio.file.FileSystem; -import java.nio.file.FileSystems; -import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.SimpleFileVisitor; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; -import java.nio.file.attribute.BasicFileAttributes; import java.util.List; import java.util.concurrent.Callable; import java.util.stream.Collectors; @@ -41,24 +36,8 @@ public class CompileSolrConfig implements Callable { - @Option(required = true, - names = {"--solr-version"}, - paramLabel = "x.y.z", - description = "The release version of Solr (must match with the ZIP files content)") - private String solrVersion; - - @Option(required = true, - names = {"--zip"}, - paramLabel = "", - description = "Path to local Solr ZIP distribution file") - private Path solrZipFile; - - @Option(required = false, - names = {"--zip-subpath"}, - paramLabel = "", - description = "Relative path within ZIP to _default configset", - defaultValue = "solr-{{ solr.version }}/server/solr/configsets/_default") - private String solrConfigSetZipPath; + @ParentCommand + private cli.solrteur cliParent; @Option(required = true, names = {"--xslts"}, @@ -66,99 +45,16 @@ class CompileSolrConfig implements Callable { description = "Path to the directory with XSLTs to adapt solrconfig.xml") private String solrConfigXSLTDir; - @Option(required = true, - names = {"--schemaxml"}, - paramLabel = "", - description = "Path to the schema.xml to include") - private String solrSchemaXmlSourcePath; - - @Option(required = true, - names = {"--configset"}, - paramLabel = "", - description = "Path to the new configset directory") - private String solrConfigSetTargetDir; - /** * Business logic routine, calling all the execution steps. * @return The exit code */ @Override public Integer call() throws Exception { - replaceVariables(); - extractConfigSet(); - replaceSchemaXML(); applySolrConfigXSLT(); return CommandLine.ExitCode.OK; } - private void replaceVariables() throws AbortScriptException { - this.solrConfigSetZipPath = this.solrConfigSetZipPath.replaceAll("\\Q{{ solr.version }}\\E", solrVersion); - } - - private void extractConfigSet() throws AbortScriptException { - // Wrap the file system in a try-with-resources statement - // to auto-close it when finished and prevent a memory leak - try (FileSystem zipFileSystem = FileSystems.newFileSystem(solrZipFile, null)) { - Path zipSource = zipFileSystem.getPath(solrConfigSetZipPath); - - // TODO: should we delete the target before copying the new content? (Usually this shouldn't change, but better safe than sorry?) - - Logger.log("Starting to extract Solr _default config set..."); - Files.walkFileTree(zipSource, new SimpleFileVisitor<>() { - // Copy the directory structure (skip existing with the same name) - @Override - public FileVisitResult preVisitDirectory(Path zippedDir, BasicFileAttributes attrs) throws IOException { - // Remove the leading path part from the ZIP file structure, as we don't want it in target - String strippedZipPath = zippedDir.toString().substring(solrConfigSetZipPath.length()); - Path targetDir = Path.of(solrConfigSetTargetDir, strippedZipPath); - - try { - Files.copy(zippedDir, targetDir, StandardCopyOption.COPY_ATTRIBUTES); - } catch (FileAlreadyExistsException e) { - // intentional ignore - simply reuse the existing directory - } - - return FileVisitResult.CONTINUE; - } - - // Copy & replace files already present (which makes any run idempotent by deleting the former run) - @Override - public FileVisitResult visitFile(Path zippedFile, BasicFileAttributes attrs) throws IOException { - String strippedZipPath = zippedFile.toString().substring(solrConfigSetZipPath.length()); - Path targetFile = Path.of(solrConfigSetTargetDir, strippedZipPath); - - Files.copy(zippedFile, targetFile, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); - - return FileVisitResult.CONTINUE; - } - }); - - } catch(IOException e) { - throw new AbortScriptException("Extracting from ZIP file "+solrZipFile+" failed", e); - } - } - - private void replaceSchemaXML() throws AbortScriptException { - Logger.log("Starting to replace the Solr schema..."); - - // Delete "managed-schema" file - try { - Path managedSchema = Path.of(solrConfigSetTargetDir, "conf", "managed-schema"); - Files.deleteIfExists(managedSchema); - } catch (IOException e) { - throw new AbortScriptException("Could not delete managed-schema", e); - } - - // Copy schema.xml in place - try { - Path sourceSchema = Path.of(solrSchemaXmlSourcePath); - Path targetSchema = Path.of(solrConfigSetTargetDir, "conf", "schema.xml"); - Files.copy(sourceSchema, targetSchema, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); - } catch (IOException e) { - throw new AbortScriptException("Could not copy schema.xml", e); - } - } - private void applySolrConfigXSLT() throws AbortScriptException { Logger.info("Starting to transform solrconfig.xml..."); diff --git a/modules/solr-configset/src/main/scripts/cli/cmd/ExtractConfigSet.java b/modules/solr-configset/src/main/scripts/cli/cmd/ExtractConfigSet.java new file mode 100644 index 00000000000..47272c71313 --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/cmd/ExtractConfigSet.java @@ -0,0 +1,108 @@ +package cli.cmd; + +import cli.solrteur; +import picocli.CommandLine; +import picocli.CommandLine.Command; +import picocli.CommandLine.ParentCommand; +import picocli.CommandLine.Option; + +import java.io.IOException; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.concurrent.Callable; + +import cli.solrteur.AbortScriptException; +import cli.solrteur.Logger; + +@Command( + name = "extract-zip", + mixinStandardHelpOptions = true, + usageHelpAutoWidth = true, + showDefaultValues = true, + sortOptions = false, + description = "Extract default configset from Solr ZIP distribution%n") +public class ExtractConfigSet implements Callable { + + @ParentCommand + private solrteur cliParent; + + @Option(required = true, + names = {"--zip"}, + paramLabel = "", + description = "Path to local Solr ZIP distribution file") + private Path solrZipFile; + + @Option(required = false, + names = {"--zip-subpath"}, + paramLabel = "", + description = "Relative path within ZIP to _default configset", + defaultValue = "solr-{{ solr.version }}/server/solr/configsets/_default") + private String solrConfigSetZipPath; + + /** + * Business logic routine, calling all the execution steps. + * @return The exit code + */ + @Override + public Integer call() throws Exception { + replaceVariables(); + extractConfigSet(); + return CommandLine.ExitCode.OK; + } + + private void replaceVariables() throws AbortScriptException { + this.solrConfigSetZipPath = this.solrConfigSetZipPath.replaceAll("\\Q{{ solr.version }}\\E", cliParent.getSolrVersion()); + } + + private void extractConfigSet() throws AbortScriptException { + // Wrap the file system in a try-with-resources statement + // to auto-close it when finished and prevent a memory leak + try (FileSystem zipFileSystem = FileSystems.newFileSystem(solrZipFile, null)) { + Path zipSource = zipFileSystem.getPath(solrConfigSetZipPath); + + // TODO: should we delete the target before copying the new content? (Usually this shouldn't change, but better safe than sorry?) + + Logger.info("Extracting " + solrConfigSetZipPath + " from " + solrZipFile + " into " + cliParent.getTargetDir()); + Files.walkFileTree(zipSource, new SimpleFileVisitor<>() { + // Copy the directory structure (skip existing with the same name) + @Override + public FileVisitResult preVisitDirectory(Path zippedDir, BasicFileAttributes attrs) throws IOException { + // Remove the leading path part from the ZIP file structure, as we don't want it in target + String strippedZipPath = zippedDir.toString().substring(solrConfigSetZipPath.length()); + Path targetDir = Path.of(cliParent.getTargetDir().toString(), strippedZipPath); + + try { + Logger.info(solrZipFile + ":" + zippedDir + " -> " + targetDir); + Files.copy(zippedDir, targetDir, StandardCopyOption.COPY_ATTRIBUTES); + } catch (FileAlreadyExistsException e) { + // intentional ignore - simply reuse the existing directory + } + + return FileVisitResult.CONTINUE; + } + + // Copy & replace files already present (which makes any run idempotent by deleting the former run) + @Override + public FileVisitResult visitFile(Path zippedFile, BasicFileAttributes attrs) throws IOException { + String strippedZipPath = zippedFile.toString().substring(solrConfigSetZipPath.length()); + Path targetFile = Path.of(cliParent.getTargetDir().toString(), strippedZipPath); + + Logger.info(solrZipFile + ":" + zippedFile + " -> " + targetFile); + Files.copy(zippedFile, targetFile, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); + + return FileVisitResult.CONTINUE; + } + }); + + } catch(IOException e) { + throw new AbortScriptException("Extracting from ZIP file "+solrZipFile+" failed", e); + } + } +} diff --git a/modules/solr-configset/src/main/scripts/cli/solrteur.java b/modules/solr-configset/src/main/scripts/cli/solrteur.java index 12082678dca..c42f78e858b 100644 --- a/modules/solr-configset/src/main/scripts/cli/solrteur.java +++ b/modules/solr-configset/src/main/scripts/cli/solrteur.java @@ -6,11 +6,15 @@ package cli; +import cli.cmd.ExtractConfigSet; import picocli.CommandLine; import picocli.CommandLine.Command; +import picocli.CommandLine.Option; import cli.cmd.CompileSolrConfig; +import java.nio.file.Path; + /** * This class is the main entry point into the different functions of handling different aspects of * the Dataverse Solr flavor. @@ -19,15 +23,45 @@ */ @Command(name = solrteur.CLI_NAME, mixinStandardHelpOptions = true, + usageHelpAutoWidth = true, version = solrteur.CLI_NAME+" "+ solrteur.CLI_VERSION, description = "Execute different tasks around Dataverse and Solr", subcommands = { + ExtractConfigSet.class, CompileSolrConfig.class - }) + }, + synopsisSubcommandLabel = "COMMAND") public class solrteur { public final static String CLI_NAME = "solrteur"; public final static String CLI_VERSION = "1.0"; + @Option( + required = true, + names = {"--solr-version", "-s"}, + paramLabel = "", + description = "Which version of Solr to use, e. g. 8.9 or 8.11.1") + private String solrVersion; + + public String getSolrVersion() { + return this.solrVersion; + } + + @Option(required = true, + names = {"--target", "-t"}, + paramLabel = "", + description = "Path to a target directory") + private Path targetDir; + + public Path getTargetDir() { + return this.targetDir; + } + + @Option( + names = {"--quiet", "-q"}, + description = "Decrease verbosity" + ) + static boolean quiet; + /** * A wrapper for Throwables to create a checked exception that leads to aborting the execution */ From efb948be4ad1bf4faa6ce8d7aafff6a78ebdaf8c Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 26 Apr 2022 17:52:38 +0200 Subject: [PATCH 36/56] chore(solrteur): add package-info.java for cli.cmd package #7662 --- .../solr-configset/src/main/scripts/cli/cmd/package-info.java | 1 + 1 file changed, 1 insertion(+) create mode 100644 modules/solr-configset/src/main/scripts/cli/cmd/package-info.java diff --git a/modules/solr-configset/src/main/scripts/cli/cmd/package-info.java b/modules/solr-configset/src/main/scripts/cli/cmd/package-info.java new file mode 100644 index 00000000000..0e5bbb76c50 --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/cmd/package-info.java @@ -0,0 +1 @@ +package cli.cmd; \ No newline at end of file From a1b25c58cf3d468064060562b3950f777b0746eb Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 26 Apr 2022 17:54:47 +0200 Subject: [PATCH 37/56] chore(solrteur): introduce helper packages cli.util and cli.util.model #7662 The handle the future TSV file parsing and analysis, we need to add more classes handling the model of a TSV metadata block file(s). This will do the heavy lifting to transform TSV files into POJOs, reusable for the Solr Schema Configuration. --- .../src/main/scripts/cli/util/model/package-info.java | 1 + .../solr-configset/src/main/scripts/cli/util/package-info.java | 1 + 2 files changed, 2 insertions(+) create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/package-info.java create mode 100644 modules/solr-configset/src/main/scripts/cli/util/package-info.java diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/package-info.java b/modules/solr-configset/src/main/scripts/cli/util/model/package-info.java new file mode 100644 index 00000000000..6fd97a0fe49 --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/package-info.java @@ -0,0 +1 @@ +package cli.util.model; \ No newline at end of file diff --git a/modules/solr-configset/src/main/scripts/cli/util/package-info.java b/modules/solr-configset/src/main/scripts/cli/util/package-info.java new file mode 100644 index 00000000000..633e3028144 --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/package-info.java @@ -0,0 +1 @@ +package cli.util; \ No newline at end of file From 39acda29db7740834a5b8f5bfe358b21cd87bd49 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 26 Apr 2022 18:08:40 +0200 Subject: [PATCH 38/56] feat(solrteur): introduce custom exception to handle parsing errors #7662 Instead of relying on Java provided exceptions, we want to track line numbers and other more details of the parsing process, so we need custom mechanics. --- .../scripts/cli/util/model/ParserError.java | 16 ++++++++++++ .../cli/util/model/ParserException.java | 25 +++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/ParserError.java create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/ParserException.java diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ParserError.java b/modules/solr-configset/src/main/scripts/cli/util/model/ParserError.java new file mode 100644 index 00000000000..d3ec70fc3dd --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/ParserError.java @@ -0,0 +1,16 @@ +package cli.util.model; + +public final class ParserError { + String message; + Integer lineNumber; + + public ParserError(String message, Integer lineNumber) { + this.message = message; + this.lineNumber = lineNumber; + } + + public ParserError(ParserException exception, Integer lineNumber) { + this.message = exception.getMessage(); + this.lineNumber = lineNumber; + } +} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ParserException.java b/modules/solr-configset/src/main/scripts/cli/util/model/ParserException.java new file mode 100644 index 00000000000..ec1390cc66e --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/ParserException.java @@ -0,0 +1,25 @@ +package cli.util.model; + +import java.util.ArrayList; +import java.util.List; + +public final class ParserException extends Throwable { + + final List subExceptions = new ArrayList<>(0); + + public ParserException(String message) { + super(message); + } + + public boolean hasSubExceptions() { + return subExceptions.size() > 0; + } + + public void addSubException(String message) { + this.subExceptions.add(new ParserException(message)); + } + + public List getSubExceptions() { + return List.copyOf(subExceptions); + } +} From a5efc7ec4716bf1f8e373504a2ed947e52800039 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 26 Apr 2022 18:10:40 +0200 Subject: [PATCH 39/56] feat(solrteur): introduce state machine for TSV file layout #7662 Our custom metadata block TSV files follow a certain order of things. We also do not allow for repetitions or similar. All of this can be most easily be depicted with a state maschine, so we know where to send a line to for parsing. This commit also adds the very basic (empty) POJOs to store the block, fields and vocabularies in to enable testing the state transition. It also adds constants we rely on, like what's the trigger char, the comment intro and the field delimiter --- .../main/scripts/cli/util/model/Block.java | 12 ++++ .../scripts/cli/util/model/Constants.java | 7 ++ .../cli/util/model/ControlledVocabulary.java | 5 ++ .../main/scripts/cli/util/model/Field.java | 12 ++++ .../scripts/cli/util/model/ParsingState.java | 45 ++++++++++++ .../java/cli/util/model/ParsingStateTest.java | 70 +++++++++++++++++++ 6 files changed, 151 insertions(+) create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/Block.java create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/Constants.java create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/Field.java create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java create mode 100644 modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java new file mode 100644 index 00000000000..3c9f8f331f7 --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java @@ -0,0 +1,12 @@ +package cli.util.model; + +import java.util.List; +import java.util.Optional; + +public final class Block { + public static final String TRIGGER = Constants.TRIGGER_INDICATOR + "metadataBlock"; + + private Block() {} + + Optional> fields = Optional.empty(); +} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Constants.java b/modules/solr-configset/src/main/scripts/cli/util/model/Constants.java new file mode 100644 index 00000000000..1fd8c6cdb67 --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Constants.java @@ -0,0 +1,7 @@ +package cli.util.model; + +public class Constants { + public static final String COMMENT_INDICATOR = "%%"; + public static final String TRIGGER_INDICATOR = "#"; + public static final String COLUMN_SEPARATOR = "\t"; +} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java b/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java new file mode 100644 index 00000000000..7804c617945 --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java @@ -0,0 +1,5 @@ +package cli.util.model; + +public class ControlledVocabulary { + public static final String TRIGGER = Constants.TRIGGER_INDICATOR + "controlledVocabulary"; +} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java new file mode 100644 index 00000000000..f4e7eed04d2 --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java @@ -0,0 +1,12 @@ +package cli.util.model; + +import java.util.List; +import java.util.Optional; + +public class Field { + public static final String TRIGGER = Constants.TRIGGER_INDICATOR + "datasetField"; + + private Field() {} + + Optional> controlledVocabularyValues = Optional.empty(); +} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java b/modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java new file mode 100644 index 00000000000..fc4e11c141f --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java @@ -0,0 +1,45 @@ +package cli.util.model; + +import cli.util.TsvBlockReader; + +public enum ParsingState { + Vocabularies(ControlledVocabulary.TRIGGER), + Fields(Field.TRIGGER, Vocabularies), + MetadataBlock(Block.TRIGGER, Fields), + // This state is only used exactly once and should never be reached from input. + // For safety, make the validation fail. + Init(Constants.COMMENT_INDICATOR, MetadataBlock); + + private final String stateTrigger; + private final ParsingState nextState; + + ParsingState(String trigger, ParsingState next) { + this.stateTrigger = trigger; + this.nextState = next; + } + + /** + * Create final state (no next step) + * @param trigger + */ + ParsingState(String trigger) { + this.stateTrigger = trigger; + this.nextState = this; + } + + public boolean isAllowedFinalState() { + return this == Fields || this == Vocabularies; + } + + public ParsingState transitionState(String headerLine) throws ParserException { + // if not null, not starting the same state again (no loops allowed) and starting the correct next state, return the next state + if(headerLine != null && ! headerLine.startsWith(this.stateTrigger) && + headerLine.startsWith(this.nextState.stateTrigger)) { + return this.nextState; + } + // otherwise throw a parsing exception + throw new ParserException("Invalid header '" + + (headerLine == null ? "null" : headerLine.substring(0, Math.min(25, headerLine.length()))) + + "...' while in section '" + this.stateTrigger + "'"); + } +} diff --git a/modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java b/modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java new file mode 100644 index 00000000000..b498639c852 --- /dev/null +++ b/modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java @@ -0,0 +1,70 @@ +package cli.util.model; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ParsingStateTest { + + static Stream failingStateTransitionExamples() { + return Stream.of( + Arguments.of(ParsingState.Init, null), + Arguments.of(ParsingState.MetadataBlock, null), + Arguments.of(ParsingState.Fields, null), + Arguments.of(ParsingState.Vocabularies, null), + + Arguments.of(ParsingState.Init, ""), + Arguments.of(ParsingState.MetadataBlock, ""), + Arguments.of(ParsingState.Fields, ""), + Arguments.of(ParsingState.Vocabularies, ""), + + Arguments.of(ParsingState.Init, "foobar"), + Arguments.of(ParsingState.MetadataBlock, "foobar"), + Arguments.of(ParsingState.Fields, "foobar"), + Arguments.of(ParsingState.Vocabularies, "foobar"), + + Arguments.of(ParsingState.Init, Constants.TRIGGER_INDICATOR), + Arguments.of(ParsingState.Init, Constants.COMMENT_INDICATOR), + Arguments.of(ParsingState.Init, Constants.COLUMN_SEPARATOR), + + Arguments.of(ParsingState.Init, Field.TRIGGER), + Arguments.of(ParsingState.Init, ControlledVocabulary.TRIGGER), + + Arguments.of(ParsingState.MetadataBlock, Constants.COMMENT_INDICATOR), + Arguments.of(ParsingState.MetadataBlock, ControlledVocabulary.TRIGGER), + + Arguments.of(ParsingState.Fields, Constants.COMMENT_INDICATOR), + Arguments.of(ParsingState.Fields, Block.TRIGGER), + + Arguments.of(ParsingState.Vocabularies, Constants.COMMENT_INDICATOR), + Arguments.of(ParsingState.Vocabularies, Block.TRIGGER), + Arguments.of(ParsingState.Vocabularies, Field.TRIGGER) + ); + } + + @ParameterizedTest + @MethodSource("failingStateTransitionExamples") + void failingTransitions(ParsingState source, String triggerLine) throws ParserException { + ParserException ex = assertThrows(ParserException.class, () -> source.transitionState(triggerLine)); + } + + static Stream successfulStateTransitionExamples() { + return Stream.of( + Arguments.of(ParsingState.Init, Block.TRIGGER, ParsingState.MetadataBlock), + Arguments.of(ParsingState.MetadataBlock, Field.TRIGGER, ParsingState.Fields), + Arguments.of(ParsingState.Fields, ControlledVocabulary.TRIGGER, ParsingState.Vocabularies) + ); + } + + @ParameterizedTest + @MethodSource("successfulStateTransitionExamples") + void successfulTransitions(ParsingState source, String triggerLine, ParsingState expected) throws ParserException { + assertEquals(expected, source.transitionState(triggerLine)); + } + +} \ No newline at end of file From 7fc08b0611c6f2f77d939844dab1a0b9b75c4616 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 26 Apr 2022 19:09:31 +0200 Subject: [PATCH 40/56] feat(solrteur): introduce a validator helper class #7662 The TSV parser needs to verify if a certain line is a header line and matching the spec. To avoid duplicated validation code, this validator can be used with an arbitrary list of strings (so it can be reused for blocks, fields and vocabularies). As we will need to validate URLs in certain fields, this validator also offers a helper function to create predicates checking for valid URLs. --- .../scripts/cli/util/model/Validator.java | 112 ++++++++++++++++++ .../java/cli/util/model/ValidatorTest.java | 96 +++++++++++++++ 2 files changed, 208 insertions(+) create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/Validator.java create mode 100644 modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Validator.java b/modules/solr-configset/src/main/scripts/cli/util/model/Validator.java new file mode 100644 index 00000000000..8a6211797cf --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Validator.java @@ -0,0 +1,112 @@ +package cli.util.model; + +import java.net.MalformedURLException; +import java.net.URISyntaxException; +import java.net.URL; +import java.util.ArrayList; +import java.util.List; + +public class Validator { + + /** + * Test if a given value is a valid {@link java.net.URL} + * + * Remember, Java only supports HTTP/S, file and JAR protocols by default! + * Any URL not using such a protocol will not be considered a valid URL! + * {@see URL Constructor Summary} + * + * @param url The value to test + * @return True if valid URL, false otherwise + */ + public static boolean isValidUrl(String url) { + try { + new URL(url).toURI(); + return true; + } catch (MalformedURLException | URISyntaxException e) { + return false; + } + } + + /** + * Split and validate a textual line declared to be a header of custom metadata block definition section + * (the block, the fields or controlled vocabularies). Will return a list of the headers found (if they match) and + * when being a spec conform header line. + * + * As this function retrieves the relevant spec parts as parameters, it can be reused for all sections. + * You will need to transform into the resulting list into real Header enum values within calling code. + * + * This validator is strict with naming and order of appearance (must be same as spec), but is lenient + * about case (so you might use camel/pascal case variants). + * + * @param headerLine The textual line to analyse. + * @param startsWith A String which needs to be present at the start of the headerLine. + * @param validOrderedHeaders A list of Strings with the column headers from the spec in order of appearance. + * @return A list of the found headers in normalized form if matching the spec + * @throws ParserException If any validation fails. Contains sub-exceptions with validation details. + */ + static List validateHeaderLine(final String headerLine, + final String startsWith, + final List validOrderedHeaders) throws ParserException { + // start a parenting parser exception to be filled with errors as subexceptions + ParserException ex = new ParserException("contains an invalid column header"); + + if (headerLine == null || headerLine.isBlank()) { + ex.addSubException("Header may not be null, empty or whitespace only"); + throw ex; + } + + // the actual split and validate length + String[] headerSplit = headerLine.split(Constants.COLUMN_SEPARATOR); + // missing headers? + if (headerSplit.length < validOrderedHeaders.size()) { + ex.addSubException( + "Less fields (" + headerSplit.length + ") found than required (" + validOrderedHeaders.size() + ")."); + } else if (headerSplit.length > validOrderedHeaders.size()) { + ex.addSubException( + "More fields (" + headerSplit.length + ") found than required (" + validOrderedHeaders.size() + ")."); + } + + // allocate a list of validated columns + List validatedColumns = new ArrayList<>(); + + // iterate the found header values + for (int i = 0; i < headerSplit.length; i++) { + String columnHeader = headerSplit[i]; + + // is the value a valid one? (in order of appearance and existing, but ignoring case) + if (i < validOrderedHeaders.size() && validOrderedHeaders.get(i).equalsIgnoreCase(columnHeader)) { + // add as entry of validated and present headers (to be used for line mapping) + // BUT use the normalized variant (makes comparisons easier) + validatedColumns.add(validOrderedHeaders.get(i)); + // when invalid, mark as such + } else { + ex.addSubException( + "Column " + (i+1) + " contains '" + columnHeader + "', but spec expects " + + (i < validOrderedHeaders.size() ? "'"+validOrderedHeaders.get(i)+"'" : "nothing") + " to be here." + ); + // additional hint when valid, but accidentally already present + if (validatedColumns.stream().anyMatch(columnHeader::equalsIgnoreCase)) { + ex.addSubException("Column " + (i+1) + " contains valid '" + columnHeader + "' already present."); + } + } + } + + // when there are headers missing, report them + if ( validatedColumns.size() < validOrderedHeaders.size() ) { + for (int i = 0; i < validOrderedHeaders.size(); i++) { + String missingHeader = validOrderedHeaders.get(i); + if (validatedColumns.stream().noneMatch(missingHeader::equalsIgnoreCase)) { + ex.addSubException("Missing column '" + missingHeader + "' from position " + (i+1) + "."); + } + } + } + + // Will only return the header column mapping if and only if the validation did not find errors. + // use an unmodifiable version of the list to avoid accidents without notice. Else throw the exception. + if (ex.hasSubExceptions()) { + throw ex; + } else { + return List.copyOf(validatedColumns); + } + } +} diff --git a/modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java b/modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java new file mode 100644 index 00000000000..71e2333e4d8 --- /dev/null +++ b/modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java @@ -0,0 +1,96 @@ +package cli.util.model; + +import cli.util.TsvBlockReader; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class ValidatorTest { + + private static final Logger logger = Logger.getLogger(ValidatorTest.class.getCanonicalName()); + + @Nested + class UtilsTest { + @ParameterizedTest + @CsvSource(nullValues = "NULL", + value = { + "NULL,NULL", + "hello,hello", + "' hello',' hello'", + "' hello ',' hello '", + "' hello',' hello\t\t\t'", + "'\t\t\thello','\t\t\thello\t\t\t'", + "'\t\t\thello\ttest','\t\t\thello\ttest\t\t'", + "'\t\t\thello\ttest\t\t ','\t\t\thello\ttest\t\t '", + }) + void trimming(String expected, String sut) { + assertEquals(expected, TsvBlockReader.rtrimColumns(sut)); + } + + @ParameterizedTest + @CsvSource(nullValues = "NULL", + value = { + "false,NULL", + "false,''", + "false,hello", + "false,https://", + "false,www.foo.bar", + "false,://foo.bar.com", + "true,https://wwww.foobar.com", + "true,https://wwww.foobar.com/hello", + "true,https://wwww.foobar.com:1214/hello", + "true,https://host/hello", + }) + void urlValidation(boolean expected, String sut) { + assertEquals(expected, Validator.isValidUrl(sut)); + } + } + + @Nested + class ValidateBlockHeader { + List blockHeaders = Block.Header.getHeaders(); + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = { + "hello", + "#metadataBlock test", + "#metadataBlock\tname\tdataverseAlias\tdisplayName", + "\t#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI", + "#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI\tfoobar", + "#metadataBlock\tname\tdataverseAlias\tdisplayName\tdisplayName\tblockURI", + "dataverseAlias\tdisplayName\tblockURI\t#metadataBlock\tname" + }) + void validateHeaderLine_Block_Throws(String line) { + ParserException exception = assertThrows(ParserException.class, () -> Validator.validateHeaderLine(line, Block.TRIGGER, blockHeaders)); + assertTrue(exception.hasSubExceptions()); + logger.log(Level.FINE, + exception.getSubExceptions().stream().map(Throwable::getMessage).collect(Collectors.joining("\n")) + ); + } + + @ParameterizedTest + @ValueSource(strings = { + "#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI", + "#metadataBlock\tNAME\tDataversealias\tDisplayname\tBlockURI" + }) + void validateHeaderLine_Block_True(String line) throws ParserException { + List headers = Validator.validateHeaderLine(line, Block.TRIGGER, blockHeaders); + assertFalse(headers.isEmpty()); + // we expect the normalized form, so the arrays should match! + assertEquals(blockHeaders, headers); + } + } +} \ No newline at end of file From ff0e91c85773dd1a549d82e1473684f405328557 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 26 Apr 2022 19:14:50 +0200 Subject: [PATCH 41/56] feat(solrteur): extend the metadata block POJO #7662 The Block POJO now contains the header specification (uses the Validator class to perform the validation) and allows to parse a line into a List. A later relaxation of the spec allowing for reordering of fields, etc is possible, while the calling code of the parser can reuse the found header definition. A builder pattern is used to parse and validate the actual definition. As the block may only be used once the definition, all fields and vocabularies have been parsed (if the is an error within the TSV the parsing has to fail!), the builder pattern is a natural match to that. --- .../main/scripts/cli/util/model/Block.java | 238 ++++++++++++++++++ .../test/java/cli/util/model/BlockTest.java | 51 ++++ 2 files changed, 289 insertions(+) create mode 100644 modules/solr-configset/src/test/java/cli/util/model/BlockTest.java diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java index 3c9f8f331f7..01134d1b8b0 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java @@ -1,11 +1,249 @@ package cli.util.model; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.EnumMap; import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Predicate; +import java.util.regex.Pattern; +import java.util.stream.Collectors; public final class Block { public static final String TRIGGER = Constants.TRIGGER_INDICATOR + "metadataBlock"; + public static final String NAME_PATTERN = "[a-z]+(([\\d_])|([A-Z0-9][a-z0-9]+))*([A-Z])?"; + + /** + * Programmatic variant of the spec of a #metadataBlock. List all the column headers and associated restrictions + * on the values of a column. + */ + public enum Header { + TRIGGER( + Block.TRIGGER, + String::isEmpty, + "must have no value (be empty)" + ), + NAME( + "name", + Predicate.not(String::isBlank).and(Pattern.compile(Block.NAME_PATTERN).asMatchPredicate()), + "must not be blank and match regex pattern " + Block.NAME_PATTERN + ), + DATAVERSE_ALIAS( + "dataverseAlias", + Predicate.not(String::isBlank).or(String::isEmpty), + "must be either empty or not blank" + ), + DISPLAY_NAME( + "displayName", + Predicate.not(String::isBlank).and(h -> h.length() < 257), + "must not be blank and shorter than 256 chars" + ), + BLOCK_URI( + "blockURI", + Validator::isValidUrl, + "must be a valid URL" + ); + + private final String value; + private final Predicate test; + private final String errorMessage; + + Header(final String value, final Predicate test, final String errorMessage) { + this.value = value; + this.test = test; + this.errorMessage = errorMessage; + } + + private static final Map valueMap; + static { + Map map = new ConcurrentHashMap<>(); + Arrays.stream(Header.values()).forEach(h -> map.put(h.toString(), h)); + valueMap = Collections.unmodifiableMap(map); + } + + /** + * Inverse lookup of a {@link Header} from a {@link String}. + * + * @param value A textual string to look up. + * @return Matching {@link Header} wrapped in {@link Optional} or an empty {@link Optional}. + */ + public static Optional
getByValue(String value) { + return Optional.ofNullable(valueMap.get(value)); + } + + /** + * Retrieve all column headers of a metadata block definition as a spec-like list of strings, + * usable for validation and more. The list is ordered as the spec defines the order of the headers. + * + * @return List of the column headers, in order + */ + public static List getHeaders() { + return Arrays.stream(Header.values()).map(Header::toString).collect(Collectors.toUnmodifiableList()); + } + + /** + * Parse a {@link String} as a header of a metadata block definition. Will validate the presence or absence + * of column headers as defined by the spec. This is a forgiving parser - the order of columns may be different + * from what the spec defaults to (spec isn't precise about strict ordering). + * + * @param line The textual line to parse for column headers + * @return A list of {@link Header} that reflects the order as given in {@see line} (order may differ from spec). + * @throws ParserException when presented column headers are missing, invalid or the complete line is just wrong. + */ + public static List
parseAndValidate(final String line) throws ParserException { + List validatedColumns = Validator.validateHeaderLine(line, Block.TRIGGER, getHeaders()); + // the IllegalStateException shall never happen, as we already validated the header! + return validatedColumns.stream() + .map(Header::getByValue) + .map(o -> o.orElseThrow(IllegalStateException::new)) + .collect(Collectors.toUnmodifiableList()); + } + + @Override + public String toString() { + return value; + } + + /** + * Test a given {@link String} if it matches the restrictions applied on values of this type. + * + * @param sut The textual string to test. + * @return True if matching or false in every other case. + */ + public boolean isValid(final String sut) { + return test.test(sut); + } + + /** + * Receive a proper error message for this type of value (should be extended with more context in calling code!). + * + * @return The error message, always in the form of "must ...". (Create a sentence with it.) + */ + public String getErrorMessage() { + return errorMessage; + } + } + + /** + * Blocks should not be build directly, but using this builder pattern. This allows for validation before handing + * over an object to work with and containing a complete POJO representation of a (custom) metadata block. + */ + public static final class BlockBuilder { + private final List
header; + private final Map settings = new EnumMap(Header.class); + private final List fieldBuilders = new ArrayList<>(); + private boolean hasErrors = false; + private boolean hasParsedALine = false; + + private BlockBuilder() { + this.header = new ArrayList<>(); + } + + /** + * Create a builder with a line containing the header of the metadata block definition, so the order of + * the columns can be determined from it. (The builder is stateful.) + * + * @param header The textual line with the column headers. + * @throws ParserException + */ + public BlockBuilder(final String header) throws ParserException { + this.header = Block.Header.parseAndValidate(header); + } + + /** + * Analyse a line containing a concrete metadata block definition by parsing and validating it. + * + * This will fail: + * - when the line is null or blanks only + * - when another line has been analysed before (spec allows only 1 definition in a single custom metadata block) + * - when the columns within the line do not match the length of the header + * - when the column values do not match the column type restrictions (as implied by the header) + * + * The exception might contain sub-exceptions, as the parser will do its best to keep going and find as many + * problems as possible to avoid unnecessary (pesky) re-iterations. + * + * @param line The metadata block definition line to analyse. + * @throws ParserException If the parsing fails (see description). + */ + public void parseAndValidateLine(final String line) throws ParserException { + // no null or blank lines for the parser. (blank lines can be skipped and not sent here by calling code) + if (line == null || line.isBlank()) { + this.hasErrors = true; + throw new ParserException("must not be empty nor blanks only nor null."); + } + + // only 1 block definition allowed as per spec + if (this.hasParsedALine) { + this.hasErrors = true; + throw new ParserException("must not try to add another metadata block definition"); + } else { + // need to set this here, even BEFORE actual parsing. If the line cannot be parsed, + // the exception is catched and another line is sent this way, we need to make sure + // we do not accept it! + this.hasParsedALine = true; + } + + String[] lineParts = line.split(Constants.COLUMN_SEPARATOR); + validateColumns(lineParts); + } + + /** + * Validate the columns (usually given by {@code parseAndValidateLine}). + * + * @param lineParts + * @throws ParserException + */ + void validateColumns(final String[] lineParts) throws ParserException { + if (lineParts == null || lineParts.length != header.size()) { + throw new ParserException("does not match length of metadata block headline"); + } + + ParserException parserException = new ParserException("has validation errors"); + for (int i = 0; i < lineParts.length; i++) { + Header column = header.get(i); + String value = lineParts[i]; + if( ! column.isValid(value)) { + parserException.addSubException( + "Invalid value '" + value + " for column '" + column + "', " + column.getErrorMessage()); + } else { + this.settings.put(column, value); + } + } + + if (parserException.hasSubExceptions()) { + // setting this to true to ensure no block will be created accidentally via build(). + this.hasErrors = true; + throw parserException; + } + } + + public boolean hasFailed() { + return this.hasErrors || ! this.hasParsedALine; + } + + public boolean hasSucceeded() { + return ! this.hasErrors && this.hasParsedALine; + } + + /** + * Execute the builder to create the {@link Block} POJO, containing the representation of the custom metadata + * block that has been analysed. Will execute associated field builders (which will execute associated + * vocabulary builders). + */ + public Block build() { + if (hasSucceeded()) { + // TODO: extend with adding the necessary bits about block properties, contained fields etc. + return new Block(); + } else { + throw new IllegalStateException("Trying to build a block with errors or without parsing a line first"); + } + } + } + private Block() {} Optional> fields = Optional.empty(); diff --git a/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java b/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java new file mode 100644 index 00000000000..512909b4f7f --- /dev/null +++ b/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java @@ -0,0 +1,51 @@ +package cli.util.model; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class BlockTest { + + private static final Logger logger = Logger.getLogger(BlockTest.class.getCanonicalName()); + + @ParameterizedTest + @ValueSource(strings = { + "#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI", + "#metadataBlock\tNAME\tDataversealias\tDisplayname\tBlockURI" + }) + void successfulParseAndValidateHeaderLine(String headerLine) throws ParserException { + List headers = Block.Header.parseAndValidate(headerLine); + assertFalse(headers.isEmpty()); + assertEquals(List.of(Block.Header.values()), headers); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = { + "hello", + "#metadataBlock test", + "#metadataBlock\tname\tdataverseAlias\tdisplayName", + "\t#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI", + "#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI\tfoobar", + "#metadataBlock\tname\tdataverseAlias\tdisplayName\tdisplayName\tblockURI", + "dataverseAlias\tdisplayName\tblockURI\t#metadataBlock\tname" + }) + void failingParseAndValidateHeaderLine(String headerLine) throws ParserException { + ParserException exception = assertThrows(ParserException.class, () -> Block.Header.parseAndValidate(headerLine)); + assertTrue(exception.hasSubExceptions()); + logger.log(Level.INFO, + exception.getSubExceptions().stream().map(Throwable::getMessage).collect(Collectors.joining("\n")) + ); + } + +} \ No newline at end of file From 2dae76c7e3696f9e2532bca128648d385f85f722 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 07:23:29 +0200 Subject: [PATCH 42/56] feat(solrteur): introduce configuration POJO #7662 This simple class will allow to make the parser somewhat configurable, so future changes and command line options can be integrated more easily. --- .../scripts/cli/util/model/Configuration.java | 59 +++++++++++++++++++ .../cli/util/model/ConfigurationTest.java | 27 +++++++++ 2 files changed, 86 insertions(+) create mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/Configuration.java create mode 100644 modules/solr-configset/src/test/java/cli/util/model/ConfigurationTest.java diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Configuration.java b/modules/solr-configset/src/main/scripts/cli/util/model/Configuration.java new file mode 100644 index 00000000000..e57671586fb --- /dev/null +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Configuration.java @@ -0,0 +1,59 @@ +package cli.util.model; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public final class Configuration { + + private final String comment; + private final String trigger; + private final String column; + private final Matcher rtrimMatcher; + + public Configuration( + String comment, + String trigger, + String column + ) { + notNullNotEmpty("Comment indicator", comment); + this.comment = comment; + + notNullNotEmpty("Triggering indicator (keyword prefix)", trigger); + this.trigger = trigger; + + notNullNotEmpty("Column separator", column); + this.column = column; + + this.rtrimMatcher = Pattern.compile("(" + this.column + ")+$").matcher(""); + } + + public static Configuration defaultConfig() { + return new Configuration("%%", "#", "\t"); + } + + private static void notNullNotEmpty(String optionName, String value) { + if (value == null || value.isEmpty()) { + throw new IllegalArgumentException(optionName + " may not be null or empty"); + } + } + + public String commentIndicator() { + return comment; + } + + public String triggerIndicator() { + return trigger; + } + + public String columnSeparator() { + return column; + } + + public String rtrimColumns(String line) { + return line == null ? null : rtrimMatcher.reset(line).replaceAll(""); + } + + public String trigger(String keyword) { + return this.triggerIndicator() + keyword; + } +} diff --git a/modules/solr-configset/src/test/java/cli/util/model/ConfigurationTest.java b/modules/solr-configset/src/test/java/cli/util/model/ConfigurationTest.java new file mode 100644 index 00000000000..322f8affa02 --- /dev/null +++ b/modules/solr-configset/src/test/java/cli/util/model/ConfigurationTest.java @@ -0,0 +1,27 @@ +package cli.util.model; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import static org.junit.jupiter.api.Assertions.*; + +class ConfigurationTest { + + private final Configuration testConfiguration = Configuration.defaultConfig(); + + @ParameterizedTest + @CsvSource(nullValues = "NULL", + value = { + "NULL,NULL", + "hello,hello", + "' hello',' hello'", + "' hello ',' hello '", + "' hello',' hello\t\t\t'", + "'\t\t\thello','\t\t\thello\t\t\t'", + "'\t\t\thello\ttest','\t\t\thello\ttest\t\t'", + "'\t\t\thello\ttest\t\t ','\t\t\thello\ttest\t\t '", + }) + void trimming(String expected, String sut) { + assertEquals(expected, testConfiguration.rtrimColumns(sut)); + } +} \ No newline at end of file From 2649a87baacef8463a1b2e1d02b9ef09f2f6a99f Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 09:41:48 +0200 Subject: [PATCH 43/56] feat(solrteur): extend Field model with minimal header spec #7662 --- .../main/scripts/cli/util/model/Field.java | 109 +++++++++++++++++- 1 file changed, 107 insertions(+), 2 deletions(-) diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java index f4e7eed04d2..a8308867c8c 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java @@ -1,10 +1,115 @@ package cli.util.model; +import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Predicate; +import java.util.stream.Collectors; -public class Field { - public static final String TRIGGER = Constants.TRIGGER_INDICATOR + "datasetField"; +public final class Field { + + public static final String KEYWORD = "datasetField"; + + /** + * Programmatic variant of the spec of a #datasetField. List all the column headers and associated restrictions + * on the values of a column. + */ + public enum Header { + // TODO: extend! + KEYWORD( + Field.KEYWORD, + String::isEmpty, + "must have no value (be empty)" + ); + + private final String value; + private final Predicate test; + private final String errorMessage; + + Header(final String value, final Predicate test, final String errorMessage) { + this.value = value; + this.test = test; + this.errorMessage = errorMessage; + } + + Header(final String value) { + this.value = value; + this.test = Predicate.not(String::isBlank); + this.errorMessage = "must not be empty or blank"; + } + + private static final Map valueMap; + static { + Map map = new ConcurrentHashMap<>(); + Arrays.stream(Header.values()).forEach(h -> map.put(h.toString(), h)); + valueMap = Collections.unmodifiableMap(map); + } + + /** + * Inverse lookup of a {@link Field.Header} from a {@link String}. + * + * @param value A textual string to look up. + * @return Matching {@link Field.Header} wrapped in {@link Optional} or an empty {@link Optional}. + */ + public static Optional
getByValue(String value) { + return Optional.ofNullable(valueMap.get(value)); + } + + /** + * Retrieve all column headers of field definitions as a spec-like list of strings, + * usable for validation and more. The list is ordered as the spec defines the order of the headers. + * + * @return List of the column headers, in order + */ + public static List getHeaders() { + return List.copyOf(valueMap.keySet()); + } + + /** + * Parse a {@link String} as a header of field definitions. Will validate the presence or absence + * of column headers as defined by the spec. This is not a lenient parser - headers need to comply with order + * from the spec. On the other hand, it is case-insensitive. + * + * @param line The textual line to parse for column headers + * @param config The parser configuration to be used + * @return A list of {@link Field.Header} build from the line of text + * @throws ParserException When presented column headers are missing, invalid or the complete line is just wrong. + * @throws IllegalStateException When a column header cannot be found within the enum {@link Field.Header}. + * This should never happen, as the validation would fail before! + */ + public static List
parseAndValidate(final String line, final Configuration config) throws ParserException { + List validatedColumns = Validator.validateHeaderLine(line, getHeaders(), config); + // the exception shall never happen, as we already validated the header! + return validatedColumns.stream() + .map(Header::getByValue) + .map(o -> o.orElseThrow(IllegalStateException::new)) + .collect(Collectors.toUnmodifiableList()); + } + + @Override + public String toString() { + return value; + } + + public boolean isValid(final String sut) { + return test.test(sut); + } + + public String getErrorMessage() { + return errorMessage; + } + } + + + public static final class FieldBuilder { + // TODO: extend! + public Field build() { + return new Field(); + } + } private Field() {} From 2aa8a4e6dd68df83b098013cf85fc2ada0757f19 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 09:42:13 +0200 Subject: [PATCH 44/56] feat(solrteur): extend ControlledVocabulary model with minimal header spec #7662 --- .../cli/util/model/ControlledVocabulary.java | 113 +++++++++++++++++- 1 file changed, 111 insertions(+), 2 deletions(-) diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java b/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java index 7804c617945..26bc8f3caef 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java @@ -1,5 +1,114 @@ package cli.util.model; -public class ControlledVocabulary { - public static final String TRIGGER = Constants.TRIGGER_INDICATOR + "controlledVocabulary"; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +public final class ControlledVocabulary { + + public static final String KEYWORD = "controlledVocabulary"; + + /** + * Programmatic variant of the spec of a #controlledVocabulary. List all the column headers and associated restrictions + * on the values of a column. + */ + public enum Header { + // TODO: add the rest of the fields + KEYWORD( + ControlledVocabulary.KEYWORD, + String::isEmpty, + "must have no value (be empty)" + ); + + private final String value; + private final Predicate test; + private final String errorMessage; + + Header(final String value, final Predicate test, final String errorMessage) { + this.value = value; + this.test = test; + this.errorMessage = errorMessage; + } + + Header(final String value) { + this.value = value; + this.test = Predicate.not(String::isBlank); + this.errorMessage = "must not be empty or blank"; + } + + private static final Map valueMap; + static { + Map map = new ConcurrentHashMap<>(); + Arrays.stream(Header.values()).forEach(h -> map.put(h.toString(), h)); + valueMap = Collections.unmodifiableMap(map); + } + + /** + * Inverse lookup of a {@link ControlledVocabulary.Header} from a {@link String}. + * + * @param value A textual string to look up. + * @return Matching {@link ControlledVocabulary.Header} wrapped in {@link Optional} or an empty {@link Optional}. + */ + public static Optional getByValue(String value) { + return Optional.ofNullable(valueMap.get(value)); + } + + /** + * Retrieve all column headers of controlled vocabulary definitions as a spec-like list of strings, + * usable for validation and more. The list is ordered as the spec defines the order of the headers. + * + * @return List of the column headers, in order + */ + public static List getHeaders() { + return List.copyOf(valueMap.keySet()); + } + + /** + * Parse a {@link String} as a header of a metadata block definition. Will validate the presence or absence + * of column headers as defined by the spec. This is not a lenient parser - headers need to comply with order + * from the spec. On the other hand, it is case-insensitive. + * + * @param line The textual line to parse for column headers + * @param config The parser configuration to be used + * @return A list of {@link ControlledVocabulary.Header} build from the line of text + * @throws ParserException When presented column headers are missing, invalid or the complete line is just wrong. + * @throws IllegalStateException When a column header cannot be found within the enum {@link ControlledVocabulary.Header}. + * This should never happen, as the validation would fail before! + */ + public static List parseAndValidate(final String line, final Configuration config) throws ParserException { + List validatedColumns = Validator.validateHeaderLine(line, getHeaders(), config); + // the exception shall never happen, as we already validated the header! + return validatedColumns.stream() + .map(ControlledVocabulary.Header::getByValue) + .map(o -> o.orElseThrow(IllegalStateException::new)) + .collect(Collectors.toUnmodifiableList()); + } + + @Override + public String toString() { + return value; + } + + public boolean isValid(final String sut) { + return test.test(sut); + } + + public String getErrorMessage() { + return errorMessage; + } + } + + public static final class ControlledVocabularyBuilder { + // TODO: extend! + public ControlledVocabulary build() { + return new ControlledVocabulary(); + } + } + + private ControlledVocabulary() {} } From 3733117c34dbcb8ba956d5a580ee83c9ba4fb1e0 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 09:43:08 +0200 Subject: [PATCH 45/56] refactor(solrteur): switch Block model to use keyword #7662 Instead of defining a static trigger, we want to be able to configure the trigger sign. Due to this, we use the keyword only and move the trigger handling into the ParsingState (which is analysing the line for state transition anyway). --- .../main/scripts/cli/util/model/Block.java | 34 +++++++++++-------- .../test/java/cli/util/model/BlockTest.java | 4 +-- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java index 01134d1b8b0..bddf5f9b329 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java @@ -13,8 +13,7 @@ import java.util.stream.Collectors; public final class Block { - public static final String TRIGGER = Constants.TRIGGER_INDICATOR + "metadataBlock"; - + public static final String KEYWORD = "metadataBlock"; public static final String NAME_PATTERN = "[a-z]+(([\\d_])|([A-Z0-9][a-z0-9]+))*([A-Z])?"; /** @@ -22,8 +21,8 @@ public final class Block { * on the values of a column. */ public enum Header { - TRIGGER( - Block.TRIGGER, + KEYWORD( + Block.KEYWORD, String::isEmpty, "must have no value (be empty)" ), @@ -87,15 +86,18 @@ public static List getHeaders() { /** * Parse a {@link String} as a header of a metadata block definition. Will validate the presence or absence - * of column headers as defined by the spec. This is a forgiving parser - the order of columns may be different - * from what the spec defaults to (spec isn't precise about strict ordering). + * of column headers as defined by the spec. This is not a lenient parser - headers need to comply with order + * from the spec. On the other hand, it is case-insensitive. * * @param line The textual line to parse for column headers - * @return A list of {@link Header} that reflects the order as given in {@see line} (order may differ from spec). - * @throws ParserException when presented column headers are missing, invalid or the complete line is just wrong. + * @param config The parser configuration to be used + * @return A list of {@link Header} build from the line of text + * @throws ParserException When presented column headers are missing, invalid or the complete line is just wrong. + * @throws IllegalStateException When a column header cannot be found within the enum {@link Header}. + * This should never happen, as the validation would fail before! */ - public static List
parseAndValidate(final String line) throws ParserException { - List validatedColumns = Validator.validateHeaderLine(line, Block.TRIGGER, getHeaders()); + public static List
parseAndValidate(final String line, final Configuration config) throws ParserException { + List validatedColumns = Validator.validateHeaderLine(line, getHeaders(), config); // the IllegalStateException shall never happen, as we already validated the header! return validatedColumns.stream() .map(Header::getByValue) @@ -133,13 +135,15 @@ public String getErrorMessage() { * over an object to work with and containing a complete POJO representation of a (custom) metadata block. */ public static final class BlockBuilder { + private final Configuration config; private final List
header; - private final Map settings = new EnumMap(Header.class); + private final Map settings = new EnumMap<>(Header.class); private final List fieldBuilders = new ArrayList<>(); private boolean hasErrors = false; private boolean hasParsedALine = false; private BlockBuilder() { + this.config = Configuration.defaultConfig(); this.header = new ArrayList<>(); } @@ -150,8 +154,9 @@ private BlockBuilder() { * @param header The textual line with the column headers. * @throws ParserException */ - public BlockBuilder(final String header) throws ParserException { - this.header = Block.Header.parseAndValidate(header); + public BlockBuilder(final String header, final Configuration config) throws ParserException { + this.config = config; + this.header = Block.Header.parseAndValidate(header, config); } /** @@ -187,8 +192,7 @@ public void parseAndValidateLine(final String line) throws ParserException { this.hasParsedALine = true; } - String[] lineParts = line.split(Constants.COLUMN_SEPARATOR); - validateColumns(lineParts); + validateColumns(line.split(config.columnSeparator())); } /** diff --git a/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java b/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java index 512909b4f7f..5fb8dcd01d7 100644 --- a/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java +++ b/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java @@ -24,7 +24,7 @@ class BlockTest { "#metadataBlock\tNAME\tDataversealias\tDisplayname\tBlockURI" }) void successfulParseAndValidateHeaderLine(String headerLine) throws ParserException { - List headers = Block.Header.parseAndValidate(headerLine); + List headers = Block.Header.parseAndValidate(headerLine, Configuration.defaultConfig()); assertFalse(headers.isEmpty()); assertEquals(List.of(Block.Header.values()), headers); } @@ -41,7 +41,7 @@ void successfulParseAndValidateHeaderLine(String headerLine) throws ParserExcept "dataverseAlias\tdisplayName\tblockURI\t#metadataBlock\tname" }) void failingParseAndValidateHeaderLine(String headerLine) throws ParserException { - ParserException exception = assertThrows(ParserException.class, () -> Block.Header.parseAndValidate(headerLine)); + ParserException exception = assertThrows(ParserException.class, () -> Block.Header.parseAndValidate(headerLine, Configuration.defaultConfig())); assertTrue(exception.hasSubExceptions()); logger.log(Level.INFO, exception.getSubExceptions().stream().map(Throwable::getMessage).collect(Collectors.joining("\n")) From 3c6a5fa6ac63c4f4fd58bab75c95ce54d5ada6b0 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 11:16:46 +0200 Subject: [PATCH 46/56] refactor(solrteur): refactor state factory with state keywords #7662 --- .../scripts/cli/util/model/Constants.java | 7 ---- .../scripts/cli/util/model/ParsingState.java | 34 ++++++++-------- .../java/cli/util/model/ParsingStateTest.java | 40 ++++++++++--------- 3 files changed, 38 insertions(+), 43 deletions(-) delete mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/Constants.java diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Constants.java b/modules/solr-configset/src/main/scripts/cli/util/model/Constants.java deleted file mode 100644 index 1fd8c6cdb67..00000000000 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Constants.java +++ /dev/null @@ -1,7 +0,0 @@ -package cli.util.model; - -public class Constants { - public static final String COMMENT_INDICATOR = "%%"; - public static final String TRIGGER_INDICATOR = "#"; - public static final String COLUMN_SEPARATOR = "\t"; -} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java b/modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java index fc4e11c141f..a89c4b31da7 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java @@ -1,20 +1,18 @@ package cli.util.model; -import cli.util.TsvBlockReader; - public enum ParsingState { - Vocabularies(ControlledVocabulary.TRIGGER), - Fields(Field.TRIGGER, Vocabularies), - MetadataBlock(Block.TRIGGER, Fields), + Vocabularies(ControlledVocabulary.KEYWORD), + Fields(Field.KEYWORD, Vocabularies), + MetadataBlock(Block.KEYWORD, Fields), // This state is only used exactly once and should never be reached from input. // For safety, make the validation fail. - Init(Constants.COMMENT_INDICATOR, MetadataBlock); + Init(null, MetadataBlock); - private final String stateTrigger; + private final String stateKeyword; private final ParsingState nextState; - ParsingState(String trigger, ParsingState next) { - this.stateTrigger = trigger; + ParsingState(String keyword, ParsingState next) { + this.stateKeyword = keyword; this.nextState = next; } @@ -22,8 +20,8 @@ public enum ParsingState { * Create final state (no next step) * @param trigger */ - ParsingState(String trigger) { - this.stateTrigger = trigger; + ParsingState(String keyword) { + this.stateKeyword = keyword; this.nextState = this; } @@ -31,15 +29,17 @@ public boolean isAllowedFinalState() { return this == Fields || this == Vocabularies; } - public ParsingState transitionState(String headerLine) throws ParserException { + public ParsingState transitionState(String headerLine, Configuration config) throws ParserException { // if not null, not starting the same state again (no loops allowed) and starting the correct next state, return the next state - if(headerLine != null && ! headerLine.startsWith(this.stateTrigger) && - headerLine.startsWith(this.nextState.stateTrigger)) { + if(headerLine != null && + ! headerLine.startsWith(config.trigger(this.stateKeyword)) && + headerLine.startsWith(config.trigger(this.nextState.stateKeyword))) { return this.nextState; } - // otherwise throw a parsing exception - throw new ParserException("Invalid header '" + + // otherwise, throw a parsing exception + throw new ParserException("Found invalid header '" + (headerLine == null ? "null" : headerLine.substring(0, Math.min(25, headerLine.length()))) + - "...' while in section '" + this.stateTrigger + "'"); + "...' while " + + (this.stateKeyword == null ? "initializing." : "in section '" + this.stateKeyword + "'.")); } } diff --git a/modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java b/modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java index b498639c852..a50b927c16b 100644 --- a/modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java +++ b/modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java @@ -10,6 +10,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; class ParsingStateTest { + + static Configuration config = Configuration.defaultConfig(); static Stream failingStateTransitionExamples() { return Stream.of( @@ -28,43 +30,43 @@ static Stream failingStateTransitionExamples() { Arguments.of(ParsingState.Fields, "foobar"), Arguments.of(ParsingState.Vocabularies, "foobar"), - Arguments.of(ParsingState.Init, Constants.TRIGGER_INDICATOR), - Arguments.of(ParsingState.Init, Constants.COMMENT_INDICATOR), - Arguments.of(ParsingState.Init, Constants.COLUMN_SEPARATOR), + Arguments.of(ParsingState.Init, config.triggerIndicator()), + Arguments.of(ParsingState.Init, config.commentIndicator()), + Arguments.of(ParsingState.Init, config.columnSeparator()), - Arguments.of(ParsingState.Init, Field.TRIGGER), - Arguments.of(ParsingState.Init, ControlledVocabulary.TRIGGER), + Arguments.of(ParsingState.Init, config.trigger(Field.KEYWORD)), + Arguments.of(ParsingState.Init, config.trigger(ControlledVocabulary.KEYWORD)), - Arguments.of(ParsingState.MetadataBlock, Constants.COMMENT_INDICATOR), - Arguments.of(ParsingState.MetadataBlock, ControlledVocabulary.TRIGGER), + Arguments.of(ParsingState.MetadataBlock, config.commentIndicator()), + Arguments.of(ParsingState.MetadataBlock, config.trigger(ControlledVocabulary.KEYWORD)), - Arguments.of(ParsingState.Fields, Constants.COMMENT_INDICATOR), - Arguments.of(ParsingState.Fields, Block.TRIGGER), + Arguments.of(ParsingState.Fields, config.commentIndicator()), + Arguments.of(ParsingState.Fields, config.trigger(Block.KEYWORD)), - Arguments.of(ParsingState.Vocabularies, Constants.COMMENT_INDICATOR), - Arguments.of(ParsingState.Vocabularies, Block.TRIGGER), - Arguments.of(ParsingState.Vocabularies, Field.TRIGGER) + Arguments.of(ParsingState.Vocabularies, config.commentIndicator()), + Arguments.of(ParsingState.Vocabularies, config.trigger(Block.KEYWORD)), + Arguments.of(ParsingState.Vocabularies, config.trigger(Field.KEYWORD)) ); } @ParameterizedTest @MethodSource("failingStateTransitionExamples") - void failingTransitions(ParsingState source, String triggerLine) throws ParserException { - ParserException ex = assertThrows(ParserException.class, () -> source.transitionState(triggerLine)); + void failingTransitions(final ParsingState source, final String triggerLine) throws ParserException { + ParserException ex = assertThrows(ParserException.class, () -> source.transitionState(triggerLine, config)); } static Stream successfulStateTransitionExamples() { return Stream.of( - Arguments.of(ParsingState.Init, Block.TRIGGER, ParsingState.MetadataBlock), - Arguments.of(ParsingState.MetadataBlock, Field.TRIGGER, ParsingState.Fields), - Arguments.of(ParsingState.Fields, ControlledVocabulary.TRIGGER, ParsingState.Vocabularies) + Arguments.of(ParsingState.Init, config.trigger(Block.KEYWORD), ParsingState.MetadataBlock), + Arguments.of(ParsingState.MetadataBlock, config.trigger(Field.KEYWORD), ParsingState.Fields), + Arguments.of(ParsingState.Fields, config.trigger(ControlledVocabulary.KEYWORD), ParsingState.Vocabularies) ); } @ParameterizedTest @MethodSource("successfulStateTransitionExamples") - void successfulTransitions(ParsingState source, String triggerLine, ParsingState expected) throws ParserException { - assertEquals(expected, source.transitionState(triggerLine)); + void successfulTransitions(final ParsingState source, final String triggerLine, final ParsingState expected) throws ParserException { + assertEquals(expected, source.transitionState(triggerLine, config)); } } \ No newline at end of file From aacb890fa56876702deb068b55174f06fb336b9a Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 11:18:44 +0200 Subject: [PATCH 47/56] refactor(solrteur): let Validator use the configuration #7662 --- .../scripts/cli/util/model/Validator.java | 18 +++++++++++----- .../java/cli/util/model/ValidatorTest.java | 21 ++----------------- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Validator.java b/modules/solr-configset/src/main/scripts/cli/util/model/Validator.java index 8a6211797cf..d419ebcda95 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Validator.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Validator.java @@ -6,7 +6,7 @@ import java.util.ArrayList; import java.util.List; -public class Validator { +public final class Validator { /** * Test if a given value is a valid {@link java.net.URL} @@ -39,14 +39,14 @@ public static boolean isValidUrl(String url) { * about case (so you might use camel/pascal case variants). * * @param headerLine The textual line to analyse. - * @param startsWith A String which needs to be present at the start of the headerLine. * @param validOrderedHeaders A list of Strings with the column headers from the spec in order of appearance. + * @param config The configuration to use * @return A list of the found headers in normalized form if matching the spec * @throws ParserException If any validation fails. Contains sub-exceptions with validation details. */ static List validateHeaderLine(final String headerLine, - final String startsWith, - final List validOrderedHeaders) throws ParserException { + final List validOrderedHeaders, + final Configuration config) throws ParserException { // start a parenting parser exception to be filled with errors as subexceptions ParserException ex = new ParserException("contains an invalid column header"); @@ -55,8 +55,16 @@ static List validateHeaderLine(final String headerLine, throw ex; } + // test for trigger being present + if (! headerLine.startsWith(config.triggerIndicator())) { + ex.addSubException("Trigger sign '" + config.triggerIndicator() + "' not found"); + throw ex; + } + // the actual split and validate length - String[] headerSplit = headerLine.split(Constants.COLUMN_SEPARATOR); + String[] headerSplit = headerLine + .substring(config.triggerIndicator().length()) // remove the trigger indicator first + .split(config.columnSeparator()); // missing headers? if (headerSplit.length < validOrderedHeaders.size()) { ex.addSubException( diff --git a/modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java b/modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java index 71e2333e4d8..990db6e7ec1 100644 --- a/modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java +++ b/modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java @@ -1,6 +1,5 @@ package cli.util.model; -import cli.util.TsvBlockReader; import org.junit.jupiter.api.Nested; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; @@ -23,22 +22,6 @@ class ValidatorTest { @Nested class UtilsTest { - @ParameterizedTest - @CsvSource(nullValues = "NULL", - value = { - "NULL,NULL", - "hello,hello", - "' hello',' hello'", - "' hello ',' hello '", - "' hello',' hello\t\t\t'", - "'\t\t\thello','\t\t\thello\t\t\t'", - "'\t\t\thello\ttest','\t\t\thello\ttest\t\t'", - "'\t\t\thello\ttest\t\t ','\t\t\thello\ttest\t\t '", - }) - void trimming(String expected, String sut) { - assertEquals(expected, TsvBlockReader.rtrimColumns(sut)); - } - @ParameterizedTest @CsvSource(nullValues = "NULL", value = { @@ -74,7 +57,7 @@ class ValidateBlockHeader { "dataverseAlias\tdisplayName\tblockURI\t#metadataBlock\tname" }) void validateHeaderLine_Block_Throws(String line) { - ParserException exception = assertThrows(ParserException.class, () -> Validator.validateHeaderLine(line, Block.TRIGGER, blockHeaders)); + ParserException exception = assertThrows(ParserException.class, () -> Validator.validateHeaderLine(line, blockHeaders, Configuration.defaultConfig())); assertTrue(exception.hasSubExceptions()); logger.log(Level.FINE, exception.getSubExceptions().stream().map(Throwable::getMessage).collect(Collectors.joining("\n")) @@ -87,7 +70,7 @@ void validateHeaderLine_Block_Throws(String line) { "#metadataBlock\tNAME\tDataversealias\tDisplayname\tBlockURI" }) void validateHeaderLine_Block_True(String line) throws ParserException { - List headers = Validator.validateHeaderLine(line, Block.TRIGGER, blockHeaders); + List headers = Validator.validateHeaderLine(line, blockHeaders, Configuration.defaultConfig()); assertFalse(headers.isEmpty()); // we expect the normalized form, so the arrays should match! assertEquals(blockHeaders, headers); From fea4d1cc5b83012cfa96acbaca703ad888a83172 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 18:36:13 +0200 Subject: [PATCH 48/56] feat(solrteur): extend Block and make it testable #7662 - Implement first details of the Block POJO - Change parsing with BlockBuilder to use an internal state with a not-exposed Block object - The BlockBuilder may manipulate the Block, but after calling build() the calling code will have no option to edit the POJO (proper capsulation and sealing) --- .../main/scripts/cli/util/model/Block.java | 57 ++++----- .../test/java/cli/util/model/BlockTest.java | 108 +++++++++++++----- 2 files changed, 111 insertions(+), 54 deletions(-) diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java index bddf5f9b329..3aae9795725 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java @@ -1,6 +1,5 @@ package cli.util.model; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.EnumMap; @@ -137,15 +136,9 @@ public String getErrorMessage() { public static final class BlockBuilder { private final Configuration config; private final List
header; - private final Map settings = new EnumMap<>(Header.class); - private final List fieldBuilders = new ArrayList<>(); - private boolean hasErrors = false; - private boolean hasParsedALine = false; - private BlockBuilder() { - this.config = Configuration.defaultConfig(); - this.header = new ArrayList<>(); - } + private Block block; + private boolean hasErrors = false; /** * Create a builder with a line containing the header of the metadata block definition, so the order of @@ -182,39 +175,38 @@ public void parseAndValidateLine(final String line) throws ParserException { } // only 1 block definition allowed as per spec - if (this.hasParsedALine) { + if (this.block != null) { this.hasErrors = true; throw new ParserException("must not try to add another metadata block definition"); } else { - // need to set this here, even BEFORE actual parsing. If the line cannot be parsed, - // the exception is catched and another line is sent this way, we need to make sure - // we do not accept it! - this.hasParsedALine = true; + this.block = parseAndValidateColumns(line.split(config.columnSeparator())); } - - validateColumns(line.split(config.columnSeparator())); } /** - * Validate the columns (usually given by {@code parseAndValidateLine}). + * Parse and validate the columns (usually given by {@code parseAndValidateLine}). + * This is package private, becoming testable this way. * * @param lineParts + * @return A {@link Block} object (modifiable for builder internal use) * @throws ParserException */ - void validateColumns(final String[] lineParts) throws ParserException { + Block parseAndValidateColumns(final String[] lineParts) throws ParserException { if (lineParts == null || lineParts.length != header.size()) { throw new ParserException("does not match length of metadata block headline"); } + Block block = new Block(); ParserException parserException = new ParserException("has validation errors"); + for (int i = 0; i < lineParts.length; i++) { - Header column = header.get(i); + Block.Header column = header.get(i); String value = lineParts[i]; if( ! column.isValid(value)) { parserException.addSubException( "Invalid value '" + value + " for column '" + column + "', " + column.getErrorMessage()); } else { - this.settings.put(column, value); + block.set(column, value); } } @@ -222,15 +214,13 @@ void validateColumns(final String[] lineParts) throws ParserException { // setting this to true to ensure no block will be created accidentally via build(). this.hasErrors = true; throw parserException; + } else { + return block; } } - public boolean hasFailed() { - return this.hasErrors || ! this.hasParsedALine; - } - public boolean hasSucceeded() { - return ! this.hasErrors && this.hasParsedALine; + return ! this.hasErrors && this.block != null; } /** @@ -241,14 +231,27 @@ public boolean hasSucceeded() { public Block build() { if (hasSucceeded()) { // TODO: extend with adding the necessary bits about block properties, contained fields etc. - return new Block(); + return block; } else { throw new IllegalStateException("Trying to build a block with errors or without parsing a line first"); } } } + /* ---- Actual Block Class starting here ---- */ + + private final Map properties = new EnumMap<>(Header.class); + private Optional> fields = Optional.empty(); + private Block() {} - Optional> fields = Optional.empty(); + private void set(Header column, String value) { + this.properties.put(column, value); + } + public Optional get(Header column) { + return Optional.ofNullable(this.properties.get(column)); + } + public String get(Header column, String defaultValue) { + return this.properties.getOrDefault(column, defaultValue); + } } diff --git a/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java b/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java index 5fb8dcd01d7..aa6f9e46342 100644 --- a/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java +++ b/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java @@ -1,5 +1,8 @@ package cli.util.model; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.NullAndEmptySource; import org.junit.jupiter.params.provider.ValueSource; @@ -18,34 +21,85 @@ class BlockTest { private static final Logger logger = Logger.getLogger(BlockTest.class.getCanonicalName()); - @ParameterizedTest - @ValueSource(strings = { - "#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI", - "#metadataBlock\tNAME\tDataversealias\tDisplayname\tBlockURI" - }) - void successfulParseAndValidateHeaderLine(String headerLine) throws ParserException { - List headers = Block.Header.parseAndValidate(headerLine, Configuration.defaultConfig()); - assertFalse(headers.isEmpty()); - assertEquals(List.of(Block.Header.values()), headers); + static final Configuration config = Configuration.defaultConfig(); + static final String validHeaderLine = "#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI"; + static final String validBlockDef = "\tmyblock\tdataverse\tFooBar Block\thttps://foobar.com/"; + + @Nested + class HeaderTest { + @ParameterizedTest + @ValueSource(strings = { + validHeaderLine, + "#metadataBlock\tNAME\tDataversealias\tDisplayname\tBlockURI" + }) + void successfulParseAndValidateHeaderLine(String headerLine) throws ParserException { + List headers = Block.Header.parseAndValidate(headerLine, config); + assertFalse(headers.isEmpty()); + assertEquals(List.of(Block.Header.values()), headers); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = { + "hello", + "#metadataBlock test", + "#metadataBlock\tname\tdataverseAlias\tdisplayName", + "\t#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI", + "#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI\tfoobar", + "#metadataBlock\tname\tdataverseAlias\tdisplayName\tdisplayName\tblockURI", + "dataverseAlias\tdisplayName\tblockURI\t#metadataBlock\tname" + }) + void failingParseAndValidateHeaderLine(String headerLine) throws ParserException { + ParserException exception = assertThrows(ParserException.class, () -> Block.Header.parseAndValidate(headerLine, config)); + assertTrue(exception.hasSubExceptions()); + logger.log(Level.FINE, + exception.getSubExceptions().stream().map(Throwable::getMessage).collect(Collectors.joining("\n")) + ); + } } - @ParameterizedTest - @NullAndEmptySource - @ValueSource(strings = { - "hello", - "#metadataBlock test", - "#metadataBlock\tname\tdataverseAlias\tdisplayName", - "\t#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI", - "#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI\tfoobar", - "#metadataBlock\tname\tdataverseAlias\tdisplayName\tdisplayName\tblockURI", - "dataverseAlias\tdisplayName\tblockURI\t#metadataBlock\tname" - }) - void failingParseAndValidateHeaderLine(String headerLine) throws ParserException { - ParserException exception = assertThrows(ParserException.class, () -> Block.Header.parseAndValidate(headerLine, Configuration.defaultConfig())); - assertTrue(exception.hasSubExceptions()); - logger.log(Level.INFO, - exception.getSubExceptions().stream().map(Throwable::getMessage).collect(Collectors.joining("\n")) - ); + @Nested + class ParseLineTest { + Block.BlockBuilder builder; + + @BeforeEach + void setUp() throws ParserException { + builder = new Block.BlockBuilder(validHeaderLine, config); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = { + "\t", + "myblock", + "\tmyblock\t", + "\tmyblock\tdataverse", + "\tmyblock\tdataverse\tFooBar Block", + "\tmyblock\tdataverse\tFooBar Block\thttps://", + "\tmyblock\tdataverse\tFooBar Block\thttps://foobar.com/\thello", + "myblock\tdataverse\tFooBar Block\thttps://foobar.com/" + }) + void failingParseLine(String line) throws ParserException { + ParserException exception = assertThrows(ParserException.class, () -> builder.parseAndValidateLine(line)); + assertFalse(builder.hasSucceeded()); + } + + @ParameterizedTest + @ValueSource(strings = { + validBlockDef + }) + void succeedingParseLine(String line) throws ParserException { + builder.parseAndValidateLine(line); + assertTrue(builder.hasSucceeded()); + } + + @Test + void failingDoubleAdditionAttempt() throws ParserException { + builder.parseAndValidateLine(validBlockDef); + assertTrue(builder.hasSucceeded()); + ParserException exception = assertThrows(ParserException.class, () -> builder.parseAndValidateLine(validBlockDef)); + assertFalse(builder.hasSucceeded()); + } } -} \ No newline at end of file +} From 03d19e0ae9f0fd2b3e914a5f99e1e5dbbf789e6c Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 18:42:09 +0200 Subject: [PATCH 49/56] feat(solrteur): add types enum to Field #7662 Add field types and make them usable as predicates for fields. Add test. --- .../main/scripts/cli/util/model/Field.java | 48 +++++++++++++++++-- .../test/java/cli/util/model/FieldTest.java | 35 ++++++++++++++ 2 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 modules/solr-configset/src/test/java/cli/util/model/FieldTest.java diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java index a8308867c8c..9dccd64b55d 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java @@ -103,15 +103,53 @@ public String getErrorMessage() { } } + public enum Types implements Predicate { + NONE("none"), + DATE("date"), + EMAIL("email"), + TEXT("text"), + TEXTBOX("textbox"), + URL("url"), + INT("int"), + FLOAT("float"); + + private final String name; + + Types(final String name) { + this.name = name; + } - public static final class FieldBuilder { - // TODO: extend! - public Field build() { - return new Field(); + private static final Map valueMap; + static { + Map map = new ConcurrentHashMap<>(); + Arrays.stream(Types.values()).forEach(type -> map.put(type.toString(), type)); + valueMap = Collections.unmodifiableMap(map); + } + + @Override + public boolean test(String sut) { + // we demand correct case! + return this.toString().equals(sut); + } + + public static Predicate matchesTypes() { + Predicate test = NONE; + for (Types type : Types.values()) { + test = test.or(type); + } + return test; + } + + public static List getTypesList() { + return valueMap.keySet().stream().collect(Collectors.toUnmodifiableList()); + } + + @Override + public String toString() { + return this.name; } } - private Field() {} Optional> controlledVocabularyValues = Optional.empty(); } diff --git a/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java b/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java new file mode 100644 index 00000000000..651bf71455a --- /dev/null +++ b/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java @@ -0,0 +1,35 @@ +package cli.util.model; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.function.Predicate; +import java.util.logging.Logger; + +import static org.junit.jupiter.api.Assertions.*; + +class FieldTest { + + private static final Logger logger = Logger.getLogger(BlockTest.class.getCanonicalName()); + + @Nested + class TypesTest { + Predicate allowedTypes = Field.Types.matchesTypes(); + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {"foobar", "hello_hello", "NONE", "DATE"}) + void failing(String subject) { + assertFalse(allowedTypes.test(subject)); + } + + @ParameterizedTest + @ValueSource(strings = {"none", "text", "textbox", "date", "email", "int", "float"}) + void succeeding(String subject) { + assertTrue(allowedTypes.test(subject)); + } + } + +} \ No newline at end of file From 13de6d58c86ff0c9d78af9382b5bb42217f94826 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 19:17:55 +0200 Subject: [PATCH 50/56] fix(solrteur): make headers not match null #7662 Predicates are not null safe - need to make validate() check for null --- .../solr-configset/src/main/scripts/cli/util/model/Block.java | 2 +- .../src/main/scripts/cli/util/model/ControlledVocabulary.java | 2 +- .../solr-configset/src/main/scripts/cli/util/model/Field.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java index 3aae9795725..d223da7e02e 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Block.java @@ -116,7 +116,7 @@ public String toString() { * @return True if matching or false in every other case. */ public boolean isValid(final String sut) { - return test.test(sut); + return sut != null && test.test(sut); } /** diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java b/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java index 26bc8f3caef..48645dbf14e 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java @@ -95,7 +95,7 @@ public String toString() { } public boolean isValid(final String sut) { - return test.test(sut); + return sut != null && test.test(sut); } public String getErrorMessage() { diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java index 9dccd64b55d..4e282c896b7 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java @@ -95,7 +95,7 @@ public String toString() { } public boolean isValid(final String sut) { - return test.test(sut); + return sut != null && test.test(sut); } public String getErrorMessage() { From 52dc5a90a3cf1692486c277820c89e3eda0772d2 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 29 Apr 2022 19:18:51 +0200 Subject: [PATCH 51/56] feat(solrteur): add all headers to Field #7662 Includes all the predicates according to spec and test for them. --- .../main/scripts/cli/util/model/Field.java | 69 ++++++++++++++++++- .../test/java/cli/util/model/FieldTest.java | 62 +++++++++++++++++ 2 files changed, 130 insertions(+), 1 deletion(-) diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java index 4e282c896b7..6ca9a3b5302 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java +++ b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java @@ -1,5 +1,6 @@ package cli.util.model; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -7,22 +8,88 @@ import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; +import java.util.regex.Pattern; import java.util.stream.Collectors; public final class Field { public static final String KEYWORD = "datasetField"; + /** + * Currently, the spec says we need to comply with Solr and DDI rules. + * De-facto commonly used are camel-cased names. + * See also https://solr.apache.org/guide/8_11/defining-fields.html#field-properties + * + * This regex matches the strict Solr name spec plus adds "." as valid chars + * (we need it for comptability with astrophysics.tsv) + */ + public static final String NAME_PATTERN = "^(?=_[\\w.]+[^_]$|[^_][\\w.]+_?$)[a-zA-Z_][\\w.]+$"; + public static final String WHITESPACE_ONLY = "^\\s+$"; + + public static final Predicate matchBoolean = s -> "TRUE".equals(s) || "FALSE".equals(s); + /** * Programmatic variant of the spec of a #datasetField. List all the column headers and associated restrictions * on the values of a column. */ public enum Header { - // TODO: extend! KEYWORD( Field.KEYWORD, String::isEmpty, "must have no value (be empty)" + ), + + NAME( + "name", + Predicate.not(String::isBlank).and(Pattern.compile(Field.NAME_PATTERN).asMatchPredicate()), + "must not be blank and match regex pattern " + Field.NAME_PATTERN + ), + TITLE("title"), + DESCRIPTION("description"), + WATERMARK( + "watermark", + Predicate.not(Pattern.compile(WHITESPACE_ONLY).asMatchPredicate()), + "must not be whitespace only" + ), + + FIELDTYPE( + "fieldType", + Types.matchesTypes(), + "must be one of [" + String.join(", ", Types.getTypesList()) + "]" + ), + + DISPLAY_ORDER( + "displayOrder", + Pattern.compile("\\d+").asMatchPredicate(), + "must be a non-negative integer" + ), + DISPLAY_FORMAT( + "displayFormat", + Predicate.not(Pattern.compile(WHITESPACE_ONLY).asMatchPredicate()), + "must not be whitespace only" + ), + + ADVANCED_SEARCH_FIELD("advancedSearchField", matchBoolean, "must be 'TRUE' or 'FALSE"), + ALLOW_CONTROLLED_VOCABULARY("allowControlledVocabulary", matchBoolean, "must be 'TRUE' or 'FALSE"), + ALLOW_MULTIPLES("allowmultiples", matchBoolean, "must be 'TRUE' or 'FALSE"), + FACETABLE("facetable", matchBoolean, "must be 'TRUE' or 'FALSE"), + DISPLAY_ON_CREATE("displayoncreate", matchBoolean, "must be 'TRUE' or 'FALSE"), + REQUIRED("required", matchBoolean, "must be 'TRUE' or 'FALSE"), + + PARENT( + "parent", + Pattern.compile(Field.NAME_PATTERN).asMatchPredicate().or(String::isEmpty), + "must be either empty or match regex pattern " + Field.NAME_PATTERN + ), + METADATABLOCK_ID( + "metadatablock_id", + Predicate.not(String::isBlank).and(Pattern.compile(Block.NAME_PATTERN).asMatchPredicate()), + "must not be blank and match regex pattern " + Block.NAME_PATTERN + ), + TERM_URI( + "termURI", + s -> s.isEmpty() || Validator.isValidUrl(s), + "must be empty or a valid URL" ); private final String value; diff --git a/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java b/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java index 651bf71455a..021e406d2ab 100644 --- a/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java +++ b/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java @@ -2,7 +2,9 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EmptySource; import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.NullSource; import org.junit.jupiter.params.provider.ValueSource; import java.util.function.Predicate; @@ -32,4 +34,64 @@ void succeeding(String subject) { } } + @Nested + class HeaderTest { + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {" ", "\t", "_foobar_", "_foo_bar_"}) + void invalidNames(String subject) { + assertFalse(Field.Header.NAME.isValid(subject)); + } + + @ParameterizedTest + @ValueSource(strings = {"foobar_", "foo_bar_", "_foobar", "_foo_bar", "foobar", "foobar1234", "foo_bar_1234"}) + void validNames(String subject) { + assertTrue(Field.Header.NAME.isValid(subject)); + assertTrue(Field.Header.PARENT.isValid(subject)); + } + + @ParameterizedTest + @EmptySource + void validParentName(String subject) { + assertTrue(Field.Header.PARENT.isValid(subject)); + } + + @ParameterizedTest + @NullSource + @ValueSource(strings = {" ", "\t"}) + void invalidEmptyOrText(String subject) { + assertFalse(Field.Header.WATERMARK.isValid(subject)); + assertFalse(Field.Header.DISPLAY_FORMAT.isValid(subject)); + } + + @ParameterizedTest + @ValueSource(strings = { "", "foobar", "My name is Hase, I know about nothing."}) + void validEmptyOrText(String subject) { + assertTrue(Field.Header.WATERMARK.isValid(subject)); + assertTrue(Field.Header.DISPLAY_FORMAT.isValid(subject)); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = { "true", "false", "0", "1", "foobar"}) + void invalidBoolean(String subject) { + assertFalse(Field.Header.ADVANCED_SEARCH_FIELD.isValid(subject)); + assertFalse(Field.Header.ALLOW_CONTROLLED_VOCABULARY.isValid(subject)); + assertFalse(Field.Header.ALLOW_MULTIPLES.isValid(subject)); + assertFalse(Field.Header.FACETABLE.isValid(subject)); + assertFalse(Field.Header.DISPLAY_ON_CREATE.isValid(subject)); + assertFalse(Field.Header.REQUIRED.isValid(subject)); + } + + @ParameterizedTest + @ValueSource(strings = { "TRUE", "FALSE" }) + void validBoolean(String subject) { + assertTrue(Field.Header.ADVANCED_SEARCH_FIELD.isValid(subject)); + assertTrue(Field.Header.ALLOW_CONTROLLED_VOCABULARY.isValid(subject)); + assertTrue(Field.Header.ALLOW_MULTIPLES.isValid(subject)); + assertTrue(Field.Header.FACETABLE.isValid(subject)); + assertTrue(Field.Header.DISPLAY_ON_CREATE.isValid(subject)); + assertTrue(Field.Header.REQUIRED.isValid(subject)); + } + } } \ No newline at end of file From bc065374f4909f62030fcd0bd6f70609ee53a6d2 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 16 Feb 2023 01:21:05 +0100 Subject: [PATCH 52/56] feat(solrteur): move classes and extend field parser #7662 --- modules/solr-configset/pom.xml | 45 +- .../io/gdcc/solrteur/cmd/CompileSchema.java | 188 +++++++ .../gdcc/solrteur}/cmd/CompileSolrConfig.java | 14 +- .../gdcc/solrteur}/cmd/ExtractConfigSet.java | 15 +- .../io/gdcc/solrteur/cmd/package-info.java | 1 + .../solrteur/mdb/MetadataBlockTSVReader.java | 184 +++++++ .../io/gdcc/solrteur/mdb/package-info.java | 1 + .../io/gdcc/solrteur/mdb/tsv}/Block.java | 51 +- .../gdcc/solrteur/mdb/tsv}/Configuration.java | 14 +- .../mdb/tsv}/ControlledVocabulary.java | 2 +- .../java/io/gdcc/solrteur/mdb/tsv/Field.java | 480 ++++++++++++++++++ .../gdcc/solrteur/mdb/tsv}/ParserError.java | 2 +- .../solrteur/mdb/tsv/ParserException.java | 39 ++ .../gdcc/solrteur/mdb/tsv}/ParsingState.java | 11 +- .../io/gdcc/solrteur/mdb/tsv}/Validator.java | 4 +- .../java/io/gdcc/solrteur/package-info.java | 1 + .../io/gdcc/solrteur}/solrteur.java | 29 +- .../main/scripts/cli/cmd/package-info.java | 1 - .../main/scripts/cli/util/model/Field.java | 222 -------- .../cli/util/model/ParserException.java | 25 - .../scripts/cli/util/model/package-info.java | 1 - .../main/scripts/cli/util/package-info.java | 1 - .../test/java/cli/util/model/FieldTest.java | 97 ---- .../gdcc/solrteur/mdb/tsv}/BlockTest.java | 2 +- .../solrteur/mdb/tsv}/ConfigurationTest.java | 3 +- .../io/gdcc/solrteur/mdb/tsv/FieldTest.java | 177 +++++++ .../solrteur/mdb/tsv}/ParsingStateTest.java | 8 +- .../gdcc/solrteur/mdb/tsv}/ValidatorTest.java | 5 +- 28 files changed, 1226 insertions(+), 397 deletions(-) create mode 100644 modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSchema.java rename modules/solr-configset/src/main/{scripts/cli => java/io/gdcc/solrteur}/cmd/CompileSolrConfig.java (92%) rename modules/solr-configset/src/main/{scripts/cli => java/io/gdcc/solrteur}/cmd/ExtractConfigSet.java (90%) create mode 100644 modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/package-info.java create mode 100644 modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/MetadataBlockTSVReader.java create mode 100644 modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/package-info.java rename modules/solr-configset/src/main/{scripts/cli/util/model => java/io/gdcc/solrteur/mdb/tsv}/Block.java (87%) rename modules/solr-configset/src/main/{scripts/cli/util/model => java/io/gdcc/solrteur/mdb/tsv}/Configuration.java (79%) rename modules/solr-configset/src/main/{scripts/cli/util/model => java/io/gdcc/solrteur/mdb/tsv}/ControlledVocabulary.java (99%) create mode 100644 modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Field.java rename modules/solr-configset/src/main/{scripts/cli/util/model => java/io/gdcc/solrteur/mdb/tsv}/ParserError.java (91%) create mode 100644 modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParserException.java rename modules/solr-configset/src/main/{scripts/cli/util/model => java/io/gdcc/solrteur/mdb/tsv}/ParsingState.java (77%) rename modules/solr-configset/src/main/{scripts/cli/util/model => java/io/gdcc/solrteur/mdb/tsv}/Validator.java (98%) create mode 100644 modules/solr-configset/src/main/java/io/gdcc/solrteur/package-info.java rename modules/solr-configset/src/main/{scripts/cli => java/io/gdcc/solrteur}/solrteur.java (79%) delete mode 100644 modules/solr-configset/src/main/scripts/cli/cmd/package-info.java delete mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/Field.java delete mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/ParserException.java delete mode 100644 modules/solr-configset/src/main/scripts/cli/util/model/package-info.java delete mode 100644 modules/solr-configset/src/main/scripts/cli/util/package-info.java delete mode 100644 modules/solr-configset/src/test/java/cli/util/model/FieldTest.java rename modules/solr-configset/src/test/java/{cli/util/model => io/gdcc/solrteur/mdb/tsv}/BlockTest.java (99%) rename modules/solr-configset/src/test/java/{cli/util/model => io/gdcc/solrteur/mdb/tsv}/ConfigurationTest.java (91%) create mode 100644 modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/FieldTest.java rename modules/solr-configset/src/test/java/{cli/util/model => io/gdcc/solrteur/mdb/tsv}/ParsingStateTest.java (91%) rename modules/solr-configset/src/test/java/{cli/util/model => io/gdcc/solrteur/mdb/tsv}/ValidatorTest.java (95%) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index 75604d53508..b13b457d577 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -17,6 +17,7 @@ ${project.build.directory}/${solr.configset} pom + ${project.build.sourceDirectory}/io/gdcc/solrteur/solrteur.java @@ -49,7 +50,7 @@ download-solr - + initialize wget @@ -68,6 +69,22 @@ jbang-maven-plugin 0.0.7 + + extract-template + generate-resources + + run + + + + --solr-version="${solr.version}" + --target="${solr.configset.directory}" + extract-zip + --zip="${project.build.directory}/solr-${solr.version}.zip" + + + + compile-solrconfig-xml process-resources @@ -76,14 +93,30 @@ - solrconfig --solr-version="${solr.version}" - --zip="${project.build.directory}/solr-${solr.version}.zip" - --configset="${solr.configset.directory}" + --target="${solr.configset.directory}" + solrconfig --xslts="${project.resources[0].directory}/solrconfig-xslt" - --schemaxml="${project.resources[0].directory}/schema.xml" - + + + + + compile-schema + process-resources + + run + + + + --solr-version="${solr.version}" + --target="${solr.configset.directory}" + schema + + --tsvs="${project.resources[0].directory}/tsv" + --base="${project.resources[0].directory}/schema.xml" + + diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSchema.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSchema.java new file mode 100644 index 00000000000..c9db66f2bc0 --- /dev/null +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSchema.java @@ -0,0 +1,188 @@ +package io.gdcc.solrteur.cmd; + +import io.gdcc.solrteur.mdb.tsv.Block; +import io.gdcc.solrteur.mdb.MetadataBlockTSVReader; +import io.gdcc.solrteur.mdb.tsv.Field; +import io.gdcc.solrteur.mdb.tsv.ParserException; +import io.gdcc.solrteur.solrteur; +import picocli.CommandLine; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.Callable; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +@CommandLine.Command( + name = "schema", + mixinStandardHelpOptions = true, + usageHelpAutoWidth = true, + showDefaultValues = true, + sortOptions = false, + description = "Compile the schema.xml from a template and add metadata fields from blocks%n") +public class CompileSchema implements Callable { + + @CommandLine.ParentCommand + private solrteur cliParent; + + @CommandLine.Option(required = true, + names = {"--base"}, + paramLabel = "", + description = "Path to the base schema.xml, to be enriched with fields from blocks") + private String baseSchemaPath; + + @CommandLine.Option(required = true, + names = {"--tsvs"}, + paramLabel = "", + description = "Repeatable argument to directories and/or TSV files with metadata blocks.") + private Path[] mdbTsvPaths; + + @CommandLine.Option(required = false, + names = {"--mfb"}, + paramLabel = "", + description = "String to indicate a mark after which all should be inserted.", + defaultValue = "SCHEMA-FIELDS::BEGIN") + private String markFieldsBegin; + + @CommandLine.Option(required = false, + names = {"--mfe"}, + paramLabel = "", + description = "String to indicate a mark where insertion of all shall end.", + defaultValue = "SCHEMA-FIELDS::END") + private String markFieldsEnd; + + @CommandLine.Option(required = false, + names = {"--mcfb"}, + paramLabel = "", + description = "String to indicate a mark after which all should be inserted.", + defaultValue = "SCHEMA-COPY-FIELDS::BEGIN") + private String markCopyFieldsBegin; + + @CommandLine.Option(required = false, + names = {"--mcfe"}, + paramLabel = "", + description = "String to indicate a mark where insertion of all shall end.", + defaultValue = "SCHEMA-COPY-FIELDS::END") + private String markCopyFieldsEnd; + + /** + * Business logic routine, calling all the execution steps. + * @return The exit code + */ + @Override + public Integer call() throws Exception { + getFieldsFromBlocks(); + + return CommandLine.ExitCode.OK; + } + + private void getFieldsFromBlocks() throws solrteur.AbortScriptException { + + MetadataBlockTSVReader reader = new MetadataBlockTSVReader(); + + // Walk all locations and add files to a large list + List allTsvFiles = new ArrayList<>(); + for (Path path : mdbTsvPaths) { + if (Files.isDirectory(path)) { + allTsvFiles.addAll(findTsvFiles(path)); + } else if (Files.isReadable(path) && Files.isRegularFile(path)) { + allTsvFiles.add(path); + } + } + + // Much nicer in output if sorted + Collections.sort(allTsvFiles); + + // Iterate the files and read the blocks to have a set of 'em (necessary for cross-checking fields) + Map blockPathMap = new HashMap<>(); + boolean hadErrors = false; + for (Path path : allTsvFiles) { + try { + List lines = Files.readAllLines(path, StandardCharsets.UTF_8); + Block block = reader.retrieveBlock(lines); + + if (blockPathMap.containsKey(block)) { + throw new ParserException("Metadata block " + block.getName() + "already present in " + blockPathMap.get(block) + "!"); + } + blockPathMap.put(block, path); + + } catch (ParserException e) { + // Log a warning but continue parsing with next file to get done as much as possible. + hadErrors = true; + logErrors(path, e); + } catch (IOException e) { + // Log a warning but continue parsing with next file to get done as much as possible. + hadErrors = true; + solrteur.Logger.warn(new solrteur.AbortScriptException("Could not read "+path, e)); + } + } + + // If all blocks could be read and are valid, let's extract the fields + Map> blockFieldsMap = new HashMap<>(); + if (!hadErrors) { + Set blocks = blockPathMap.keySet(); + //blocks.forEach(b -> System.out.println(b.getName())); + + for (Map.Entry mdb : blockPathMap.entrySet()) { + Block block = mdb.getKey(); + Path path = mdb.getValue(); + + try { + List lines = Files.readAllLines(path, StandardCharsets.UTF_8); + // First store all retrieved fields, we check on uniqueness later + blockFieldsMap.put(block, reader.retrieveFields(lines, blocks)); + } catch (ParserException e) { + // Log a warning but continue parsing with next file to get done as much as possible. + logErrors(path, e); + } catch (IOException e) { + // Log a warning but continue parsing with next file to get done as much as possible. + solrteur.Logger.warn(new solrteur.AbortScriptException("Could not read "+path, e)); + } + } + } + + } + private void injectFields() {} + private void injectCopyFields() {} + + private List findTsvFiles(Path dir) throws solrteur.AbortScriptException { + if (!Files.isDirectory(dir)) { + return List.of(); + } + + try (Stream walk = Files.walk(dir, 2)) { + return walk + .filter(Files::isRegularFile) + .filter(Files::isReadable) + .filter(path -> path.toString().endsWith(".tsv")) + .sorted() + .collect(Collectors.toList()); + } catch (IOException e) { + throw new solrteur.AbortScriptException("Could not walk over TSV files at " + dir, e); + } + } + + private void logErrors(final Path path, final ParserException e) { + String fileName = path.getFileName().toString(); + logPE(fileName, "", "", e); + } + + private void logPE(final String fileName, final String lineNumber, final String indent, final ParserException e) { + String ln = lineNumber.isEmpty() ? e.getLineNumber() : lineNumber; + + solrteur.Logger.warn(fileName + ln + ": " + indent + e.getMessage()); + for (ParserException pe : e.getSubExceptions()) { + logPE(fileName, ln, indent + " ", pe); + } + } +} diff --git a/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSolrConfig.java similarity index 92% rename from modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java rename to modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSolrConfig.java index d4949a2752d..0a5269f2d99 100644 --- a/modules/solr-configset/src/main/scripts/cli/cmd/CompileSolrConfig.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSolrConfig.java @@ -1,7 +1,8 @@ //DEPS net.sf.saxon:Saxon-HE:10.6 -package cli.cmd; +package io.gdcc.solrteur.cmd; +import io.gdcc.solrteur.solrteur; import net.sf.saxon.s9api.Processor; import net.sf.saxon.s9api.SaxonApiException; import net.sf.saxon.s9api.Serializer; @@ -24,8 +25,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import cli.solrteur.AbortScriptException; -import cli.solrteur.Logger; +import io.gdcc.solrteur.solrteur.AbortScriptException; +import io.gdcc.solrteur.solrteur.Logger; @Command(name = "solrconfig", mixinStandardHelpOptions = true, @@ -33,11 +34,10 @@ showDefaultValues = true, sortOptions = false, description = "Compile the solrconfig.xml from a source and XSLT files%n") -public -class CompileSolrConfig implements Callable { +public class CompileSolrConfig implements Callable { @ParentCommand - private cli.solrteur cliParent; + private solrteur cliParent; @Option(required = true, names = {"--xslts"}, @@ -114,5 +114,7 @@ private void applySolrConfigXSLT() throws AbortScriptException { } catch (SaxonApiException e) { throw new AbortScriptException("XML transformation failed", e); } + + Logger.info("Finished applying XSLT transformations, saved to " + solrConfig); } } diff --git a/modules/solr-configset/src/main/scripts/cli/cmd/ExtractConfigSet.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/ExtractConfigSet.java similarity index 90% rename from modules/solr-configset/src/main/scripts/cli/cmd/ExtractConfigSet.java rename to modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/ExtractConfigSet.java index 47272c71313..e51c4ec2457 100644 --- a/modules/solr-configset/src/main/scripts/cli/cmd/ExtractConfigSet.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/ExtractConfigSet.java @@ -1,6 +1,6 @@ -package cli.cmd; +package io.gdcc.solrteur.cmd; -import cli.solrteur; +import io.gdcc.solrteur.solrteur; import picocli.CommandLine; import picocli.CommandLine.Command; import picocli.CommandLine.ParentCommand; @@ -18,8 +18,8 @@ import java.nio.file.attribute.BasicFileAttributes; import java.util.concurrent.Callable; -import cli.solrteur.AbortScriptException; -import cli.solrteur.Logger; +import io.gdcc.solrteur.solrteur.AbortScriptException; +import io.gdcc.solrteur.solrteur.Logger; @Command( name = "extract-zip", @@ -64,7 +64,8 @@ private void replaceVariables() throws AbortScriptException { private void extractConfigSet() throws AbortScriptException { // Wrap the file system in a try-with-resources statement // to auto-close it when finished and prevent a memory leak - try (FileSystem zipFileSystem = FileSystems.newFileSystem(solrZipFile, null)) { + // NOTE: the explicit casting to ClassLoader is necessary to make this compatible with JDK 11 _and_ 17 + try (FileSystem zipFileSystem = FileSystems.newFileSystem(solrZipFile, (ClassLoader) null)) { Path zipSource = zipFileSystem.getPath(solrConfigSetZipPath); // TODO: should we delete the target before copying the new content? (Usually this shouldn't change, but better safe than sorry?) @@ -79,7 +80,7 @@ public FileVisitResult preVisitDirectory(Path zippedDir, BasicFileAttributes att Path targetDir = Path.of(cliParent.getTargetDir().toString(), strippedZipPath); try { - Logger.info(solrZipFile + ":" + zippedDir + " -> " + targetDir); + //Logger.info(solrZipFile + ":" + zippedDir + " -> " + targetDir); Files.copy(zippedDir, targetDir, StandardCopyOption.COPY_ATTRIBUTES); } catch (FileAlreadyExistsException e) { // intentional ignore - simply reuse the existing directory @@ -94,7 +95,7 @@ public FileVisitResult visitFile(Path zippedFile, BasicFileAttributes attrs) thr String strippedZipPath = zippedFile.toString().substring(solrConfigSetZipPath.length()); Path targetFile = Path.of(cliParent.getTargetDir().toString(), strippedZipPath); - Logger.info(solrZipFile + ":" + zippedFile + " -> " + targetFile); + //Logger.info(solrZipFile + ":" + zippedFile + " -> " + targetFile); Files.copy(zippedFile, targetFile, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES); return FileVisitResult.CONTINUE; diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/package-info.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/package-info.java new file mode 100644 index 00000000000..485085204df --- /dev/null +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/package-info.java @@ -0,0 +1 @@ +package io.gdcc.solrteur.cmd; \ No newline at end of file diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/MetadataBlockTSVReader.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/MetadataBlockTSVReader.java new file mode 100644 index 00000000000..71699efdf75 --- /dev/null +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/MetadataBlockTSVReader.java @@ -0,0 +1,184 @@ +package io.gdcc.solrteur.mdb; + +import io.gdcc.solrteur.mdb.tsv.Block; +import io.gdcc.solrteur.mdb.tsv.Configuration; +import io.gdcc.solrteur.mdb.tsv.Field; +import io.gdcc.solrteur.mdb.tsv.ParserException; +import io.gdcc.solrteur.mdb.tsv.ParsingState; + +import javax.swing.text.html.parser.Parser; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +public class MetadataBlockTSVReader { + + private Configuration config; + + public MetadataBlockTSVReader() { + this.config = Configuration.defaultConfig(); + } + + public MetadataBlockTSVReader(Configuration config) { + this.config = config; + } + + /** + * Extract the metadata block definition from a single TSV file and return it. + * @param lines + * @return + * @throws ParserException + */ + public Block retrieveBlock(final List lines) throws ParserException { + // Assertions + Objects.requireNonNull(lines, "You must provide a list of strings, it may be empty but not null"); + + ParsingState state = ParsingState.Init; + Block.BlockBuilder blockBuilder = null; + ParserException parentException = new ParserException("Has errors:"); + int lineIndex = 0; + + for (String line : lines) { + // Skip lines that are empty, blanks only or comments + if (line.isBlank() || line.startsWith(config.commentIndicator())) { + // Increment line counter before skipping to next line + lineIndex++; + continue; + } + + // If the line starts with a new section trigger, analyse the state + if (line.startsWith(config.triggerIndicator())) { + state = state.transitionState(line, config); + + // Being here means we transitioned from one state to the next - otherwise there would have been an exception. + switch (state) { + case MetadataBlock: + try { + blockBuilder = new Block.BlockBuilder(line, config); + } catch (ParserException e) { + // This is critical, as we cannot parse the following lines with a broken header + throw e.withLineNumber(lineIndex); + } + break; + case Fields: + Objects.requireNonNull(blockBuilder, "BlockBuilder not initialized, cannot build block"); + + // In case there had been parsing errors, stop here + if (parentException.hasSubExceptions()) { + throw parentException; + } + + // We managed to complete parsing the block section, return the block now. + // The last line of the block section is the line before this one (which transitioned the state) + return blockBuilder.build(lineIndex-1); + default: + // Intentionally left blank, as the other sections are of no interest to us here. + } + } else { + // Proceed analysis + switch(state) { + case Init: throw new ParserException("Only comments, empty or blank lines allowed before block definition") + .withLineNumber(lineIndex); + case MetadataBlock: + Objects.requireNonNull(blockBuilder, "BlockBuilder not initialized, cannot parse"); + try { + blockBuilder.parseAndValidateLine(line); + } catch (ParserException e) { + parentException.addSubException(e.withLineNumber(lineIndex)); + } + break; + default: + throw new ParserException("We should never see this exception, as we looked for the block only") + .withLineNumber(lineIndex); + } + } + + // Increment line counter + lineIndex++; + } + + // The trigger switch did not kick in - only one explanation. + throw new ParserException("Missing fields section."); + } + + public List retrieveFields(final List lines, final Set knownBlocks) throws ParserException { + Objects.requireNonNull(lines, "You must provide a list of strings, it may be empty but not null"); + Objects.requireNonNull(knownBlocks, "You must provide a set of known blocks, it may be empty"); + + // Read the block again, so we are at that stage and can continue with fields + Block currentBlock = retrieveBlock(lines); + ParsingState state = ParsingState.MetadataBlock; + + int lineIndex = currentBlock.getIndexLastLineofBlockSection()+1; + List linesAfterBlock = lines.stream() + .skip(lineIndex) + .collect(Collectors.toUnmodifiableList()); + + Field.FieldsBuilder fieldsBuilder = null; + ParserException parentException = new ParserException("Has errors:"); + + for (String line : linesAfterBlock) { + // Skip lines that are empty, blanks only or comments + if (line.isBlank() || line.startsWith(config.commentIndicator())) { + // Increment line counter + lineIndex++; + continue; + } + + // If the line starts with a new section trigger, analyse the state + if (line.startsWith(config.triggerIndicator())) { + state = state.transitionState(line, config); + + // Being here means we transitioned from one state to the next - otherwise there would have been an exception. + switch (state) { + case Fields: + try { + fieldsBuilder = new Field.FieldsBuilder(line, currentBlock.getName(), config); + } catch (ParserException e) { + // This is critical, as we cannot parse the following lines with a broken header + throw e.withLineNumber(lineIndex); + } + break; + case Vocabularies: + // We managed to get to the vocab section, meaning the fields are all done. Return fields! + Objects.requireNonNull(fieldsBuilder, "FieldsBuilder not initialized, cannot build fields"); + + // In case there had been parsing errors, stop here + if (parentException.hasSubExceptions()) { + throw parentException; + } + + return fieldsBuilder.build(); + default: + // Intentionally left blank, as the other sections are of no interest to us here. + } + } else { + // Proceed analysis + switch (state) { + case Fields: + Objects.requireNonNull(fieldsBuilder, "FieldsBuilder not initialized, cannot parse"); + + try{ + // TODO: Extend with checking if this field is for a different block (which is allowed by spec) + fieldsBuilder.parseAndValidateLine(lineIndex, line); + } catch (ParserException e) { + parentException.addSubException(e.withLineNumber(lineIndex)); + } + + break; + default: + throw new ParserException("We should never see this exception, as we looked for the fields only") + .withLineNumber(lineIndex); + } + } + + // Increment line counter + lineIndex++; + } + + // The trigger switch did not kick in - only one explanation. + throw new ParserException("Missing fields section."); + } + +} diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/package-info.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/package-info.java new file mode 100644 index 00000000000..a74044f0e2b --- /dev/null +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/package-info.java @@ -0,0 +1 @@ +package io.gdcc.solrteur.mdb; \ No newline at end of file diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Block.java similarity index 87% rename from modules/solr-configset/src/main/scripts/cli/util/model/Block.java rename to modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Block.java index d223da7e02e..b3b12f32a52 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Block.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Block.java @@ -1,10 +1,11 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; import java.util.Arrays; import java.util.Collections; import java.util.EnumMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; @@ -171,13 +172,13 @@ public void parseAndValidateLine(final String line) throws ParserException { // no null or blank lines for the parser. (blank lines can be skipped and not sent here by calling code) if (line == null || line.isBlank()) { this.hasErrors = true; - throw new ParserException("must not be empty nor blanks only nor null."); + throw new ParserException("Must not be empty nor blanks only nor null."); } // only 1 block definition allowed as per spec if (this.block != null) { this.hasErrors = true; - throw new ParserException("must not try to add another metadata block definition"); + throw new ParserException("Must not add more than one metadata block definition"); } else { this.block = parseAndValidateColumns(line.split(config.columnSeparator())); } @@ -192,19 +193,19 @@ public void parseAndValidateLine(final String line) throws ParserException { * @throws ParserException */ Block parseAndValidateColumns(final String[] lineParts) throws ParserException { - if (lineParts == null || lineParts.length != header.size()) { - throw new ParserException("does not match length of metadata block headline"); + if (lineParts == null || lineParts.length > header.size()) { + throw new ParserException("Does not match length of metadata block headline"); } Block block = new Block(); - ParserException parserException = new ParserException("has validation errors"); + ParserException parserException = new ParserException("Has validation errors:"); for (int i = 0; i < lineParts.length; i++) { Block.Header column = header.get(i); String value = lineParts[i]; if( ! column.isValid(value)) { parserException.addSubException( - "Invalid value '" + value + " for column '" + column + "', " + column.getErrorMessage()); + "Invalid value '" + value + "' for column '" + column + "', " + column.getErrorMessage()); } else { block.set(column, value); } @@ -228,9 +229,9 @@ public boolean hasSucceeded() { * block that has been analysed. Will execute associated field builders (which will execute associated * vocabulary builders). */ - public Block build() { + public Block build(int indexLastLineofBlockSection) { if (hasSucceeded()) { - // TODO: extend with adding the necessary bits about block properties, contained fields etc. + block.indexLastLineofBlockSection = indexLastLineofBlockSection; return block; } else { throw new IllegalStateException("Trying to build a block with errors or without parsing a line first"); @@ -241,7 +242,8 @@ public Block build() { /* ---- Actual Block Class starting here ---- */ private final Map properties = new EnumMap<>(Header.class); - private Optional> fields = Optional.empty(); + private List fields = Collections.emptyList(); + private int indexLastLineofBlockSection; private Block() {} @@ -254,4 +256,33 @@ public Optional get(Header column) { public String get(Header column, String defaultValue) { return this.properties.getOrDefault(column, defaultValue); } + + public int getIndexLastLineofBlockSection() { + return indexLastLineofBlockSection; + } + + public String getName() { + return this.properties.get(Header.NAME); + } + + /** + * Get fields for this metadata block. + * @return List of fields. May be empty, but never null. + */ + public List getFields() { + return fields; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Block)) return false; + Block block = (Block) o; + return this.getName().equals(block.getName()); + } + + @Override + public int hashCode() { + return Objects.hash(properties); + } } diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Configuration.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Configuration.java similarity index 79% rename from modules/solr-configset/src/main/scripts/cli/util/model/Configuration.java rename to modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Configuration.java index e57671586fb..5bce96576a8 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Configuration.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Configuration.java @@ -1,4 +1,4 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -9,11 +9,13 @@ public final class Configuration { private final String trigger; private final String column; private final Matcher rtrimMatcher; + private final Boolean allowDeepFieldNesting; public Configuration( String comment, String trigger, - String column + String column, + boolean allowDeepFieldNesting ) { notNullNotEmpty("Comment indicator", comment); this.comment = comment; @@ -25,10 +27,12 @@ public Configuration( this.column = column; this.rtrimMatcher = Pattern.compile("(" + this.column + ")+$").matcher(""); + + this.allowDeepFieldNesting = allowDeepFieldNesting; } public static Configuration defaultConfig() { - return new Configuration("%%", "#", "\t"); + return new Configuration("%%", "#", "\t", false); } private static void notNullNotEmpty(String optionName, String value) { @@ -56,4 +60,8 @@ public String rtrimColumns(String line) { public String trigger(String keyword) { return this.triggerIndicator() + keyword; } + + public boolean deepFieldNestingEnabled() { + return this.allowDeepFieldNesting; + } } diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ControlledVocabulary.java similarity index 99% rename from modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java rename to modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ControlledVocabulary.java index 48645dbf14e..487b2198079 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/ControlledVocabulary.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ControlledVocabulary.java @@ -1,4 +1,4 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; import java.util.Arrays; import java.util.Collections; diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Field.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Field.java new file mode 100644 index 00000000000..33e70a1fa3b --- /dev/null +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Field.java @@ -0,0 +1,480 @@ +package io.gdcc.solrteur.mdb.tsv; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.EnumMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Predicate; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +public final class Field { + + public static final String KEYWORD = "datasetField"; + + /** + * Currently, the spec says we need to comply with Solr and DDI rules. + * De-facto commonly used are camel-cased names. + * See also https://solr.apache.org/guide/8_11/defining-fields.html#field-properties + * + * This regex matches the strict Solr name spec plus adds "." as valid chars + * (we need it for comptability with astrophysics.tsv) + */ + public static final String NAME_PATTERN = "^(?=_[\\w.]+[^_]$|[^_][\\w.]+_?$)[a-zA-Z_][\\w.]+$"; + public static final String WHITESPACE_ONLY = "^\\s+$"; + + public static final Predicate matchBoolean = s -> "TRUE".equals(s) || "FALSE".equals(s); + + /** + * Programmatic variant of the spec of a #datasetField. List all the column headers and associated restrictions + * on the values of a column. + */ + public enum Header { + KEYWORD( + Field.KEYWORD, + String::isEmpty, + "must have no value (be empty)" + ), + + NAME( + "name", + Predicate.not(String::isBlank).and(Pattern.compile(Field.NAME_PATTERN).asMatchPredicate()), + "must not be blank and match regex pattern " + Field.NAME_PATTERN + ), + TITLE("title"), + DESCRIPTION("description"), + WATERMARK( + "watermark", + Predicate.not(Pattern.compile(WHITESPACE_ONLY).asMatchPredicate()), + "must not be whitespace only" + ), + + FIELDTYPE( + "fieldType", + Types.matchesTypes(), + "must be one of [" + String.join(", ", Types.getTypesList()) + "]" + ), + + DISPLAY_ORDER( + "displayOrder", + Pattern.compile("\\d+").asMatchPredicate(), + "must be a non-negative integer" + ), + DISPLAY_FORMAT( + "displayFormat", + Predicate.not(Pattern.compile(WHITESPACE_ONLY).asMatchPredicate()), + "must not be whitespace only" + ), + + ADVANCED_SEARCH_FIELD("advancedSearchField", matchBoolean, "must be 'TRUE' or 'FALSE"), + ALLOW_CONTROLLED_VOCABULARY("allowControlledVocabulary", matchBoolean, "must be 'TRUE' or 'FALSE"), + ALLOW_MULTIPLES("allowmultiples", matchBoolean, "must be 'TRUE' or 'FALSE"), + FACETABLE("facetable", matchBoolean, "must be 'TRUE' or 'FALSE"), + DISPLAY_ON_CREATE("displayoncreate", matchBoolean, "must be 'TRUE' or 'FALSE"), + REQUIRED("required", matchBoolean, "must be 'TRUE' or 'FALSE"), + + PARENT( + "parent", + Pattern.compile(Field.NAME_PATTERN).asMatchPredicate().or(String::isEmpty), + "must be either empty or match regex pattern " + Field.NAME_PATTERN + ), + METADATABLOCK_ID( + "metadatablock_id", + Predicate.not(String::isBlank).and(Pattern.compile(Block.NAME_PATTERN).asMatchPredicate()), + "must not be blank and match regex pattern " + Block.NAME_PATTERN + ), + TERM_URI( + "termURI", + s -> s.isEmpty() || Validator.isValidUrl(s), + "must be empty or a valid URL" + ); + + private final String value; + private final Predicate test; + private final String errorMessage; + + Header(final String value, final Predicate test, final String errorMessage) { + this.value = value; + this.test = test; + this.errorMessage = errorMessage; + } + + Header(final String value) { + this.value = value; + this.test = Predicate.not(String::isBlank); + this.errorMessage = "must not be empty or blank"; + } + + private static final Map valueMap; + static { + Map map = new ConcurrentHashMap<>(); + Arrays.stream(Header.values()).forEach(h -> map.put(h.toString(), h)); + valueMap = Collections.unmodifiableMap(map); + } + + /** + * Inverse lookup of a {@link Field.Header} from a {@link String}. + * + * @param value A textual string to look up. + * @return Matching {@link Field.Header} wrapped in {@link Optional} or an empty {@link Optional}. + */ + public static Optional
getByValue(String value) { + return Optional.ofNullable(valueMap.get(value)); + } + + /** + * Retrieve all column headers of field definitions as a spec-like list of strings, + * usable for validation and more. The list is ordered as the spec defines the order of the headers. + * + * @return List of the column headers, in order + */ + public static List getHeaders() { + return Arrays.stream(Header.values()).map(Header::toString).collect(Collectors.toUnmodifiableList()); + } + + /** + * Parse a {@link String} as a header of field definitions. Will validate the presence or absence + * of column headers as defined by the spec. This is not a lenient parser - headers need to comply with order + * from the spec. On the other hand, it is case-insensitive. + * + * @param line The textual line to parse for column headers + * @param config The parser configuration to be used + * @return A list of {@link Field.Header} build from the line of text + * @throws ParserException When presented column headers are missing, invalid or the complete line is just wrong. + * @throws IllegalStateException When a column header cannot be found within the enum {@link Field.Header}. + * This should never happen, as the validation would fail before! + */ + public static List
parseAndValidate(final String line, final Configuration config) throws ParserException { + List validatedColumns = Validator.validateHeaderLine(line, getHeaders(), config); + // the exception shall never happen, as we already validated the header! + return validatedColumns.stream() + .map(Header::getByValue) + .map(o -> o.orElseThrow(IllegalStateException::new)) + .collect(Collectors.toUnmodifiableList()); + } + + @Override + public String toString() { + return value; + } + + public boolean isValid(final String sut) { + return sut != null && test.test(sut); + } + + public String getErrorMessage() { + return errorMessage; + } + } + + public enum Types implements Predicate { + NONE("none"), + DATE("date"), + EMAIL("email"), + TEXT("text"), + TEXTBOX("textbox"), + URL("url"), + INT("int"), + FLOAT("float"); + + private final String name; + + Types(final String name) { + this.name = name; + } + + private static final Map valueMap; + static { + Map map = new ConcurrentHashMap<>(); + Arrays.stream(Types.values()).forEach(type -> map.put(type.toString(), type)); + valueMap = Collections.unmodifiableMap(map); + } + + @Override + public boolean test(String sut) { + // we demand correct case! + return this.toString().equals(sut); + } + + public static Predicate matchesTypes() { + Predicate test = NONE; + for (Types type : Types.values()) { + test = test.or(type); + } + return test; + } + + public static List getTypesList() { + return valueMap.keySet().stream().collect(Collectors.toUnmodifiableList()); + } + + @Override + public String toString() { + return this.name; + } + } + + public static final class FieldsBuilder { + private final Configuration config; + private final String containingBlockName; + private final List
header; + private final List fields; + + private int parsedLines = 0; + + /** + * Create a parser for field lines. As this is on the block it + * @param headerLine + * @param containingBlockName + * @param config + * @throws ParserException + */ + public FieldsBuilder(final String headerLine, String containingBlockName, final Configuration config) throws ParserException { + this.header = Header.parseAndValidate(headerLine, config); + this.containingBlockName = containingBlockName; + this.config = config; + this.fields = new ArrayList<>(); + } + + /** + * Analyse a line containing a concrete dataset field definition by parsing and validating it. + * + * This will fail: + * - when the line is null or blanks only + * - when the columns within the line do not match the length of the header + * - when the column values do not match the column type restrictions (as implied by the header) + * + * The exception might contain sub-exceptions, as the parser will do its best to keep going and find as many + * problems as possible to avoid unnecessary (pesky) re-iterations. + * + * Beware: + * 1. This method does not check for parent/child dependencies, as the spec does not say parent fields + * must be defined first. This needs to be done by {@link #build()}, as this is the indication all lines + * have been attempted to parse. + * + * @param line The dataset field definition line to analyse. + * @throws ParserException If the parsing fails (see description). + */ + public void parseAndValidateLine(final int lineIndex, final String line) throws ParserException { + // no null or blank lines for the parser. (blank lines can be skipped and not sent here by calling code) + if (line == null || line.isBlank()) { + throw new ParserException("must not be empty nor blanks only nor null.").withLineNumber(lineIndex); + } + + // save the field for further builder internal manipulation + // (if validation fails, the exception will prevent it) + this.fields.add(parseAndValidateColumns(lineIndex, line.split(config.columnSeparator()))); + } + + /** + * Parse and validate the columns (usually given by {@code parseAndValidateLine}). + * This is package private, becoming testable this way. + * + * @param lineParts + * @return A {@link Block} object (modifiable for builder internal use) + * @throws ParserException + */ + Field parseAndValidateColumns(final int lineIndex, final String[] lineParts) throws ParserException { + if (lineParts == null || lineParts.length > header.size()) { + throw new ParserException("Not matching length of dataset fields headline"); + } + + Field field = new Field(lineIndex); + ParserException parserException = new ParserException("Has validation errors:").withLineNumber(lineIndex); + + for (int i = 0; i < lineParts.length; i++) { + Field.Header column = header.get(i); + String value = lineParts[i]; + if( ! column.isValid(value)) { + parserException.addSubException( + "Invalid value '" + value + "' for column '" + column + "', " + column.getErrorMessage()); + } else { + field.set(column, value); + } + } + + // TODO: extend with the possibility to reference a different, but existing other block (also not recommended) + // check the metadata block reference matches the block this field is defined in + field.get(Header.METADATABLOCK_ID).ifPresent(id -> { + if (! this.containingBlockName.equals(id)) { + parserException.addSubException( + "Metadata block reference '" + id + + "' does not match parsed containing block '" + this.containingBlockName + "'"); + } + }); + + if (parserException.hasSubExceptions()) { + throw parserException; + } else { + return field; + } + } + + // TODO: extend with: + // 2) unique numbers in ordering (attention: compound fields!) + // 4) warnings for facetable fields with suboptimal field types + public List build() throws ParserException { + // Check if all fields are unique within this block + List duplicates = fields.stream() + .collect(Collectors.groupingBy(Field::getName)) + .entrySet() + .stream() + .filter(entry -> entry.getValue().size() > 1) + .map(Map.Entry::getKey) + .collect(Collectors.toUnmodifiableList()); + + // This is critical - we cannot build relations when duplicates are present. Step away now. + if (!duplicates.isEmpty()) { + throw new ParserException("Found duplicate field definitions for '" + String.join("', '", duplicates) + "'"); + } + + // Create parent / child relations for compound fields and validate + // 1. Get all fields that claim to have a parent + List children = fields.stream() + .filter(field -> !field.get(Header.PARENT, "").isEmpty()) + .collect(Collectors.toUnmodifiableList()); + + ParserException parentException = new ParserException("Compound field errors:"); + // 2. Search and associate the parent field to these children + for (Field child : children) { + // Always non-null as we filtered for this before, so will never be "" (but we avoid the Optional) + String parentName = child.get(Header.PARENT, ""); + + Optional parentCandidate = fields.stream() + .filter(field -> field.getName().equals(parentName)) + .findFirst(); + + if (parentCandidate.isEmpty()) { + parentException.addSubException( + new ParserException("'" + child.getName() + "' misses its parent '" + parentName + "'") + .withLineNumber(child.lineIndex)); + } else { + // TODO: when this parsing is extended to allow fields belonging to other blocks, this would + // need extension to verify the parent candidate is within the same metadata block + Field parent = parentCandidate.get(); + + // Associate children and parents + child.parent = parent; + parent.children.add(child); + } + } + + // 3. No cyclic dependencies / deep nesting is optional + // First, fetch fields that are children and parents at the same time: + List parentChilds = fields.stream() + .filter(field -> !field.children.isEmpty() && field.parent != null) + .collect(Collectors.toUnmodifiableList()); + + // When deep nesting is disabled, this list must be empty (only 1 level of nesting allowed means + // there cannot be any parents that are children at the same time) + if (!config.deepFieldNestingEnabled() && !parentChilds.isEmpty()) { + parentChilds.forEach(field -> + parentException.addSubException( + new ParserException("'" + field.getName() + "' is not allowed to be parent and child at once") + .withLineNumber(field.lineIndex))); + // With deep nesting enabled, cyclic dependencies might happen. Need to recurse over these elements. + } else if (config.deepFieldNestingEnabled() && !parentChilds.isEmpty()) { + parentChilds.forEach(field -> { + if (hasCyclicDependency(field, field.parent)) { + parentException.addSubException( + new ParserException("'" + field.getName() + "' is part of a cyclic dependency") + .withLineNumber(field.lineIndex)); + } + }); + } + + // 4. Ensure all compound parents are of type "none" + List parents = fields.stream() + .filter(field -> !field.children.isEmpty()) + .collect(Collectors.toUnmodifiableList()); + + parents.forEach(field -> { + // The first check shall never be true, but just in case... + if (field.get(Header.FIELDTYPE).isEmpty() || !Types.NONE.test(field.get(Header.FIELDTYPE).get())) { + parentException.addSubException( + new ParserException("'" + field.getName() + "' is a parent but does not have fieldType 'none'") + .withLineNumber(field.lineIndex)); + } + }); + + // Check display order uniqueness + // 1. Check all top level fields first + /* + List topLevel = fields.stream() + // get all fields that have no parent, which means top level + .filter(field -> field.parent == null) + .collect(Collectors.groupingBy(field -> field.get(Header.DISPLAY_ORDER, ""))) + .entrySet() + .stream() + .filter(entry -> entry.getValue().size() > 1) + .map(Map.Entry::getKey) + .collect(Collectors.toUnmodifiableList()); + */ + + // Now either die or return fields + if (parentException.hasSubExceptions()) { + throw parentException; + } + return List.copyOf(fields); + } + + private boolean hasCyclicDependency(final Field root, final Field parent) { + // Found ourselves - stop recursion here. + if (parent == root) { + return true; + } + // Simple recursive search. Should be cheap enough for our expected workload. + if (parent.parent != null) { + return hasCyclicDependency(root, parent.parent); + } else { + return false; + } + } + } + + /* ---- Actual Field Class starting here ---- */ + + private Field parent; + private List children = new ArrayList<>(); + private Map properties = new EnumMap<>(Header.class); + private int lineIndex; + + private Field(int lineIndex) { + this.lineIndex = lineIndex; + } + + private void set(final Header column, final String value) { + this.properties.put(column, value); + } + public Optional get(final Header column) { + return Optional.ofNullable(properties.get(column)); + } + public String get(final Header column, final String defaultValue) { + return properties.getOrDefault(column, defaultValue); + } + + public String getName() { + // Will always be non-null after creating the field with the builder + return properties.get(Header.NAME); + } + + List controlledVocabularyValues = List.of(); + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Field)) return false; + Field field = (Field) o; + return field.getName().equals(this.getName()); + } + + @Override + public int hashCode() { + return Objects.hash(properties); + } +} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ParserError.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParserError.java similarity index 91% rename from modules/solr-configset/src/main/scripts/cli/util/model/ParserError.java rename to modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParserError.java index d3ec70fc3dd..fd120ce4f1e 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/ParserError.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParserError.java @@ -1,4 +1,4 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; public final class ParserError { String message; diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParserException.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParserException.java new file mode 100644 index 00000000000..dca7f86b44a --- /dev/null +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParserException.java @@ -0,0 +1,39 @@ +package io.gdcc.solrteur.mdb.tsv; + +import java.util.ArrayList; +import java.util.List; + +public final class ParserException extends Throwable { + + private final List subExceptions = new ArrayList<>(0); + private int lineNumber = -1; + + public ParserException(String message) { + super(message); + } + + public boolean hasSubExceptions() { + return !subExceptions.isEmpty(); + } + + public void addSubException(String message) { + this.subExceptions.add(new ParserException(message)); + } + + public void addSubException(ParserException ex) { + this.subExceptions.add(ex); + } + + public List getSubExceptions() { + return List.copyOf(subExceptions); + } + + public String getLineNumber() { + return lineNumber > 0 ? ":"+lineNumber : ""; + } + + public ParserException withLineNumber(int lineIndex) { + this.lineNumber = lineIndex + 1; + return this; + } +} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParsingState.java similarity index 77% rename from modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java rename to modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParsingState.java index a89c4b31da7..731cbc939c5 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/ParsingState.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/ParsingState.java @@ -1,4 +1,4 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; public enum ParsingState { Vocabularies(ControlledVocabulary.KEYWORD), @@ -29,6 +29,15 @@ public boolean isAllowedFinalState() { return this == Fields || this == Vocabularies; } + /** + * Return the next state if the given line is triggering the change correctly. + * Wrong header lines will trigger exceptions. + * + * @param headerLine The line to analyse for switching to the next parsing state + * @param config The parser configuration + * @return The new state (will throw if line not valid) + * @throws ParserException When the given line wasn't the expected one to transition the state + */ public ParsingState transitionState(String headerLine, Configuration config) throws ParserException { // if not null, not starting the same state again (no loops allowed) and starting the correct next state, return the next state if(headerLine != null && diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Validator.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Validator.java similarity index 98% rename from modules/solr-configset/src/main/scripts/cli/util/model/Validator.java rename to modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Validator.java index d419ebcda95..2c950f1fd91 100644 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Validator.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Validator.java @@ -1,4 +1,4 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; import java.net.MalformedURLException; import java.net.URISyntaxException; @@ -48,7 +48,7 @@ static List validateHeaderLine(final String headerLine, final List validOrderedHeaders, final Configuration config) throws ParserException { // start a parenting parser exception to be filled with errors as subexceptions - ParserException ex = new ParserException("contains an invalid column header"); + ParserException ex = new ParserException("Invalid headline:"); if (headerLine == null || headerLine.isBlank()) { ex.addSubException("Header may not be null, empty or whitespace only"); diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/package-info.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/package-info.java new file mode 100644 index 00000000000..ef7bd55f222 --- /dev/null +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/package-info.java @@ -0,0 +1 @@ +package io.gdcc.solrteur; \ No newline at end of file diff --git a/modules/solr-configset/src/main/scripts/cli/solrteur.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/solrteur.java similarity index 79% rename from modules/solr-configset/src/main/scripts/cli/solrteur.java rename to modules/solr-configset/src/main/java/io/gdcc/solrteur/solrteur.java index c42f78e858b..1a8fa5ae69b 100644 --- a/modules/solr-configset/src/main/scripts/cli/solrteur.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/solrteur.java @@ -1,17 +1,20 @@ ///usr/bin/env jbang "$0" "$@" ; exit $? // //SOURCES cmd/*.java +//SOURCES mdb/*.java +//SOURCES mdb/**/*.java // //DEPS info.picocli:picocli:4.6.3 -package cli; +package io.gdcc.solrteur; -import cli.cmd.ExtractConfigSet; +import io.gdcc.solrteur.cmd.CompileSchema; +import io.gdcc.solrteur.cmd.ExtractConfigSet; import picocli.CommandLine; import picocli.CommandLine.Command; import picocli.CommandLine.Option; -import cli.cmd.CompileSolrConfig; +import io.gdcc.solrteur.cmd.CompileSolrConfig; import java.nio.file.Path; @@ -28,7 +31,8 @@ description = "Execute different tasks around Dataverse and Solr", subcommands = { ExtractConfigSet.class, - CompileSolrConfig.class + CompileSolrConfig.class, + CompileSchema.class }, synopsisSubcommandLabel = "COMMAND") public class solrteur { @@ -80,9 +84,8 @@ public static final class Logger { static void log(String message) { System.out.println(message); } - static void log(AbortScriptException ex) { - System.out.println(ex.getMessage()); - ex.getCause().printStackTrace(); + static void logError(String message) { + System.err.println(message); } public static void info(String message) { @@ -92,9 +95,19 @@ public static void info(String message) { } public static void info(AbortScriptException ex) { if (!quiet) { - log(ex); + log(ex.getMessage()); + log(ex.getCause().getMessage()); } } + + public static void warn(String message) { + logError(message); + } + + public static void warn(AbortScriptException ex) { + logError(ex.getMessage()); + logError(ex.getCause().getMessage()); + } } public static void main(String... args) { diff --git a/modules/solr-configset/src/main/scripts/cli/cmd/package-info.java b/modules/solr-configset/src/main/scripts/cli/cmd/package-info.java deleted file mode 100644 index 0e5bbb76c50..00000000000 --- a/modules/solr-configset/src/main/scripts/cli/cmd/package-info.java +++ /dev/null @@ -1 +0,0 @@ -package cli.cmd; \ No newline at end of file diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java b/modules/solr-configset/src/main/scripts/cli/util/model/Field.java deleted file mode 100644 index 6ca9a3b5302..00000000000 --- a/modules/solr-configset/src/main/scripts/cli/util/model/Field.java +++ /dev/null @@ -1,222 +0,0 @@ -package cli.util.model; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Predicate; -import java.util.regex.Pattern; -import java.util.stream.Collectors; - -public final class Field { - - public static final String KEYWORD = "datasetField"; - - /** - * Currently, the spec says we need to comply with Solr and DDI rules. - * De-facto commonly used are camel-cased names. - * See also https://solr.apache.org/guide/8_11/defining-fields.html#field-properties - * - * This regex matches the strict Solr name spec plus adds "." as valid chars - * (we need it for comptability with astrophysics.tsv) - */ - public static final String NAME_PATTERN = "^(?=_[\\w.]+[^_]$|[^_][\\w.]+_?$)[a-zA-Z_][\\w.]+$"; - public static final String WHITESPACE_ONLY = "^\\s+$"; - - public static final Predicate matchBoolean = s -> "TRUE".equals(s) || "FALSE".equals(s); - - /** - * Programmatic variant of the spec of a #datasetField. List all the column headers and associated restrictions - * on the values of a column. - */ - public enum Header { - KEYWORD( - Field.KEYWORD, - String::isEmpty, - "must have no value (be empty)" - ), - - NAME( - "name", - Predicate.not(String::isBlank).and(Pattern.compile(Field.NAME_PATTERN).asMatchPredicate()), - "must not be blank and match regex pattern " + Field.NAME_PATTERN - ), - TITLE("title"), - DESCRIPTION("description"), - WATERMARK( - "watermark", - Predicate.not(Pattern.compile(WHITESPACE_ONLY).asMatchPredicate()), - "must not be whitespace only" - ), - - FIELDTYPE( - "fieldType", - Types.matchesTypes(), - "must be one of [" + String.join(", ", Types.getTypesList()) + "]" - ), - - DISPLAY_ORDER( - "displayOrder", - Pattern.compile("\\d+").asMatchPredicate(), - "must be a non-negative integer" - ), - DISPLAY_FORMAT( - "displayFormat", - Predicate.not(Pattern.compile(WHITESPACE_ONLY).asMatchPredicate()), - "must not be whitespace only" - ), - - ADVANCED_SEARCH_FIELD("advancedSearchField", matchBoolean, "must be 'TRUE' or 'FALSE"), - ALLOW_CONTROLLED_VOCABULARY("allowControlledVocabulary", matchBoolean, "must be 'TRUE' or 'FALSE"), - ALLOW_MULTIPLES("allowmultiples", matchBoolean, "must be 'TRUE' or 'FALSE"), - FACETABLE("facetable", matchBoolean, "must be 'TRUE' or 'FALSE"), - DISPLAY_ON_CREATE("displayoncreate", matchBoolean, "must be 'TRUE' or 'FALSE"), - REQUIRED("required", matchBoolean, "must be 'TRUE' or 'FALSE"), - - PARENT( - "parent", - Pattern.compile(Field.NAME_PATTERN).asMatchPredicate().or(String::isEmpty), - "must be either empty or match regex pattern " + Field.NAME_PATTERN - ), - METADATABLOCK_ID( - "metadatablock_id", - Predicate.not(String::isBlank).and(Pattern.compile(Block.NAME_PATTERN).asMatchPredicate()), - "must not be blank and match regex pattern " + Block.NAME_PATTERN - ), - TERM_URI( - "termURI", - s -> s.isEmpty() || Validator.isValidUrl(s), - "must be empty or a valid URL" - ); - - private final String value; - private final Predicate test; - private final String errorMessage; - - Header(final String value, final Predicate test, final String errorMessage) { - this.value = value; - this.test = test; - this.errorMessage = errorMessage; - } - - Header(final String value) { - this.value = value; - this.test = Predicate.not(String::isBlank); - this.errorMessage = "must not be empty or blank"; - } - - private static final Map valueMap; - static { - Map map = new ConcurrentHashMap<>(); - Arrays.stream(Header.values()).forEach(h -> map.put(h.toString(), h)); - valueMap = Collections.unmodifiableMap(map); - } - - /** - * Inverse lookup of a {@link Field.Header} from a {@link String}. - * - * @param value A textual string to look up. - * @return Matching {@link Field.Header} wrapped in {@link Optional} or an empty {@link Optional}. - */ - public static Optional
getByValue(String value) { - return Optional.ofNullable(valueMap.get(value)); - } - - /** - * Retrieve all column headers of field definitions as a spec-like list of strings, - * usable for validation and more. The list is ordered as the spec defines the order of the headers. - * - * @return List of the column headers, in order - */ - public static List getHeaders() { - return List.copyOf(valueMap.keySet()); - } - - /** - * Parse a {@link String} as a header of field definitions. Will validate the presence or absence - * of column headers as defined by the spec. This is not a lenient parser - headers need to comply with order - * from the spec. On the other hand, it is case-insensitive. - * - * @param line The textual line to parse for column headers - * @param config The parser configuration to be used - * @return A list of {@link Field.Header} build from the line of text - * @throws ParserException When presented column headers are missing, invalid or the complete line is just wrong. - * @throws IllegalStateException When a column header cannot be found within the enum {@link Field.Header}. - * This should never happen, as the validation would fail before! - */ - public static List
parseAndValidate(final String line, final Configuration config) throws ParserException { - List validatedColumns = Validator.validateHeaderLine(line, getHeaders(), config); - // the exception shall never happen, as we already validated the header! - return validatedColumns.stream() - .map(Header::getByValue) - .map(o -> o.orElseThrow(IllegalStateException::new)) - .collect(Collectors.toUnmodifiableList()); - } - - @Override - public String toString() { - return value; - } - - public boolean isValid(final String sut) { - return sut != null && test.test(sut); - } - - public String getErrorMessage() { - return errorMessage; - } - } - - public enum Types implements Predicate { - NONE("none"), - DATE("date"), - EMAIL("email"), - TEXT("text"), - TEXTBOX("textbox"), - URL("url"), - INT("int"), - FLOAT("float"); - - private final String name; - - Types(final String name) { - this.name = name; - } - - private static final Map valueMap; - static { - Map map = new ConcurrentHashMap<>(); - Arrays.stream(Types.values()).forEach(type -> map.put(type.toString(), type)); - valueMap = Collections.unmodifiableMap(map); - } - - @Override - public boolean test(String sut) { - // we demand correct case! - return this.toString().equals(sut); - } - - public static Predicate matchesTypes() { - Predicate test = NONE; - for (Types type : Types.values()) { - test = test.or(type); - } - return test; - } - - public static List getTypesList() { - return valueMap.keySet().stream().collect(Collectors.toUnmodifiableList()); - } - - @Override - public String toString() { - return this.name; - } - } - - - Optional> controlledVocabularyValues = Optional.empty(); -} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/ParserException.java b/modules/solr-configset/src/main/scripts/cli/util/model/ParserException.java deleted file mode 100644 index ec1390cc66e..00000000000 --- a/modules/solr-configset/src/main/scripts/cli/util/model/ParserException.java +++ /dev/null @@ -1,25 +0,0 @@ -package cli.util.model; - -import java.util.ArrayList; -import java.util.List; - -public final class ParserException extends Throwable { - - final List subExceptions = new ArrayList<>(0); - - public ParserException(String message) { - super(message); - } - - public boolean hasSubExceptions() { - return subExceptions.size() > 0; - } - - public void addSubException(String message) { - this.subExceptions.add(new ParserException(message)); - } - - public List getSubExceptions() { - return List.copyOf(subExceptions); - } -} diff --git a/modules/solr-configset/src/main/scripts/cli/util/model/package-info.java b/modules/solr-configset/src/main/scripts/cli/util/model/package-info.java deleted file mode 100644 index 6fd97a0fe49..00000000000 --- a/modules/solr-configset/src/main/scripts/cli/util/model/package-info.java +++ /dev/null @@ -1 +0,0 @@ -package cli.util.model; \ No newline at end of file diff --git a/modules/solr-configset/src/main/scripts/cli/util/package-info.java b/modules/solr-configset/src/main/scripts/cli/util/package-info.java deleted file mode 100644 index 633e3028144..00000000000 --- a/modules/solr-configset/src/main/scripts/cli/util/package-info.java +++ /dev/null @@ -1 +0,0 @@ -package cli.util; \ No newline at end of file diff --git a/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java b/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java deleted file mode 100644 index 021e406d2ab..00000000000 --- a/modules/solr-configset/src/test/java/cli/util/model/FieldTest.java +++ /dev/null @@ -1,97 +0,0 @@ -package cli.util.model; - -import org.junit.jupiter.api.Nested; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.EmptySource; -import org.junit.jupiter.params.provider.NullAndEmptySource; -import org.junit.jupiter.params.provider.NullSource; -import org.junit.jupiter.params.provider.ValueSource; - -import java.util.function.Predicate; -import java.util.logging.Logger; - -import static org.junit.jupiter.api.Assertions.*; - -class FieldTest { - - private static final Logger logger = Logger.getLogger(BlockTest.class.getCanonicalName()); - - @Nested - class TypesTest { - Predicate allowedTypes = Field.Types.matchesTypes(); - - @ParameterizedTest - @NullAndEmptySource - @ValueSource(strings = {"foobar", "hello_hello", "NONE", "DATE"}) - void failing(String subject) { - assertFalse(allowedTypes.test(subject)); - } - - @ParameterizedTest - @ValueSource(strings = {"none", "text", "textbox", "date", "email", "int", "float"}) - void succeeding(String subject) { - assertTrue(allowedTypes.test(subject)); - } - } - - @Nested - class HeaderTest { - @ParameterizedTest - @NullAndEmptySource - @ValueSource(strings = {" ", "\t", "_foobar_", "_foo_bar_"}) - void invalidNames(String subject) { - assertFalse(Field.Header.NAME.isValid(subject)); - } - - @ParameterizedTest - @ValueSource(strings = {"foobar_", "foo_bar_", "_foobar", "_foo_bar", "foobar", "foobar1234", "foo_bar_1234"}) - void validNames(String subject) { - assertTrue(Field.Header.NAME.isValid(subject)); - assertTrue(Field.Header.PARENT.isValid(subject)); - } - - @ParameterizedTest - @EmptySource - void validParentName(String subject) { - assertTrue(Field.Header.PARENT.isValid(subject)); - } - - @ParameterizedTest - @NullSource - @ValueSource(strings = {" ", "\t"}) - void invalidEmptyOrText(String subject) { - assertFalse(Field.Header.WATERMARK.isValid(subject)); - assertFalse(Field.Header.DISPLAY_FORMAT.isValid(subject)); - } - - @ParameterizedTest - @ValueSource(strings = { "", "foobar", "My name is Hase, I know about nothing."}) - void validEmptyOrText(String subject) { - assertTrue(Field.Header.WATERMARK.isValid(subject)); - assertTrue(Field.Header.DISPLAY_FORMAT.isValid(subject)); - } - - @ParameterizedTest - @NullAndEmptySource - @ValueSource(strings = { "true", "false", "0", "1", "foobar"}) - void invalidBoolean(String subject) { - assertFalse(Field.Header.ADVANCED_SEARCH_FIELD.isValid(subject)); - assertFalse(Field.Header.ALLOW_CONTROLLED_VOCABULARY.isValid(subject)); - assertFalse(Field.Header.ALLOW_MULTIPLES.isValid(subject)); - assertFalse(Field.Header.FACETABLE.isValid(subject)); - assertFalse(Field.Header.DISPLAY_ON_CREATE.isValid(subject)); - assertFalse(Field.Header.REQUIRED.isValid(subject)); - } - - @ParameterizedTest - @ValueSource(strings = { "TRUE", "FALSE" }) - void validBoolean(String subject) { - assertTrue(Field.Header.ADVANCED_SEARCH_FIELD.isValid(subject)); - assertTrue(Field.Header.ALLOW_CONTROLLED_VOCABULARY.isValid(subject)); - assertTrue(Field.Header.ALLOW_MULTIPLES.isValid(subject)); - assertTrue(Field.Header.FACETABLE.isValid(subject)); - assertTrue(Field.Header.DISPLAY_ON_CREATE.isValid(subject)); - assertTrue(Field.Header.REQUIRED.isValid(subject)); - } - } -} \ No newline at end of file diff --git a/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/BlockTest.java similarity index 99% rename from modules/solr-configset/src/test/java/cli/util/model/BlockTest.java rename to modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/BlockTest.java index aa6f9e46342..9fc5008eeff 100644 --- a/modules/solr-configset/src/test/java/cli/util/model/BlockTest.java +++ b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/BlockTest.java @@ -1,4 +1,4 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; diff --git a/modules/solr-configset/src/test/java/cli/util/model/ConfigurationTest.java b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/ConfigurationTest.java similarity index 91% rename from modules/solr-configset/src/test/java/cli/util/model/ConfigurationTest.java rename to modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/ConfigurationTest.java index 322f8affa02..5b632955bac 100644 --- a/modules/solr-configset/src/test/java/cli/util/model/ConfigurationTest.java +++ b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/ConfigurationTest.java @@ -1,5 +1,6 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; +import io.gdcc.solrteur.mdb.tsv.Configuration; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; diff --git a/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/FieldTest.java b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/FieldTest.java new file mode 100644 index 00000000000..f26a3f3ff58 --- /dev/null +++ b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/FieldTest.java @@ -0,0 +1,177 @@ +package io.gdcc.solrteur.mdb.tsv; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvFileSource; +import org.junit.jupiter.params.provider.EmptySource; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.NullSource; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.List; +import java.util.function.Predicate; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.*; + +class FieldTest { + + private static final Logger logger = Logger.getLogger(FieldTest.class.getCanonicalName()); + + static final Configuration config = Configuration.defaultConfig(); + static final String validHeaderLine = "#datasetField\tname\ttitle\tdescription\twatermark\tfieldType" + + "\tdisplayOrder\tdisplayFormat\tadvancedSearchField\tallowControlledVocabulary\tallowmultiples\tfacetable" + + "\tdisplayoncreate\trequired\tparent\tmetadatablock_id\ttermURI"; + static final String validContainingBlockName = "citation"; + static final String validFieldDef = "\ttitle\tTitle\tThe main title of the Dataset\t\ttext" + + "\t0\t\tTRUE\tFALSE\tFALSE\tFALSE\tTRUE\tTRUE\t\tcitation\thttp://purl.org/dc/terms/title"; + + @Nested + class TypesTest { + Predicate allowedTypes = Field.Types.matchesTypes(); + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {"foobar", "hello_hello", "NONE", "DATE"}) + void failing(String subject) { + assertFalse(allowedTypes.test(subject)); + } + + @ParameterizedTest + @ValueSource(strings = {"none", "text", "textbox", "date", "email", "int", "float"}) + void succeeding(String subject) { + assertTrue(allowedTypes.test(subject)); + } + } + + @Nested + class HeaderFieldValueValidationTest { + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = {" ", "\t", "_foobar_", "_foo_bar_"}) + void invalidNames(String subject) { + assertFalse(Field.Header.NAME.isValid(subject)); + } + + @ParameterizedTest + @ValueSource(strings = {"foobar_", "foo_bar_", "_foobar", "_foo_bar", "foobar", "foobar1234", "foo_bar_1234"}) + void validNames(String subject) { + assertTrue(Field.Header.NAME.isValid(subject)); + assertTrue(Field.Header.PARENT.isValid(subject)); + } + + @ParameterizedTest + @EmptySource + void validParentName(String subject) { + assertTrue(Field.Header.PARENT.isValid(subject)); + } + + @ParameterizedTest + @NullSource + @ValueSource(strings = {" ", "\t"}) + void invalidEmptyOrText(String subject) { + assertFalse(Field.Header.WATERMARK.isValid(subject)); + assertFalse(Field.Header.DISPLAY_FORMAT.isValid(subject)); + } + + @ParameterizedTest + @ValueSource(strings = { "", "foobar", "My name is Hase, I know about nothing."}) + void validEmptyOrText(String subject) { + assertTrue(Field.Header.WATERMARK.isValid(subject)); + assertTrue(Field.Header.DISPLAY_FORMAT.isValid(subject)); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = { "true", "false", "0", "1", "foobar"}) + void invalidBoolean(String subject) { + assertFalse(Field.Header.ADVANCED_SEARCH_FIELD.isValid(subject)); + assertFalse(Field.Header.ALLOW_CONTROLLED_VOCABULARY.isValid(subject)); + assertFalse(Field.Header.ALLOW_MULTIPLES.isValid(subject)); + assertFalse(Field.Header.FACETABLE.isValid(subject)); + assertFalse(Field.Header.DISPLAY_ON_CREATE.isValid(subject)); + assertFalse(Field.Header.REQUIRED.isValid(subject)); + } + + @ParameterizedTest + @ValueSource(strings = { "TRUE", "FALSE" }) + void validBoolean(String subject) { + assertTrue(Field.Header.ADVANCED_SEARCH_FIELD.isValid(subject)); + assertTrue(Field.Header.ALLOW_CONTROLLED_VOCABULARY.isValid(subject)); + assertTrue(Field.Header.ALLOW_MULTIPLES.isValid(subject)); + assertTrue(Field.Header.FACETABLE.isValid(subject)); + assertTrue(Field.Header.DISPLAY_ON_CREATE.isValid(subject)); + assertTrue(Field.Header.REQUIRED.isValid(subject)); + } + } + + @Nested + class HeaderLineTest { + @ParameterizedTest + @ValueSource(strings = { + validHeaderLine, + "#datasetfield\tName\tTITLE\tdescription\tWAtermark\tfieldType\tdisplayOrder\tdisplayFormat" + + "\tadvancedSearchField\tallowControlledVocabulary\tallowmultiples\tfacetable" + + "\tdisplayOnCreate\trequired\tparent\tmetadataBLOCK_ID\ttermUri" + }) + void successfulParseAndValidateHeaderLine(String headerLine) throws ParserException { + List headers = Field.Header.parseAndValidate(headerLine, config); + assertFalse(headers.isEmpty()); + assertEquals(List.of(Field.Header.values()), headers); + } + + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = { + "hello", + "datasetField", + "#datasetField test", + "#datasetField\tname\ttitle\tdescription\twatermark\tfieldType", + "#datasetField\tname\ttitle\tdescription\twatermark\tfieldType\t\tdisplayoncreate\trequired\tparent\tmetadatablock_id\ttermURI" + }) + void failingParseAndValidateHeaderLine(String headerLine) throws ParserException { + ParserException exception = assertThrows(ParserException.class, () -> Field.Header.parseAndValidate(headerLine, config)); + assertTrue(exception.hasSubExceptions()); + logger.log(Level.FINE, + exception.getSubExceptions().stream().map(Throwable::getMessage).collect(Collectors.joining("\n")) + ); + } + } + + @Nested + class ParseLineTest { + Field.FieldsBuilder builder; + + @BeforeEach + void setUp() throws ParserException { + builder = new Field.FieldsBuilder(validHeaderLine, validContainingBlockName, config); + } + + @ParameterizedTest + @NullAndEmptySource + void failingParseLine(String line) throws ParserException { + ParserException exception = assertThrows(ParserException.class, () -> builder.parseAndValidateLine(0, line)); + //assertFalse(builder.hasSucceeded()); + } + + @ParameterizedTest + @ValueSource(strings = {validFieldDef}) + @CsvFileSource(files = "/testlines/valid_datasetfields.csv") + void succeedingParseLine(String line) throws ParserException { + builder.parseAndValidateLine(0, line); + //assertTrue(builder.hasSucceeded()); + } + + @Test + void failingDoubleAdditionAttempt() throws ParserException { + builder.parseAndValidateLine(0, validFieldDef); + //assertTrue(builder.hasSucceeded()); + ParserException exception = assertThrows(ParserException.class, () -> builder.parseAndValidateLine(0, validFieldDef)); + //assertFalse(builder.hasSucceeded()); + } + } +} \ No newline at end of file diff --git a/modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/ParsingStateTest.java similarity index 91% rename from modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java rename to modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/ParsingStateTest.java index a50b927c16b..3722eb78289 100644 --- a/modules/solr-configset/src/test/java/cli/util/model/ParsingStateTest.java +++ b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/ParsingStateTest.java @@ -1,5 +1,11 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; +import io.gdcc.solrteur.mdb.tsv.Configuration; +import io.gdcc.solrteur.mdb.tsv.Block; +import io.gdcc.solrteur.mdb.tsv.ControlledVocabulary; +import io.gdcc.solrteur.mdb.tsv.Field; +import io.gdcc.solrteur.mdb.tsv.ParserException; +import io.gdcc.solrteur.mdb.tsv.ParsingState; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; diff --git a/modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/ValidatorTest.java similarity index 95% rename from modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java rename to modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/ValidatorTest.java index 990db6e7ec1..5462c41130c 100644 --- a/modules/solr-configset/src/test/java/cli/util/model/ValidatorTest.java +++ b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/ValidatorTest.java @@ -1,5 +1,6 @@ -package cli.util.model; +package io.gdcc.solrteur.mdb.tsv; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Nested; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; @@ -37,7 +38,7 @@ class UtilsTest { "true,https://host/hello", }) void urlValidation(boolean expected, String sut) { - assertEquals(expected, Validator.isValidUrl(sut)); + Assertions.assertEquals(expected, Validator.isValidUrl(sut)); } } From ae4c7f1c0c1850c24d53b731abf621feaabff456 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 17 Feb 2023 00:15:25 +0100 Subject: [PATCH 53/56] feat(solrteur): continue improving MDB TSV field parsing --- .../io/gdcc/solrteur/cmd/CompileSchema.java | 68 ++++++++++++----- .../java/io/gdcc/solrteur/mdb/tsv/Field.java | 74 ++++++++++++------- 2 files changed, 98 insertions(+), 44 deletions(-) diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSchema.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSchema.java index c9db66f2bc0..c63232b1edc 100644 --- a/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSchema.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/cmd/CompileSchema.java @@ -126,31 +126,65 @@ private void getFieldsFromBlocks() throws solrteur.AbortScriptException { solrteur.Logger.warn(new solrteur.AbortScriptException("Could not read "+path, e)); } } + + // Abort here if there were errors + if (hadErrors) { + throw new solrteur.AbortScriptException("Experienced parsing errors, fix your block definitions first to continue", null); + } // If all blocks could be read and are valid, let's extract the fields Map> blockFieldsMap = new HashMap<>(); - if (!hadErrors) { - Set blocks = blockPathMap.keySet(); - //blocks.forEach(b -> System.out.println(b.getName())); + Set blocks = blockPathMap.keySet(); + + for (Map.Entry mdb : blockPathMap.entrySet()) { + Block block = mdb.getKey(); + Path path = mdb.getValue(); + + try { + List lines = Files.readAllLines(path, StandardCharsets.UTF_8); + // First store all retrieved fields, we check on uniqueness later + blockFieldsMap.put(block, reader.retrieveFields(lines, blocks)); + } catch (ParserException e) { + // Log a warning but continue parsing with next file to get done as much as possible. + hadErrors = true; + logErrors(path, e); + } catch (IOException e) { + // Log a warning but continue parsing with next file to get done as much as possible. + hadErrors = true; + solrteur.Logger.warn(new solrteur.AbortScriptException("Could not read "+path, e)); + } + } + + // Abort here if there were errors + if (hadErrors) { + throw new solrteur.AbortScriptException("Experienced parsing errors, fix your field definitions first to continue", null); + } + + // We need to check uniqueness of fields across blocks + Map fieldBlockMap = new HashMap<>(); + for (Map.Entry> entry : blockFieldsMap.entrySet()) { + Block block = entry.getKey(); + System.out.println("BLOCK: " + block.getName()); - for (Map.Entry mdb : blockPathMap.entrySet()) { - Block block = mdb.getKey(); - Path path = mdb.getValue(); - - try { - List lines = Files.readAllLines(path, StandardCharsets.UTF_8); - // First store all retrieved fields, we check on uniqueness later - blockFieldsMap.put(block, reader.retrieveFields(lines, blocks)); - } catch (ParserException e) { - // Log a warning but continue parsing with next file to get done as much as possible. - logErrors(path, e); - } catch (IOException e) { - // Log a warning but continue parsing with next file to get done as much as possible. - solrteur.Logger.warn(new solrteur.AbortScriptException("Could not read "+path, e)); + for (Field field : entry.getValue()) { + System.out.println(" FIELD: " + field.getName()); + + if (fieldBlockMap.containsKey(field)) { + hadErrors = true; + solrteur.Logger.warn("Duplicate field '" + field.getName() + "' in block '" + block.getName() + + "' from '" + blockPathMap.get(block) + "': already defined by block '" + + fieldBlockMap.get(field).getName() + "' from '" + blockPathMap.get(fieldBlockMap.get(field)) + "'"); + } else { + fieldBlockMap.put(field, block); } } } + // Abort here if there were errors + if (hadErrors) { + throw new solrteur.AbortScriptException("Stopping analysis", null); + } + } private void injectFields() {} private void injectCopyFields() {} diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Field.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Field.java index 33e70a1fa3b..d6eff8a25ef 100644 --- a/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Field.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Field.java @@ -29,6 +29,7 @@ public final class Field { public static final String WHITESPACE_ONLY = "^\\s+$"; public static final Predicate matchBoolean = s -> "TRUE".equals(s) || "FALSE".equals(s); + public static final Predicate matchParentDisplayFormat = Pattern.compile("^[;,:]?$").asMatchPredicate(); /** * Programmatic variant of the spec of a #datasetField. List all the column headers and associated restrictions @@ -315,9 +316,7 @@ Field parseAndValidateColumns(final int lineIndex, final String[] lineParts) thr } } - // TODO: extend with: - // 2) unique numbers in ordering (attention: compound fields!) - // 4) warnings for facetable fields with suboptimal field types + // TODO: extract distinct checks and move to separate methods to reduce complexity public List build() throws ParserException { // Check if all fields are unique within this block List duplicates = fields.stream() @@ -334,13 +333,13 @@ public List build() throws ParserException { } // Create parent / child relations for compound fields and validate - // 1. Get all fields that claim to have a parent + // a) Get all fields that claim to have a parent List children = fields.stream() .filter(field -> !field.get(Header.PARENT, "").isEmpty()) .collect(Collectors.toUnmodifiableList()); ParserException parentException = new ParserException("Compound field errors:"); - // 2. Search and associate the parent field to these children + // b) Search and associate the parent field to these children for (Field child : children) { // Always non-null as we filtered for this before, so will never be "" (but we avoid the Optional) String parentName = child.get(Header.PARENT, ""); @@ -364,10 +363,10 @@ public List build() throws ParserException { } } - // 3. No cyclic dependencies / deep nesting is optional + // Check no cyclic dependencies present / deep nesting is optional. // First, fetch fields that are children and parents at the same time: List parentChilds = fields.stream() - .filter(field -> !field.children.isEmpty() && field.parent != null) + .filter(field -> field.isParent() && field.isChild()) .collect(Collectors.toUnmodifiableList()); // When deep nesting is disabled, this list must be empty (only 1 level of nesting allowed means @@ -388,33 +387,46 @@ public List build() throws ParserException { }); } - // 4. Ensure all compound parents are of type "none" + // Ensure compound parents are a) of type "none", b) have no displayFormat other than ":", ";" or "," and + // are c) not facetable and d) not allowed to use a CV List parents = fields.stream() - .filter(field -> !field.children.isEmpty()) + .filter(Field::isParent) .collect(Collectors.toUnmodifiableList()); parents.forEach(field -> { + // a) type none // The first check shall never be true, but just in case... if (field.get(Header.FIELDTYPE).isEmpty() || !Types.NONE.test(field.get(Header.FIELDTYPE).get())) { parentException.addSubException( - new ParserException("'" + field.getName() + "' is a parent but does not have fieldType 'none'") + new ParserException("'" + field.getName() + "' is a parent but does not have 'fieldType'='none'") + .withLineNumber(field.lineIndex)); + } + // b) displayFormat contains only ":", ";" or "," or empty + if (field.get(Header.DISPLAY_FORMAT).isEmpty() || !matchParentDisplayFormat.test(field.get(Header.DISPLAY_FORMAT).get())) { + parentException.addSubException( + new ParserException("'" + field.getName() + "' is a parent but 'displayFormat' is not empty or from [;,:]") + .withLineNumber(field.lineIndex)); + } + // c) not facetable + if (field.get(Header.FACETABLE).isEmpty() || field.get(Header.FACETABLE).get().equals("TRUE")) { + parentException.addSubException( + new ParserException("'" + field.getName() + "' is a parent but has 'facetable'='TRUE'") + .withLineNumber(field.lineIndex)); + } + // c) not using a CV + if (field.get(Header.ALLOW_CONTROLLED_VOCABULARY).isEmpty() || field.get(Header.ALLOW_CONTROLLED_VOCABULARY).get().equals("TRUE")) { + parentException.addSubException( + new ParserException("'" + field.getName() + "' is a parent but has 'allowControlledVocabulary'='TRUE'") .withLineNumber(field.lineIndex)); } }); - - // Check display order uniqueness - // 1. Check all top level fields first - /* - List topLevel = fields.stream() - // get all fields that have no parent, which means top level - .filter(field -> field.parent == null) - .collect(Collectors.groupingBy(field -> field.get(Header.DISPLAY_ORDER, ""))) - .entrySet() - .stream() - .filter(entry -> entry.getValue().size() > 1) - .map(Map.Entry::getKey) - .collect(Collectors.toUnmodifiableList()); - */ + + // TODO: Extend check here with: + // 1) Unique numbers in displayOrder (attention: compound fields!) - should it print warnings when unsorted? + // 2) Warnings for facetable fields with suboptimal field types + // 3) Check if any compound fields are optional but have required children and warn about potential unwanted conditional requirements + // 4) Check no primitive (non-parent) field has type "none" + // 5) ... // Now either die or return fields if (parentException.hasSubExceptions()) { @@ -440,9 +452,9 @@ private boolean hasCyclicDependency(final Field root, final Field parent) { /* ---- Actual Field Class starting here ---- */ private Field parent; - private List children = new ArrayList<>(); - private Map properties = new EnumMap<>(Header.class); - private int lineIndex; + private final List children = new ArrayList<>(); + private final Map properties = new EnumMap<>(Header.class); + private final int lineIndex; private Field(int lineIndex) { this.lineIndex = lineIndex; @@ -463,6 +475,14 @@ public String getName() { return properties.get(Header.NAME); } + public boolean isParent() { + return !this.children.isEmpty(); + } + + public boolean isChild() { + return this.parent != null; + } + List controlledVocabularyValues = List.of(); @Override From 9790e6415c991ed4f77f318e9dd3c6f4cdc4ba9e Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 8 Mar 2023 17:31:10 +0100 Subject: [PATCH 54/56] fix(solrteur): check for correct length of block line --- .../main/java/io/gdcc/solrteur/mdb/tsv/Block.java | 2 +- .../java/io/gdcc/solrteur/mdb/tsv/BlockTest.java | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Block.java b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Block.java index b3b12f32a52..b294b949e4f 100644 --- a/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Block.java +++ b/modules/solr-configset/src/main/java/io/gdcc/solrteur/mdb/tsv/Block.java @@ -193,7 +193,7 @@ public void parseAndValidateLine(final String line) throws ParserException { * @throws ParserException */ Block parseAndValidateColumns(final String[] lineParts) throws ParserException { - if (lineParts == null || lineParts.length > header.size()) { + if (lineParts == null || lineParts.length != header.size()) { throw new ParserException("Does not match length of metadata block headline"); } diff --git a/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/BlockTest.java b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/BlockTest.java index 9fc5008eeff..b12da3702e3 100644 --- a/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/BlockTest.java +++ b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/BlockTest.java @@ -23,7 +23,8 @@ class BlockTest { static final Configuration config = Configuration.defaultConfig(); static final String validHeaderLine = "#metadataBlock\tname\tdataverseAlias\tdisplayName\tblockURI"; - static final String validBlockDef = "\tmyblock\tdataverse\tFooBar Block\thttps://foobar.com/"; + static final String validBlockDef1 = "\tmyblock\t\tFooBar Block\thttps://foobar.com/"; + static final String validBlockDef2 = "\tmyblock\tdataverse\tFooBar Block\thttps://foobar.com/"; @Nested class HeaderTest { @@ -77,7 +78,9 @@ void setUp() throws ParserException { "\tmyblock\tdataverse\tFooBar Block", "\tmyblock\tdataverse\tFooBar Block\thttps://", "\tmyblock\tdataverse\tFooBar Block\thttps://foobar.com/\thello", - "myblock\tdataverse\tFooBar Block\thttps://foobar.com/" + "\tmyblock\t\tFooBar Block\thttps://foobar.com/\thello", + "myblock\tdataverse\tFooBar Block\thttps://foobar.com/", + "myblock\t\tFooBar Block\thttps://foobar.com/" }) void failingParseLine(String line) throws ParserException { ParserException exception = assertThrows(ParserException.class, () -> builder.parseAndValidateLine(line)); @@ -86,7 +89,7 @@ void failingParseLine(String line) throws ParserException { @ParameterizedTest @ValueSource(strings = { - validBlockDef + validBlockDef1, validBlockDef2 }) void succeedingParseLine(String line) throws ParserException { builder.parseAndValidateLine(line); @@ -95,9 +98,9 @@ void succeedingParseLine(String line) throws ParserException { @Test void failingDoubleAdditionAttempt() throws ParserException { - builder.parseAndValidateLine(validBlockDef); + builder.parseAndValidateLine(validBlockDef1); assertTrue(builder.hasSucceeded()); - ParserException exception = assertThrows(ParserException.class, () -> builder.parseAndValidateLine(validBlockDef)); + ParserException exception = assertThrows(ParserException.class, () -> builder.parseAndValidateLine(validBlockDef1)); assertFalse(builder.hasSucceeded()); } } From 329c2005a96fa599d8c63ac51bfeac2225db8385 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 8 Mar 2023 19:43:34 +0100 Subject: [PATCH 55/56] build(solrteur): switch to JAR build to also release the code itself to be reusable --- modules/solr-configset/pom.xml | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/modules/solr-configset/pom.xml b/modules/solr-configset/pom.xml index b13b457d577..1fab865dd77 100644 --- a/modules/solr-configset/pom.xml +++ b/modules/solr-configset/pom.xml @@ -16,7 +16,7 @@ ${project.build.directory}/${solr.configset} - pom + jar ${project.build.sourceDirectory}/io/gdcc/solrteur/solrteur.java @@ -25,16 +25,6 @@ org.apache.maven.plugins maven-compiler-plugin - - - test-compile - test-compile - - testCompile - - - - ${target.java.version} From 0da817293d82b37add652b9f35a8a56aeec23c90 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 8 Mar 2023 19:44:21 +0100 Subject: [PATCH 56/56] test(solrteur): add more tests for parsing fields from TSVs --- .../io/gdcc/solrteur/mdb/tsv/FieldTest.java | 52 +++++++++++++++---- .../test/resources/fields/valid_fields.csv | 12 +++++ 2 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 modules/solr-configset/src/test/resources/fields/valid_fields.csv diff --git a/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/FieldTest.java b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/FieldTest.java index f26a3f3ff58..028296ed290 100644 --- a/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/FieldTest.java +++ b/modules/solr-configset/src/test/java/io/gdcc/solrteur/mdb/tsv/FieldTest.java @@ -3,20 +3,35 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvFileSource; import org.junit.jupiter.params.provider.EmptySource; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.NullAndEmptySource; import org.junit.jupiter.params.provider.NullSource; import org.junit.jupiter.params.provider.ValueSource; +import java.io.File; +import java.io.IOException; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import java.util.stream.Stream; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; class FieldTest { @@ -143,6 +158,7 @@ void failingParseAndValidateHeaderLine(String headerLine) throws ParserException } @Nested + @TestInstance(TestInstance.Lifecycle.PER_CLASS) class ParseLineTest { Field.FieldsBuilder builder; @@ -153,6 +169,7 @@ void setUp() throws ParserException { @ParameterizedTest @NullAndEmptySource + @MethodSource("invalidFieldExamples") void failingParseLine(String line) throws ParserException { ParserException exception = assertThrows(ParserException.class, () -> builder.parseAndValidateLine(0, line)); //assertFalse(builder.hasSucceeded()); @@ -160,18 +177,33 @@ void failingParseLine(String line) throws ParserException { @ParameterizedTest @ValueSource(strings = {validFieldDef}) - @CsvFileSource(files = "/testlines/valid_datasetfields.csv") + @MethodSource("validFieldExamples") void succeedingParseLine(String line) throws ParserException { - builder.parseAndValidateLine(0, line); + try { + System.out.println(line); + builder.parseAndValidateLine(0, line); + } catch (ParserException e) { + e.getSubExceptions().forEach(System.out::println); + fail(e); + } //assertTrue(builder.hasSucceeded()); } - - @Test - void failingDoubleAdditionAttempt() throws ParserException { - builder.parseAndValidateLine(0, validFieldDef); - //assertTrue(builder.hasSucceeded()); - ParserException exception = assertThrows(ParserException.class, () -> builder.parseAndValidateLine(0, validFieldDef)); - //assertFalse(builder.hasSucceeded()); + + Stream validFieldExamples() throws IOException { + Path file = Path.of("", "src/test/resources", "fields", "valid_fields.csv"); + return Files.readAllLines(file, StandardCharsets.UTF_8).stream().map(s -> s.replaceAll(";", "\t")); } + + Stream invalidFieldExamples() throws IOException { + // TODO: write a file with such examples that fail already at parsing the line + Path file = Path.of("", "src/test/resources", "fields", "invalid_fields.csv"); + return Files.readAllLines(file, StandardCharsets.UTF_8).stream().map(s -> s.replaceAll(";", "\t")); + } + } + + @Nested + @TestInstance(TestInstance.Lifecycle.PER_CLASS) + class BuildFieldListTest { + // TODO: write tests for the checks at build time } } \ No newline at end of file diff --git a/modules/solr-configset/src/test/resources/fields/valid_fields.csv b/modules/solr-configset/src/test/resources/fields/valid_fields.csv new file mode 100644 index 00000000000..13fd8c956ee --- /dev/null +++ b/modules/solr-configset/src/test/resources/fields/valid_fields.csv @@ -0,0 +1,12 @@ +;title;Title;The main title of the Dataset;;text;0;;TRUE;FALSE;FALSE;FALSE;TRUE;TRUE;;citation;http://purl.org/dc/terms/title +;subtitle;Subtitle;A secondary title that amplifies or states certain limitations on the main title;;text;1;;FALSE;FALSE;FALSE;FALSE;FALSE;FALSE;;citation; +;alternativeURL;Alternative URL;Another URL where one can view or access the data in the Dataset, e.g. a project or personal webpage;https://;url;3;#VALUE;FALSE;FALSE;FALSE;FALSE;FALSE;FALSE;;citation;https://schema.org/distribution +;author;Author;The entity, e.g. a person or organization, that created the Dataset;;none;7;;FALSE;FALSE;TRUE;FALSE;TRUE;TRUE;;citation;http://purl.org/dc/terms/creator +;authorName;Name;The name of the author, such as the person's name or the name of an organization;1) Family Name, Given Name or 2) Organization XYZ;text;8;#VALUE;TRUE;FALSE;FALSE;TRUE;TRUE;TRUE;author;citation; +;authorAffiliation;Affiliation;The name of the entity affiliated with the author, e.g. an organization's name;Organization XYZ;text;9;(#VALUE);TRUE;FALSE;FALSE;TRUE;TRUE;FALSE;author;citation; +;authorIdentifierScheme;Identifier Type;The type of identifier that uniquely identifies the author (e.g. ORCID, ISNI);;text;10;- #VALUE:;FALSE;TRUE;FALSE;FALSE;TRUE;FALSE;author;citation;http://purl.org/spar/datacite/AgentIdentifierScheme +;authorIdentifier;Identifier;Uniquely identifies the author when paired with an identifier type;;text;11;#VALUE;FALSE;FALSE;FALSE;FALSE;TRUE;FALSE;author;citation;http://purl.org/spar/datacite/AgentIdentifier +;datasetContactEmail;E-mail;The point of contact's email address;name@email.xyz;email;15;#EMAIL;FALSE;FALSE;FALSE;FALSE;TRUE;TRUE;datasetContact;citation; +;dsDescriptionValue;Text;A summary describing the purpose, nature, and scope of the Dataset;;textbox;17;#VALUE;TRUE;FALSE;FALSE;FALSE;TRUE;TRUE;dsDescription;citation; +;dsDescriptionDate;Date;The date when the description was added to the Dataset. If the Dataset contains more than one description, e.g. the data producer supplied one description and the data repository supplied another, this date is used to distinguish between the descriptions;YYYY-MM-DD;date;18;(#VALUE);FALSE;FALSE;FALSE;FALSE;TRUE;FALSE;dsDescription;citation; +;keywordVocabularyURI;Controlled Vocabulary URL;The URL where one can access information about the term's controlled vocabulary;https://;url;23;#VALUE;FALSE;FALSE;FALSE;FALSE;TRUE;FALSE;keyword;citation;