Skip to content

Commit

Permalink
Build improvements and race condition fix (#371)
Browse files Browse the repository at this point in the history
* Updated dependencies to newer versions. Cleaned up some unused dependencies.
* Updated assembly configuration to remove a compile warning.
* Added the ability to directly get the message associated with an exit status.
* Fixed a race condition around setting private member access that is caused when multiple threads attempt to write to a bound bean. This is a temporary fix. A longer term fix will be to use the setter and getter methods directly to avoid the need to change access to a private member.
  • Loading branch information
david-waltermire authored Feb 27, 2025
1 parent c16dcd5 commit e837c3c
Show file tree
Hide file tree
Showing 19 changed files with 132 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,6 @@ public ExitStatus withThrowable(@NonNull Throwable throwable) {
return this;
}

/**
* Get the associated message.
*
* @return the message or {@code null}
*/
@Nullable
protected abstract String getMessage();

/**
* Determines the appropriate LogBuilder based on the exit code status. For
* non-positive exit codes (success/info), returns an INFO level builder. For
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ public interface ExitStatus {
*/
void generateMessage(boolean showStackTrace);

/**
* Get the associated message, or {@code null} if there is no message.
*
* @return the message or {@code null}
*/
@Nullable
String getMessage();

/**
* Associate a throwable with the exit status.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class NonMessageExitStatus
* @return {@code null} as this implementation does not support messages
*/
@Override
protected String getMessage() {
public String getMessage() {
return null;
}
}
6 changes: 0 additions & 6 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,6 @@
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>biz.aQute.bnd</groupId>
<artifactId>biz.aQute.bnd.util</artifactId>
<scope>provided</scope>
</dependency>
</dependencies>

<build>
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
* @provides IFunctionLibrary for core built-in Metapath functions
* @uses IDataTypeProvider to discover data types implementing
* {@link gov.nist.secauto.metaschema.core.datatype.IDataTypeAdapter}
* @uses IFunctionLibrary to discover collections of Metapath functions implementing
* @uses IFunctionLibrary to discover collections of Metapath functions
* implementing
* {@link gov.nist.secauto.metaschema.core.metapath.function.IFunction}
*/
module gov.nist.secauto.metaschema.core {
Expand All @@ -23,7 +24,6 @@
requires java.xml;

requires static org.eclipse.jdt.annotation;
requires static biz.aQute.bnd.util;
requires static com.github.spotbugs.annotations;

requires com.ctc.wstx;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ default Class<?> getItemType() {
@Override
default Object getValue(@NonNull Object parent) {
Field field = getField();
boolean accessable = field.canAccess(parent);
field.setAccessible(true); // NOPMD - intentional
// boolean accessable = field.canAccess(parent);
// field.setAccessible(true); // NOPMD - intentional
Object retval;
try {
Object result = field.get(parent);
Expand All @@ -59,17 +59,17 @@ default Object getValue(@NonNull Object parent) {
String.format("Unable to get the value of field '%s' in class '%s'.", field.getName(),
field.getDeclaringClass().getName()),
ex);
} finally {
field.setAccessible(accessable); // NOPMD - intentional
// } finally {
// field.setAccessible(accessable); // NOPMD - intentional
}
return retval;
}

@Override
default void setValue(@NonNull Object parentObject, Object value) {
Field field = getField();
boolean accessable = field.canAccess(parentObject);
field.setAccessible(true); // NOPMD - intentional
// boolean accessable = field.canAccess(parentObject);
// field.setAccessible(true); // NOPMD - intentional
try {
field.set(parentObject, value);
} catch (IllegalArgumentException | IllegalAccessException ex) {
Expand All @@ -80,8 +80,8 @@ default void setValue(@NonNull Object parentObject, Object value) {
field.getName(),
field.getDeclaringClass().getName()),
ex);
} finally {
field.setAccessible(accessable); // NOPMD - intentional
// } finally {
// field.setAccessible(accessable); // NOPMD - intentional
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ private DefinitionField(
clazz.getName(),
BoundFieldValue.class.getName())); // NOPMD false positive
}
FieldSupport.bindField(field);
this.fieldValue = new FieldValue(field, BoundFieldValue.class, bindingContext);
this.flagContainer = ObjectUtils.notNull(Lazy.lazy(() -> new FlagContainerSupport(this, this::handleFlagInstance)));

Expand Down Expand Up @@ -269,6 +270,7 @@ protected FieldValue(
@NonNull Field javaField,
@NonNull Class<BoundFieldValue> annotationClass,
@NonNull IBindingContext bindingContext) {
FieldSupport.bindField(javaField);
this.javaField = javaField;
this.annotation = ModelUtil.getAnnotation(javaField, annotationClass);
this.javaTypeAdapter = ModelUtil.getDataTypeAdapter(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* SPDX-FileCopyrightText: none
* SPDX-License-Identifier: CC0-1.0
*/

package gov.nist.secauto.metaschema.databind.model.impl;

import java.lang.reflect.Field;

import edu.umd.cs.findbugs.annotations.NonNull;

public final class FieldSupport {

/**
* Ensure that the provided field can be accessed.
*
* @param field
* the field to check
*/
@SuppressWarnings("PMD.AvoidAccessibilityAlteration")
public static void bindField(@NonNull Field field) {
field.setAccessible(true);
}

private FieldSupport() {
// disable construction
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public InstanceFlagInline(
@NonNull Field javaField,
@NonNull IBoundDefinitionModel<IBoundObject> parent) {
super(parent);
FieldSupport.bindField(javaField);
this.javaField = javaField;
this.annotation = ModelUtil.getAnnotation(javaField, BoundFlag.class);
Class<? extends IDataTypeAdapter<?>> adapterClass = ObjectUtils.notNull(getAnnotation().typeAdapter());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ private InstanceModelAssemblyComplex(
@NonNull IBoundDefinitionModelAssembly definition,
@NonNull IBoundDefinitionModelAssembly containingDefinition) {
super(containingDefinition);
FieldSupport.bindField(javaField);
this.javaField = javaField;
this.annotation = annotation;
this.groupAs = groupAs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ private InstanceModelChoiceGroup(
@NonNull IGroupAs groupAs,
@NonNull IBoundDefinitionModelAssembly parent) {
super(parent);
FieldSupport.bindField(javaField);
this.javaField = javaField;
this.annotation = annotation;
this.groupAs = groupAs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ private InstanceModelFieldComplex(
@NonNull DefinitionField definition,
@NonNull IBoundDefinitionModelAssembly parent) {
super(parent);
FieldSupport.bindField(javaField);
this.javaField = javaField;
this.annotation = annotation;
this.collectionInfo = ObjectUtils.notNull(Lazy.lazy(() -> IModelInstanceCollectionInfo.of(this)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ private InstanceModelFieldScalar(
@NonNull IGroupAs groupAs,
@NonNull IBoundDefinitionModelAssembly parent) {
super(parent);
FieldSupport.bindField(javaField);
this.javaField = javaField;
this.annotation = annotation;
this.collectionInfo = ObjectUtils.notNull(Lazy.lazy(() -> IModelInstanceCollectionInfo.of(this)));
Expand Down
23 changes: 20 additions & 3 deletions metaschema-cli/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -148,18 +148,35 @@
<version>0.7.0</version>
</dependency>
</dependencies>
<configuration>
<tarLongFileMode>posix</tarLongFileMode>
</configuration>
<executions>
<execution>
<id>make-assembly-bin</id> <!-- this is used for inheritance merges -->
<id>make-assembly-dir</id> <!-- this is used for
inheritance merges -->
<phase>package</phase> <!-- bind to the packaging phase -->
<goals>
<goal>single</goal>
</goals>
<configuration>
<attach>false</attach>
<descriptors>
<descriptor>src/main/assembly/dir.xml</descriptor>
</descriptors>
</configuration>
</execution>
<execution>
<id>make-assembly-archives</id> <!-- this is used for
inheritance merges -->
<phase>package</phase> <!-- bind to the packaging phase -->
<goals>
<goal>single</goal>
</goals>
<configuration>
<descriptors>
<descriptor>src/main/assembly/bin.xml</descriptor>
<descriptor>src/main/assembly/assembly.xml</descriptor>
</descriptors>
<tarLongFileMode>posix</tarLongFileMode>
</configuration>
</execution>
</executions>
Expand Down
14 changes: 14 additions & 0 deletions metaschema-cli/src/main/assembly/assembly.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<assembly
xmlns="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.2"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.2 http://maven.apache.org/xsd/assembly-1.1.2.xsd">
<id>metaschema-cli</id>
<formats>
<format>zip</format>
<format>tar.bz2</format>
</formats>
<includeBaseDirectory>false</includeBaseDirectory>
<componentDescriptors>
<componentDescriptor>component.xml</componentDescriptor>
</componentDescriptors>
</assembly>
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
<assembly
xmlns="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.2"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.2 http://maven.apache.org/xsd/assembly-1.1.2.xsd">
<id>metaschema-cli</id>
<formats>
<format>dir</format>
<format>zip</format>
<format>tar.bz2</format>
</formats>
<includeBaseDirectory>false</includeBaseDirectory>
<component xmlns="http://maven.apache.org/ASSEMBLY-COMPONENT/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/ASSEMBLY-COMPONENT/2.0.0 https://maven.apache.org/xsd/assembly-component-2.0.0.xsd">
<dependencySets>
<dependencySet>
<outputDirectory>/lib</outputDirectory>
Expand Down Expand Up @@ -65,4 +57,4 @@
</configuration>
</containerDescriptorHandler>
</containerDescriptorHandlers>
</assembly>
</component>
13 changes: 13 additions & 0 deletions metaschema-cli/src/main/assembly/dir.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<assembly
xmlns="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.2"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/plugins/maven-assembly-plugin/assembly/1.1.2 http://maven.apache.org/xsd/assembly-1.1.2.xsd">
<id>metaschema-cli</id>
<formats>
<format>dir</format>
</formats>
<includeBaseDirectory>false</includeBaseDirectory>
<componentDescriptors>
<componentDescriptor>component.xml</componentDescriptor>
</componentDescriptors>
</assembly>
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,6 @@

package gov.nist.secauto.metaschema.cli.commands;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.URI;
import java.net.UnknownHostException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.List;
import java.util.Set;

import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Option;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import gov.nist.secauto.metaschema.cli.processor.CLIProcessor;
import gov.nist.secauto.metaschema.cli.processor.CLIProcessor.CallingContext;
import gov.nist.secauto.metaschema.cli.processor.ExitCode;
Expand All @@ -46,6 +29,24 @@
import gov.nist.secauto.metaschema.databind.io.IBoundLoader;
import gov.nist.secauto.metaschema.modules.sarif.SarifValidationHandler;

import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Option;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.URI;
import java.net.UnknownHostException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.List;
import java.util.Set;

import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;

/**
* Used by implementing classes to provide a content validation command.
*/
Expand Down
Loading

0 comments on commit e837c3c

Please sign in to comment.