[java-metadata-aggregator] branch main updated: MDA-258 - Stage classes should only have 0-arg constructors
Ian Young
ian at iay.org.uk
Thu Apr 20 16:58:56 UTC 2023
This is an automated email from the git hooks/post-receive script.
iay pushed a commit to branch main
in repository java-metadata-aggregator.
View the commit online:
http://git.shibboleth.net/view/?p=java-metadata-aggregator.git;a=commit;h=97686fd8daccb2359d051613f0a1421beca9d387
The following commit(s) were added to refs/heads/main by this push:
new 97686fd MDA-258 - Stage classes should only have 0-arg constructors
97686fd is described below
commit 97686fd8daccb2359d051613f0a1421beca9d387
Author: Ian Young <ian at iay.org.uk>
AuthorDate: Thu Apr 20 17:58:47 2023 +0100
MDA-258 - Stage classes should only have 0-arg constructors
https://shibboleth.atlassian.net/browse/MDA-258
---
.../metadata/dom/saml/GenerateIdStage.java | 67 ++++++++++++++++++----
.../net/shibboleth/metadata/BeansFileTest.java | 17 +++++-
.../metadata/dom/saml/GenerateIdStageTest.java | 16 +++---
3 files changed, 81 insertions(+), 19 deletions(-)
diff --git a/mda-framework/src/main/java/net/shibboleth/metadata/dom/saml/GenerateIdStage.java b/mda-framework/src/main/java/net/shibboleth/metadata/dom/saml/GenerateIdStage.java
index aee9979..961a8e4 100644
--- a/mda-framework/src/main/java/net/shibboleth/metadata/dom/saml/GenerateIdStage.java
+++ b/mda-framework/src/main/java/net/shibboleth/metadata/dom/saml/GenerateIdStage.java
@@ -18,6 +18,7 @@
package net.shibboleth.metadata.dom.saml;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.GuardedBy;
import javax.annotation.concurrent.ThreadSafe;
import javax.xml.namespace.QName;
@@ -27,12 +28,24 @@ import org.w3c.dom.Element;
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.pipeline.AbstractIteratingStage;
import net.shibboleth.metadata.pipeline.StageProcessingException;
+import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.primitive.DeprecationSupport;
+import net.shibboleth.shared.primitive.DeprecationSupport.ObjectType;
import net.shibboleth.shared.security.IdentifierGenerationStrategy;
import net.shibboleth.shared.security.impl.Type4UUIDIdentifierGenerationStrategy;
import net.shibboleth.shared.xml.AttributeSupport;
-/** A stage that populates the ID attribute of an EntitiesDescriptor or EntityDescriptor. */
+
+/**
+ * A stage that populates the ID attribute of an EntitiesDescriptor or EntityDescriptor.
+ *
+ * <p>
+ * By default, the stage will use a {@link Type4UUIDIdentifierGenerationStrategy}
+ * to generate identifiers.
+ * </p>
+ */
@ThreadSafe
public class GenerateIdStage extends AbstractIteratingStage<Element> {
@@ -40,21 +53,48 @@ public class GenerateIdStage extends AbstractIteratingStage<Element> {
public static final @Nonnull QName ID_ATTRIB = new QName("ID");
/** Strategy used to generate identifiers. */
- @Nonnull
- private final IdentifierGenerationStrategy idGenerator;
+ @GuardedBy("this")
+ private @NonnullAfterInit IdentifierGenerationStrategy generator;
- /** Constructor. Initialize the {@link #idGenerator} to a {@link Type4UUIDIdentifierGenerationStrategy}. */
+ /** Constructor. */
public GenerateIdStage() {
- idGenerator = new Type4UUIDIdentifierGenerationStrategy();
}
/**
* Constructor.
+ *
+ * <p>
+ * Note: BeansFileTest has an explicit exception to allow this deprecated
+ * constructor. It should be updated when this constructor is removed.
+ * </p>
*
- * @param generator ID generation strategy used
+ * @param newGenerator ID generation strategy to use
+ * @deprecated Use the zero-argument constructor and the property instead.
+ */
+ @Deprecated(since="0.10.0", forRemoval=true)
+ public GenerateIdStage(@Nonnull final IdentifierGenerationStrategy newGenerator) {
+ DeprecationSupport.warnOnce(ObjectType.METHOD, "single-argument constructor", "GenerateIdStage",
+ "zero-argument constructor and 'generator' property");
+ generator = Constraint.isNotNull(newGenerator, "ID generation strategy can not be null");
+ }
+
+ /**
+ * Gets the {@link IdentifierGenerationStrategy} being used.
+ *
+ * @return the {@link IdentifierGenerationStrategy} being used
*/
- public GenerateIdStage(@Nonnull final IdentifierGenerationStrategy generator) {
- idGenerator = Constraint.isNotNull(generator, "ID generation strategy can not be null");
+ public synchronized @NonnullAfterInit IdentifierGenerationStrategy getGenerator() {
+ return generator;
+ }
+
+ /**
+ * Sets the {@link IdentifierGenerationStrategy} to use.
+ *
+ * @param newGenerator the {@link IdentifierGenerationStrategy} to use
+ */
+ public synchronized void setGenerator(final @Nonnull IdentifierGenerationStrategy newGenerator) {
+ checkSetterPreconditions();
+ generator = Constraint.isNotNull(newGenerator, "ID generation strategy can not be null");
}
@Override
@@ -72,7 +112,14 @@ public class GenerateIdStage extends AbstractIteratingStage<Element> {
element.setAttributeNode(idAttribute);
}
- // Don't need to synchronize; field initialized by constructor
- idAttribute.setValue(idGenerator.generateIdentifier());
+ idAttribute.setValue(getGenerator().generateIdentifier());
+ }
+
+ @Override
+ protected void doInitialize() throws ComponentInitializationException {
+ super.doInitialize();
+ if (generator == null) {
+ generator = new Type4UUIDIdentifierGenerationStrategy();
+ }
}
}
diff --git a/mda-framework/src/test/java/net/shibboleth/metadata/BeansFileTest.java b/mda-framework/src/test/java/net/shibboleth/metadata/BeansFileTest.java
index 93cbfe8..717554d 100644
--- a/mda-framework/src/test/java/net/shibboleth/metadata/BeansFileTest.java
+++ b/mda-framework/src/test/java/net/shibboleth/metadata/BeansFileTest.java
@@ -52,16 +52,29 @@ public class BeansFileTest {
if (className != null) {
try {
// Check that the class can be loaded
- Class.forName(className);
+ final var clazz = Class.forName(className);
// The name of the class within its package should be included in the bean
// name's prefix; there may be more after that
final String classLastPart = className.replaceFirst("^.*\\.", "");
Assert.assertTrue(defName.startsWith("mda." + classLastPart), "does not start with correct prefix: " + defName);
- // If the class represents a stage, its parent should be the stage parent
+ // Process a class representing a Stage
if (className.endsWith("Stage")) {
+ // If the class represents a stage, its parent should be the stage parent
Assert.assertEquals(def.getParentName(), "mda.stage_parent");
+
+ // Look at constructors. We should only have zero-arg ones (MDA-258)
+ // Because we have only DEPRECATED the constructor on GenerateIdStage, skip that
+ // class here.
+ if (defName.equals("mda.GenerateIdStage")) {
+ continue;
+ }
+ final var constructors = clazz.getDeclaredConstructors();
+ for (final var constructor : constructors) {
+ Assert.assertEquals(constructor.getParameterCount(), 0,
+ "non-zero-argument constructor in class implementing Stage: " + clazz.getName());
+ }
}
} catch (ClassNotFoundException e) {
// Could not load class
diff --git a/mda-framework/src/test/java/net/shibboleth/metadata/dom/saml/GenerateIdStageTest.java b/mda-framework/src/test/java/net/shibboleth/metadata/dom/saml/GenerateIdStageTest.java
index 929fac6..aba30df 100644
--- a/mda-framework/src/test/java/net/shibboleth/metadata/dom/saml/GenerateIdStageTest.java
+++ b/mda-framework/src/test/java/net/shibboleth/metadata/dom/saml/GenerateIdStageTest.java
@@ -27,18 +27,18 @@ import java.util.regex.Pattern;
import javax.xml.namespace.QName;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.dom.DOMElementItem;
import net.shibboleth.metadata.dom.testing.BaseDOMTest;
import net.shibboleth.metadata.util.FixedStringIdentifierGenerationStrategy;
-import net.shibboleth.shared.xml.impl.BasicParserPool;
import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.xml.ElementSupport;
-
-import org.testng.Assert;
-import org.testng.annotations.Test;
-import org.w3c.dom.Document;
-import org.w3c.dom.Element;
+import net.shibboleth.shared.xml.impl.BasicParserPool;
public class GenerateIdStageTest extends BaseDOMTest {
@@ -104,6 +104,7 @@ public class GenerateIdStageTest extends BaseDOMTest {
public void testExplicitConstructor() throws Exception {
final var strat = new FixedStringIdentifierGenerationStrategy("hello");
+ @SuppressWarnings("removal")
final var stage = new GenerateIdStage(strat);
stage.setId("test");
stage.initialize();
@@ -121,8 +122,9 @@ public class GenerateIdStageTest extends BaseDOMTest {
public void testNotSAMLEntity() throws Exception {
final var strat = new FixedStringIdentifierGenerationStrategy("hello");
- final var stage = new GenerateIdStage(strat);
+ final var stage = new GenerateIdStage();
stage.setId("test");
+ stage.setGenerator(strat);
stage.initialize();
final var item = readDOMItem("notentity.xml");
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list