[java-identity-provider] branch master updated: Refactor guages to allow better handling of late initializing services.
Rod Widdowson
rdw at steadingsoftware.com
Tue Jun 18 08:42:20 EDT 2019
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch master
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=57399da1ffacfad1260da1c5692ca26841af9f87
The following commit(s) were added to refs/heads/master by this push:
new 57399da Refactor guages to allow better handling of late initializing services.
57399da is described below
commit 57399da1ffacfad1260da1c5692ca26841af9f87
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Tue Jun 18 13:40:50 2019 +0100
Refactor guages to allow better handling of late initializing services.
MetadataResolverServiceGaugeSet also refactors to make us of Java 8 features
so as to make the checking code slightly less cumbersome.
---
.../impl/AttributeResolverServiceGaugeSet.java | 23 +++-
.../idp/metrics/ReloadableServiceGaugeSet.java | 17 ++-
.../impl/MetadataResolverServiceGaugeSet.java | 148 +++++++++++----------
3 files changed, 110 insertions(+), 78 deletions(-)
diff --git a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverServiceGaugeSet.java b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverServiceGaugeSet.java
index b70a1b8..1494b93 100644
--- a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverServiceGaugeSet.java
+++ b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverServiceGaugeSet.java
@@ -22,6 +22,9 @@ import java.util.Map;
import javax.annotation.Nonnull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import com.codahale.metrics.Gauge;
import com.codahale.metrics.MetricFilter;
import com.codahale.metrics.MetricRegistry;
@@ -42,6 +45,9 @@ import net.shibboleth.utilities.java.support.service.ServiceableComponent;
*/
public class AttributeResolverServiceGaugeSet extends ReloadableServiceGaugeSet implements MetricSet, MetricFilter {
+ /** Class logger. */
+ @Nonnull private final Logger log = LoggerFactory.getLogger(AttributeResolverServiceGaugeSet.class);
+
/**
* Constructor.
*
@@ -61,7 +67,7 @@ public class AttributeResolverServiceGaugeSet extends ReloadableServiceGaugeSet
getService().getServiceableComponent();
if (component != null) {
try {
- final AttributeResolver resolver = component.getComponent();
+ final Object resolver = component.getComponent();
if (resolver instanceof AttributeResolverImpl) {
final Collection<DataConnector> connectors =
((AttributeResolverImpl) resolver).getDataConnectors().values();
@@ -70,6 +76,13 @@ public class AttributeResolverServiceGaugeSet extends ReloadableServiceGaugeSet
mapBuilder.put(connector.getId(), connector.getLastFail());
}
}
+ } else if (resolver instanceof AttributeResolver) {
+ log.debug("{} : Cannot get Data Connector failure " +
+ " information from unsupported class type {}",
+ getLogPrefix(), resolver.getClass());
+ } else {
+ log.warn("{} : Injected Service was not for an AttributeResolver ({})",
+ getLogPrefix(), resolver.getClass());
}
} finally {
component.unpinComponent();
@@ -90,15 +103,17 @@ public class AttributeResolverServiceGaugeSet extends ReloadableServiceGaugeSet
final ServiceableComponent component = getService().getServiceableComponent();
if (component != null) {
try {
- if (component instanceof AttributeResolver) {
+ if (component.getComponent() instanceof AttributeResolver) {
return;
+ } else {
+ log.error("{} : Injected service was not for an AttributeResolver ({})",
+ getLogPrefix(), component.getClass());
+ throw new ComponentInitializationException("Injected service was not for an AttributeResolver");
}
} finally {
component.unpinComponent();
}
}
-
- throw new ComponentInitializationException("Injected service was null or not an AttributeResolver");
}
}
\ No newline at end of file
diff --git a/idp-core/src/main/java/net/shibboleth/idp/metrics/ReloadableServiceGaugeSet.java b/idp-core/src/main/java/net/shibboleth/idp/metrics/ReloadableServiceGaugeSet.java
index 1abc167..01c3ef4 100644
--- a/idp-core/src/main/java/net/shibboleth/idp/metrics/ReloadableServiceGaugeSet.java
+++ b/idp-core/src/main/java/net/shibboleth/idp/metrics/ReloadableServiceGaugeSet.java
@@ -54,13 +54,16 @@ public class ReloadableServiceGaugeSet extends AbstractInitializableComponent im
/** The service to report on. */
@NonnullAfterInit private ReloadableService service;
+ /** The log Prefix. */
+ @Nonnull @NotEmpty private final String logPrefix;
+
/**
* Constructor.
*
* @param metricName name to include in metric names produced by this set
*/
public ReloadableServiceGaugeSet(@Nonnull @NotEmpty @ParameterName(name="metricName") final String metricName) {
- Constraint.isNotEmpty(metricName, "Metric name cannot be null or empty");
+ logPrefix = Constraint.isNotEmpty(metricName, "Metric name cannot be null or empty");
gauges = new HashMap<>();
@@ -113,11 +116,10 @@ public class ReloadableServiceGaugeSet extends AbstractInitializableComponent im
/** {@inheritDoc} */
@Override
protected void doInitialize() throws ComponentInitializationException {
- super.doInitialize();
-
if (service == null) {
- throw new ComponentInitializationException("ReloadableService cannot be null");
+ throw new ComponentInitializationException("Injected ReloadableService cannot be null");
}
+ super.doInitialize();
}
/** {@inheritDoc} */
@@ -138,4 +140,11 @@ public class ReloadableServiceGaugeSet extends AbstractInitializableComponent im
@Nonnull @NonnullElements @Live protected Map<String,Metric> getMetricMap() {
return gauges;
}
+
+ /** Get the log prefix.
+ * @return the log prefix (usually the metric name).
+ */
+ @Nonnull @NotEmpty protected final String getLogPrefix() {
+ return logPrefix;
+ }
}
\ No newline at end of file
diff --git a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/MetadataResolverServiceGaugeSet.java b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/MetadataResolverServiceGaugeSet.java
index 5af8950..848f0ed 100644
--- a/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/MetadataResolverServiceGaugeSet.java
+++ b/idp-saml-impl/src/main/java/net/shibboleth/idp/saml/metadata/impl/MetadataResolverServiceGaugeSet.java
@@ -19,6 +19,7 @@ package net.shibboleth.idp.saml.metadata.impl;
import java.time.Instant;
import java.util.Collections;
import java.util.Map;
+import java.util.function.BiConsumer;
import javax.annotation.Nonnull;
@@ -26,6 +27,8 @@ import org.opensaml.saml.metadata.resolver.BatchMetadataResolver;
import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver;
import org.opensaml.saml.metadata.resolver.MetadataResolver;
import org.opensaml.saml.metadata.resolver.RefreshableMetadataResolver;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import com.codahale.metrics.Gauge;
import com.codahale.metrics.MetricFilter;
@@ -47,36 +50,31 @@ import net.shibboleth.utilities.java.support.service.ServiceableComponent;
*/
public class MetadataResolverServiceGaugeSet extends ReloadableServiceGaugeSet implements MetricSet, MetricFilter {
+ /** Class logger. */
+ @Nonnull private final Logger log = LoggerFactory.getLogger(MetadataResolverServiceGaugeSet.class);
+
/**
* Constructor.
*
* @param metricName name to include in metric names produced by this set
*/
- // Checkstyle: MethodLength OFF
public MetadataResolverServiceGaugeSet(
@Nonnull @NotEmpty @ParameterName(name="metricName") final String metricName) {
super(metricName);
-
+
getMetricMap().put(
MetricRegistry.name(DEFAULT_METRIC_NAME, metricName, "update"),
new Gauge<Map<String,Instant>>() {
public Map<String,Instant> getValue() {
- final Builder mapBuilder = ImmutableMap.<String,Instant>builder();
- final ServiceableComponent<MetadataResolver> component = getService().getServiceableComponent();
- if (component != null) {
- try {
- for (final MetadataResolver resolver : getMetadataResolvers(component.getComponent())) {
- if (resolver instanceof RefreshableMetadataResolver
- && ((RefreshableMetadataResolver) resolver).getLastUpdate() != null) {
- mapBuilder.put(resolver.getId(),
- ((RefreshableMetadataResolver) resolver).getLastUpdate());
- }
+ return valueGetter(new BiConsumer<Builder, MetadataResolver>() {
+ public void accept(final Builder mapBuilder, final MetadataResolver resolver) {
+ if (resolver instanceof RefreshableMetadataResolver
+ && ((RefreshableMetadataResolver) resolver).getLastUpdate() != null) {
+ mapBuilder.put(resolver.getId(),
+ ((RefreshableMetadataResolver) resolver).getLastUpdate());
}
- } finally {
- component.unpinComponent();
- }
- }
- return mapBuilder.build();
+ };
+ });
}
});
@@ -84,77 +82,82 @@ public class MetadataResolverServiceGaugeSet extends ReloadableServiceGaugeSet i
MetricRegistry.name(DEFAULT_METRIC_NAME, metricName, "refresh"),
new Gauge<Map<String,Instant>>() {
public Map<String,Instant> getValue() {
- final Builder mapBuilder = ImmutableMap.<String,Instant>builder();
- final ServiceableComponent<MetadataResolver> component = getService().getServiceableComponent();
- if (component != null) {
- try {
- for (final MetadataResolver resolver : getMetadataResolvers(component.getComponent())) {
- if (resolver instanceof RefreshableMetadataResolver
- && ((RefreshableMetadataResolver) resolver).getLastRefresh() != null) {
- mapBuilder.put(resolver.getId(),
- ((RefreshableMetadataResolver) resolver).getLastRefresh());
- }
+ return valueGetter(new BiConsumer<Builder, MetadataResolver>() {
+ public void accept(final Builder mapBuilder, final MetadataResolver resolver) {
+ if (resolver instanceof RefreshableMetadataResolver
+ && ((RefreshableMetadataResolver) resolver).getLastRefresh() != null) {
+ mapBuilder.put(resolver.getId(),
+ ((RefreshableMetadataResolver) resolver).getLastRefresh());
}
- } finally {
- component.unpinComponent();
- }
- }
- return mapBuilder.build();
+ };
+ });
}
});
-
+
//TODO v4.0.0 - Switch to use RefreshableMetadataResolver when new methods promoted up
- // Checkstyle: AnonInnerLength OFF
getMetricMap().put(
MetricRegistry.name(DEFAULT_METRIC_NAME, metricName, "successfulRefresh"),
new Gauge<Map<String,Instant>>() {
public Map<String,Instant> getValue() {
- final Builder mapBuilder = ImmutableMap.<String,Instant>builder();
- final ServiceableComponent<MetadataResolver> component = getService().getServiceableComponent();
- if (component != null) {
- try {
- for (final MetadataResolver resolver : getMetadataResolvers(component.getComponent())) {
- if (resolver instanceof RefreshableMetadataResolver
- && ((RefreshableMetadataResolver) resolver)
- .getLastSuccessfulRefresh() != null) {
- mapBuilder.put(resolver.getId(),
- ((RefreshableMetadataResolver) resolver).getLastSuccessfulRefresh());
- }
+ return valueGetter(new BiConsumer<Builder, MetadataResolver>() {
+ public void accept(final Builder mapBuilder, final MetadataResolver resolver) {
+ if (resolver instanceof RefreshableMetadataResolver
+ && ((RefreshableMetadataResolver) resolver)
+ .getLastSuccessfulRefresh() != null) {
+ mapBuilder.put(resolver.getId(),
+ ((RefreshableMetadataResolver) resolver).getLastSuccessfulRefresh());
}
- } finally {
- component.unpinComponent();
- }
- }
- return mapBuilder.build();
+ };
+ });
}
});
- // Checkstyle: AnonInnerLength ON
//TODO v4.0.0 - Switch to use BatchMetadataResolver when new methods promoted up
getMetricMap().put(
MetricRegistry.name(DEFAULT_METRIC_NAME, metricName, "rootValidUntil"),
new Gauge<Map<String,Instant>>() {
public Map<String,Instant> getValue() {
- final Builder mapBuilder = ImmutableMap.<String,Instant>builder();
- final ServiceableComponent<MetadataResolver> component = getService().getServiceableComponent();
- if (component != null) {
- try {
- for (final MetadataResolver resolver : getMetadataResolvers(component.getComponent())) {
- if (resolver instanceof BatchMetadataResolver
- && ((BatchMetadataResolver) resolver).getRootValidUntil() != null) {
- mapBuilder.put(resolver.getId(),
- ((BatchMetadataResolver) resolver).getRootValidUntil());
- }
+ return valueGetter(new BiConsumer<Builder, MetadataResolver>() {
+ public void accept(final Builder mapBuilder, final MetadataResolver resolver) {
+ if (resolver instanceof BatchMetadataResolver
+ && ((BatchMetadataResolver) resolver).getRootValidUntil() != null) {
+ mapBuilder.put(resolver.getId(),
+ ((BatchMetadataResolver) resolver).getRootValidUntil());
}
- } finally {
- component.unpinComponent();
- }
- }
- return mapBuilder.build();
+ };
+ });
}
});
}
- // Checkstyle: MethodLength ON
+
+ /** Helper Function for map construction.<br/>
+ *
+ * This does all the service handling and just calls the specific {@link BiConsumer} to
+ * add each appropriate the value to the map.
+ * @param consume the thing which does checking and adding the building
+ * @return an appropriate map
+ */
+ private Map<String,Instant> valueGetter(final BiConsumer<Builder, MetadataResolver> consume) {
+ final Builder mapBuilder = ImmutableMap.<String,Instant>builder();
+ final ServiceableComponent<MetadataResolver> component = getService().getServiceableComponent();
+ if (component != null) {
+ try {
+ // Check type - just in case
+ if (!(component.getComponent() instanceof MetadataResolver)) {
+ log.warn("{} : Injected Service was not for an Metadata Resolver : ({}) ",
+ getLogPrefix(), component.getComponent().getClass());
+ } else {
+ for (final MetadataResolver resolver : getMetadataResolvers(component.getComponent())) {
+ consume.accept(mapBuilder, resolver);
+ }
+ }
+ } finally {
+ component.unpinComponent();
+ }
+ }
+ return mapBuilder.build();
+ }
+
/** {@inheritDoc} */
@Override
@@ -164,15 +167,20 @@ public class MetadataResolverServiceGaugeSet extends ReloadableServiceGaugeSet i
final ServiceableComponent component = getService().getServiceableComponent();
if (component != null) {
try {
- if (component instanceof MetadataResolver) {
+ if (component.getComponent() instanceof MetadataResolver) {
return;
+ } else {
+ log.error("{} : Injected service was not for a MetadataResolver ({}) ",
+ getLogPrefix(), component.getClass());
+ throw new ComponentInitializationException("Injected service was not for a MetadataResolver");
}
} finally {
component.unpinComponent();
}
+ } else {
+ log.debug("{} : Injected service has not initialized sucessfully yet. Skipping type test",
+ getLogPrefix());
}
-
- throw new ComponentInitializationException("Injected service was null or not a MetadataResolver");
}
/**
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list