Skip to content

Commit

Permalink
[refactor] Review feedback (human and automatic)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@alanpaxton
alanpaxton committed Apr 6, 2023
1 parent 67ecfda commit 71955d4
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 67 deletions.
3 changes: 1 addition & 2 deletions exist-core/src/main/java/org/exist/storage/BrokerPool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions exist-core/src/main/java/org/exist/util/Configuration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public Configuration(String configFilename, Optional<Path> 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));
}
Expand Down Expand Up @@ -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);
}
}

Expand Down
105 changes: 55 additions & 50 deletions exist-core/src/main/java/org/exist/util/SaxonConfigurationHolder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -125,54 +120,64 @@ private net.sf.saxon.Configuration loadConfiguration() {
final var saxonConfigFile = getSaxonConfigFile(existConfiguration);
Optional<net.sf.saxon.Configuration> 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());
}

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<net.sf.saxon.Configuration> 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 + ".");
}
}

/**
Expand All @@ -198,17 +203,17 @@ private static File resolveConfigurationFile(final Configuration existConfigurat

private static Optional<File> 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);
}
Expand Down
4 changes: 2 additions & 2 deletions exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Options.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 0 additions & 1 deletion exist-core/src/main/java/org/exist/xquery/functions/fn/transform/Transform.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions exist-core/src/test/java/org/exist/config/SaxonConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
14 changes: 7 additions & 7 deletions exist-distribution/Saxon-EE.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
<dependency>
<groupId>net.sf.saxon</groupId>
<artifactId>Saxon-HE</artifactId>
Expand All @@ -26,7 +26,7 @@ In both `$EXIST_HOME/etc/client.xml` and `$EXIST_HOME/etc/startup.xml` replace t
</dependency>
```
with a dependency on the JAR you just installed
```
```xml
<dependency>
<groupId>net.sf.saxon</groupId>
<artifactId>Saxon-EE</artifactId>
Expand All @@ -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
<configuration edition="HE"
xmlns="http://saxon.sf.net/ns/configuration"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://saxon.sf.net/ns/configuration config.xsd">
</configuration>
```
with
```
```xml
<configuration edition="EE"
xmlns="http://saxon.sf.net/ns/configuration"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://saxon.sf.net/ns/configuration config.xsd">
Expand Down

0 comments on commit 71955d4

Please sign in to comment.