From 71955d4ad829ae6eac361a66b19e7b721e871e6d Mon Sep 17 00:00:00 2001 From: Alan Paxton Date: Thu, 6 Apr 2023 14:54:14 +0100 Subject: [PATCH] [refactor] Review feedback (human and automatic) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit De-genericize static names Don’t capitalize static method Remove unused imports Unpick method for complexity metric Re-order some qualifiers Make markdown conform to checker rules --- .../java/org/exist/storage/BrokerPool.java | 3 +- .../java/org/exist/util/Configuration.java | 6 +- .../exist/util/SaxonConfigurationHolder.java | 105 +++++++++--------- .../functions/fn/transform/Options.java | 4 +- .../functions/fn/transform/Transform.java | 1 - .../org/exist/config/SaxonConfigTest.java | 6 +- exist-distribution/Saxon-EE.md | 14 +-- 7 files changed, 72 insertions(+), 67 deletions(-) diff --git a/exist-core/src/main/java/org/exist/storage/BrokerPool.java b/exist-core/src/main/java/org/exist/storage/BrokerPool.java index c07f253704a..af10028e1ac 100644 --- a/exist-core/src/main/java/org/exist/storage/BrokerPool.java +++ b/exist-core/src/main/java/org/exist/storage/BrokerPool.java @@ -23,7 +23,6 @@ import com.evolvedbinary.j8fu.fsm.AtomicFSM; import com.evolvedbinary.j8fu.fsm.FSM; -import com.evolvedbinary.j8fu.lazy.AtomicLazyVal; import net.jcip.annotations.ThreadSafe; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -420,7 +419,7 @@ public String getStatus() { this.pageSize = conf.getProperty(PROPERTY_PAGE_SIZE, DEFAULT_PAGE_SIZE); - this.saxonConfigurationHolder = SaxonConfigurationHolder.GetHolderForBroker(this); + this.saxonConfigurationHolder = SaxonConfigurationHolder.getHolderForBroker(this); //Configuration is valid, save it this.conf = conf; diff --git a/exist-core/src/main/java/org/exist/util/Configuration.java b/exist-core/src/main/java/org/exist/util/Configuration.java index 721d2f588ef..7df8a9e2e70 100644 --- a/exist-core/src/main/java/org/exist/util/Configuration.java +++ b/exist-core/src/main/java/org/exist/util/Configuration.java @@ -261,7 +261,7 @@ public Configuration(String configFilename, Optional existHomeDirname) thr } //saxon settings (most importantly license file for PE or EE features) - final NodeList saxon = doc.getElementsByTagName(SaxonConfigurationHolder.CONFIGURATION_ELEMENT_NAME); + final NodeList saxon = doc.getElementsByTagName(SaxonConfigurationHolder.SAXON_CONFIGURATION_ELEMENT_NAME); if( saxon.getLength() > 0 ) { configureSaxon((Element)saxon.item(0)); } @@ -547,9 +547,9 @@ private void configureXUpdate( Element xupdate ) throws NumberFormatException private void configureSaxon( Element saxon ) { final String configurationFile = getConfigAttributeValue( saxon, - SaxonConfigurationHolder.CONFIGURATION_FILE_ATTRIBUTE); + SaxonConfigurationHolder.SAXON_CONFIGURATION_FILE_ATTRIBUTE); if (configurationFile != null) { - config.put(SaxonConfigurationHolder.CONFIGURATION_FILE_PROPERTY, configurationFile); + config.put(SaxonConfigurationHolder.SAXON_CONFIGURATION_FILE_PROPERTY, configurationFile); } } diff --git a/exist-core/src/main/java/org/exist/util/SaxonConfigurationHolder.java b/exist-core/src/main/java/org/exist/util/SaxonConfigurationHolder.java index 01c7ccf2bb2..13394777b7a 100644 --- a/exist-core/src/main/java/org/exist/util/SaxonConfigurationHolder.java +++ b/exist-core/src/main/java/org/exist/util/SaxonConfigurationHolder.java @@ -22,7 +22,6 @@ package org.exist.util; import com.evolvedbinary.j8fu.lazy.AtomicLazyVal; -import net.sf.saxon.lib.Feature; import net.sf.saxon.s9api.Processor; import net.sf.saxon.trans.XPathException; import org.exist.storage.BrokerPool; @@ -32,17 +31,16 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; -import java.io.IOException; import java.util.HashMap; import java.util.Map; import java.util.Optional; public final class SaxonConfigurationHolder { - public final static String CONFIGURATION_ELEMENT_NAME = "saxon"; - public final static String CONFIGURATION_FILE_ATTRIBUTE = "configuration-file"; - public final static String CONFIGURATION_FILE_PROPERTY = "saxon.configuration"; - private final static String DEFAULT_SAXON_CONFIG_FILE = "saxon-config.xml"; + public static final String SAXON_CONFIGURATION_ELEMENT_NAME = "saxon"; + public static final String SAXON_CONFIGURATION_FILE_ATTRIBUTE = "configuration-file"; + public static final String SAXON_CONFIGURATION_FILE_PROPERTY = "saxon.configuration"; + private static final String SAXON_DEFAULT_SAXON_CONFIG_FILE = "saxon-config.xml"; /** * Maintain a separate, singleton Saxon configuration for each broker pool @@ -77,11 +75,8 @@ private SaxonConfigurationHolder(final BrokerPool brokerPool) { * * @return the associated Saxon configuration wrapper */ - public synchronized static SaxonConfigurationHolder GetHolderForBroker(final BrokerPool brokerPool) { - if (!perBroker.containsKey(brokerPool)) { - perBroker.put(brokerPool, new SaxonConfigurationHolder(brokerPool)); - } - return perBroker.get(brokerPool); + public static synchronized SaxonConfigurationHolder getHolderForBroker(final BrokerPool brokerPool) { + return perBroker.computeIfAbsent(brokerPool, SaxonConfigurationHolder::new); } /** @@ -125,20 +120,7 @@ private net.sf.saxon.Configuration loadConfiguration() { final var saxonConfigFile = getSaxonConfigFile(existConfiguration); Optional saxonConfiguration = Optional.empty(); if (saxonConfigFile.isPresent()) { - try { - saxonConfiguration = Optional.of(net.sf.saxon.Configuration.readConfiguration( - new StreamSource(new FileInputStream(saxonConfigFile.get())))); - } catch (XPathException | FileNotFoundException e) { - Log.warn("Saxon could not read the configuration file: " + saxonConfigFile.get() + - ", with error: " + e.getMessage(), e); - } catch (RuntimeException runtimeException) { - if (runtimeException.getCause() instanceof ClassNotFoundException e) { - Log.warn("Saxon could not honour the configuration file: " + saxonConfigFile.get() + - ", with class not found error: " + e.getMessage() + ". You may need to install the SaxonPE or SaxonEE JAR in eXist."); - } else { - throw runtimeException; - } - } + saxonConfiguration = readSaxonConfigurationFile(saxonConfigFile.get()); } if (saxonConfiguration.isEmpty()) { saxonConfiguration = Optional.of(net.sf.saxon.Configuration.newConfiguration()); @@ -146,33 +128,56 @@ private net.sf.saxon.Configuration loadConfiguration() { if (saxonConfigFile.isEmpty()) { Log.warn("eXist could not find any Saxon configuration:\n" + - "No Saxon configuration file in configuration item " + CONFIGURATION_FILE_PROPERTY + "\n" + - "No default eXist Saxon configuration file " + DEFAULT_SAXON_CONFIG_FILE); + "No Saxon configuration file in configuration item " + SAXON_CONFIGURATION_FILE_PROPERTY + "\n" + + "No default eXist Saxon configuration file " + SAXON_DEFAULT_SAXON_CONFIG_FILE); } - saxonConfiguration.ifPresent(net.sf.saxon.Configuration::displayLicenseMessage); - saxonConfiguration.ifPresent(configuration -> { - final var sb = new StringBuilder(); - if (configuration.isLicensedFeature(net.sf.saxon.Configuration.LicenseFeature.SCHEMA_VALIDATION)) { - sb.append(" SCHEMA_VALIDATION"); - } - if (configuration.isLicensedFeature(net.sf.saxon.Configuration.LicenseFeature.ENTERPRISE_XSLT)) { - sb.append(" ENTERPRISE_XSLT"); - } - if (configuration.isLicensedFeature(net.sf.saxon.Configuration.LicenseFeature.ENTERPRISE_XQUERY)) { - sb.append(" ENTERPRISE_XQUERY"); - } - if (configuration.isLicensedFeature(net.sf.saxon.Configuration.LicenseFeature.PROFESSIONAL_EDITION)) { - sb.append(" PROFESSIONAL_EDITION"); - } - if (sb.length() == 0) { - Log.info("Saxon - no licensed features reported."); + saxonConfiguration.ifPresent(SaxonConfigurationHolder::reportLicensedFeatures); + + return saxonConfiguration.get(); + } + + static private Optional readSaxonConfigurationFile(final File saxonConfigFile) { + + try { + return Optional.of(net.sf.saxon.Configuration.readConfiguration( + new StreamSource(new FileInputStream(saxonConfigFile)))); + } catch (XPathException | FileNotFoundException e) { + Log.warn("Saxon could not read the configuration file: " + saxonConfigFile + + ", with error: " + e.getMessage(), e); + } catch (RuntimeException runtimeException) { + if (runtimeException.getCause() instanceof ClassNotFoundException e) { + Log.warn("Saxon could not honour the configuration file: " + saxonConfigFile + + ", with class not found error: " + e.getMessage() + ". You may need to install the SaxonPE or SaxonEE JAR in eXist."); } else { - Log.info("Saxon - licensed features are" + sb + "."); + throw runtimeException; } - }); + } - return saxonConfiguration.get(); + return Optional.empty(); + } + + static private void reportLicensedFeatures(final net.sf.saxon.Configuration configuration) { + configuration.displayLicenseMessage(); + + final var sb = new StringBuilder(); + if (configuration.isLicensedFeature(net.sf.saxon.Configuration.LicenseFeature.SCHEMA_VALIDATION)) { + sb.append(" SCHEMA_VALIDATION"); + } + if (configuration.isLicensedFeature(net.sf.saxon.Configuration.LicenseFeature.ENTERPRISE_XSLT)) { + sb.append(" ENTERPRISE_XSLT"); + } + if (configuration.isLicensedFeature(net.sf.saxon.Configuration.LicenseFeature.ENTERPRISE_XQUERY)) { + sb.append(" ENTERPRISE_XQUERY"); + } + if (configuration.isLicensedFeature(net.sf.saxon.Configuration.LicenseFeature.PROFESSIONAL_EDITION)) { + sb.append(" PROFESSIONAL_EDITION"); + } + if (sb.length() == 0) { + Log.info("Saxon - no licensed features reported."); + } else { + Log.info("Saxon - licensed features are" + sb + "."); + } } /** @@ -198,17 +203,17 @@ private static File resolveConfigurationFile(final Configuration existConfigurat private static Optional getSaxonConfigFile(final Configuration existConfiguration) { - if (existConfiguration.getProperty(CONFIGURATION_FILE_PROPERTY) instanceof String saxonConfigurationFile) { + if (existConfiguration.getProperty(SAXON_CONFIGURATION_FILE_PROPERTY) instanceof String saxonConfigurationFile) { final var configurationFile = resolveConfigurationFile(existConfiguration, saxonConfigurationFile); if (configurationFile.canRead()) { return Optional.of(configurationFile); } else { - Log.warn("Configuration item " + CONFIGURATION_FILE_PROPERTY + " : " + configurationFile + + Log.warn("Configuration item " + SAXON_CONFIGURATION_FILE_PROPERTY + " : " + configurationFile + " does not refer to a readable file. Continuing search for Saxon configuration."); } } - final var configurationFile = resolveConfigurationFile(existConfiguration, DEFAULT_SAXON_CONFIG_FILE); + final var configurationFile = resolveConfigurationFile(existConfiguration, SAXON_DEFAULT_SAXON_CONFIG_FILE); if (configurationFile.canRead()) { return Optional.of(configurationFile); } diff --git a/exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Options.java b/exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Options.java index 20faf279f55..1008ae30c1b 100644 --- a/exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Options.java +++ b/exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Options.java @@ -654,13 +654,13 @@ public Reader getReader() { static class SystemProperties { + private final RetainedStaticContext retainedStaticContext; + private SystemProperties(XQueryContext context) { final var saxonConfiguration = context.getBroker().getBrokerPool().getSaxonConfiguration(); this.retainedStaticContext = new RetainedStaticContext(saxonConfiguration); } - private final RetainedStaticContext retainedStaticContext; - String get(org.exist.dom.QName qName) { return SystemProperty.getProperty(qName.getNamespaceURI(), qName.getLocalPart(), retainedStaticContext); } diff --git a/exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Transform.java b/exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Transform.java index 834edc08ab2..8689b392d3c 100644 --- a/exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Transform.java +++ b/exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Transform.java @@ -34,7 +34,6 @@ import org.apache.logging.log4j.Logger; import org.exist.dom.QName; import org.exist.dom.memtree.DocumentImpl; -import org.exist.storage.BrokerPool; import org.exist.util.Holder; import org.exist.xquery.ErrorCodes; import org.exist.xquery.XPathException; diff --git a/exist-core/src/test/java/org/exist/config/SaxonConfigTest.java b/exist-core/src/test/java/org/exist/config/SaxonConfigTest.java index d1ec068694f..f7ad8eb1e31 100644 --- a/exist-core/src/test/java/org/exist/config/SaxonConfigTest.java +++ b/exist-core/src/test/java/org/exist/config/SaxonConfigTest.java @@ -41,12 +41,14 @@ public void configFromBroker() { final var existConfiguration = brokerPool.getConfiguration(); assertThat(existConfiguration.getProperty("saxon.configuration")).isEqualTo("saxon-config.xml"); - final var saxonConfigurationHolder = SaxonConfigurationHolder.GetHolderForBroker(brokerPool); + final var saxonConfigurationHolder = SaxonConfigurationHolder.getHolderForBroker(brokerPool); final var saxonConfiguration = saxonConfigurationHolder.getConfiguration(); + // There is no way to install EE at the test/build phase. + // Sanity check is to confirm this does indeed return "HE" (Home Edition). final var saxonProcessor = saxonConfigurationHolder.getProcessor(); + assertThat(saxonProcessor.getSaxonEdition()).isEqualTo("HE"); - //TODO (AP) - brokerPool.getSaxonConfiguration() needs to use SaxonConfigurationHolder.GetHolderForBroker(brokerPool) final var saxonConfiguration2 = brokerPool.getSaxonConfiguration(); assertThat(saxonConfiguration2).isSameAs(saxonConfiguration); diff --git a/exist-distribution/Saxon-EE.md b/exist-distribution/Saxon-EE.md index 01c6140a3f6..781f4aa7fbe 100644 --- a/exist-distribution/Saxon-EE.md +++ b/exist-distribution/Saxon-EE.md @@ -10,14 +10,14 @@ This is the directory which is prompted for (and or defaulted) by the eXist inst ## Replace the Saxon JAR file - * Replace the existing `$EXIST_HOME/lib/Saxon-HE-9.9.1-8.jar` with `saxon9ee.jar` fetched by downloading `https://www.saxonica.com/download/SaxonEE9-9-1-8J.zip` and unzipping. - * At present only Saxon EE (or PE) version 9.9.1.8 is supported. - * Confirm the JAR is present at `$EXIST_HOME/lib/saxon9ee.jar` +* Replace the existing `$EXIST_HOME/lib/Saxon-HE-9.9.1-8.jar` with `saxon9ee.jar` fetched by downloading `https://www.saxonica.com/download/SaxonEE9-9-1-8J.zip` and unzipping. +* At present only Saxon EE (or PE) version 9.9.1.8 is supported. +* Confirm the JAR is present at `$EXIST_HOME/lib/saxon9ee.jar` ## Update configuration In both `$EXIST_HOME/etc/client.xml` and `$EXIST_HOME/etc/startup.xml` replace the dependency -``` +```xml net.sf.saxon Saxon-HE @@ -26,7 +26,7 @@ In both `$EXIST_HOME/etc/client.xml` and `$EXIST_HOME/etc/startup.xml` replace t ``` with a dependency on the JAR you just installed -``` +```xml net.sf.saxon Saxon-EE @@ -42,14 +42,14 @@ Place the `saxon-license.lic` file in `$EXIST_HOME/lib` ## Update the config file At a mimimun, you will need to replace -``` +```xml ``` with -``` +```xml