[java-opensaml] branch master updated: OSJ-249: Enhance FilesystemLoadSaveManager to check file last ...
Brent Putman
putmanb at georgetown.edu
Thu Sep 27 20:22:50 EDT 2018
This is an automated email from the git hooks/post-receive script.
putmanb pushed a commit to branch master
in repository java-opensaml.
View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=2e5a674041bde75515a858de63d4f5d8df2ac350
The following commit(s) were added to refs/heads/master by this push:
new 2e5a674 OSJ-249: Enhance FilesystemLoadSaveManager to check file last ...
2e5a674 is described below
commit 2e5a674041bde75515a858de63d4f5d8df2ac350
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Thu Sep 27 20:03:45 2018 -0400
OSJ-249: Enhance FilesystemLoadSaveManager to check file last ...
Fix regression. LocalDynamicMetadataResolver needs to be able to tell
its XMLObjectLoadSaveManger to clear the lastModified tracking data
when it removes an entity from its internal cache, so subsequent
resolve calls for entityID will re-resolve from the source manager.
---
...actConditionalLoadXMLObjectLoadSaveManager.java | 115 ++++++++++++
.../ConditionalLoadXMLObjectLoadSaveManager.java | 78 ++++++++
.../xml/persist/FilesystemLoadSaveManager.java | 106 ++---------
.../core/xml/persist/MapLoadSaveManager.java | 55 +++++-
.../xml/persist/FilesystemLoadSaveManagerTest.java | 22 +--
...anagerTest.java => MapLoadSaveManagerTest.java} | 202 +++------------------
.../impl/LocalDynamicMetadataResolver.java | 14 ++
.../impl/LocalDynamicMetadataResolverTest.java | 25 +++
8 files changed, 333 insertions(+), 284 deletions(-)
diff --git a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/AbstractConditionalLoadXMLObjectLoadSaveManager.java b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/AbstractConditionalLoadXMLObjectLoadSaveManager.java
new file mode 100644
index 0000000..bca03f7
--- /dev/null
+++ b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/AbstractConditionalLoadXMLObjectLoadSaveManager.java
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You under the Apache
+ * License, Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.opensaml.core.xml.persist;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.core.xml.XMLObject;
+
+/**
+ * Abstract base class for {@link XMLObjectLoadSaveManager} implementations which
+ * track the modify times of requested data such that {@link #load(String)} returns
+ * data only if the data associated with the key has been modified since the last
+ * request.
+ *
+ * @param <T> the base type of XML objects being managed
+ */
+public abstract class AbstractConditionalLoadXMLObjectLoadSaveManager<T extends XMLObject>
+ implements ConditionalLoadXMLObjectLoadSaveManager<T> {
+
+ /** Configuration flag for whether {@link #load(String)} will check and return data only if modified
+ * since the last request for that data. */
+ private boolean loadConditionally;
+
+ /** Storage for last modified time of requested data. */
+ private Map<String, Long> loadLastModified;
+
+ /** Constructor. */
+ protected AbstractConditionalLoadXMLObjectLoadSaveManager() {
+ loadLastModified = new HashMap<>();
+ }
+
+ /** {@inheritDoc} */
+ public boolean isLoadConditionally() {
+ return loadConditionally;
+ }
+
+ /** {@inheritDoc} */
+ public void setLoadConditionally(final boolean flag) {
+ loadConditionally = flag;
+ }
+
+ /** {@inheritDoc} */
+ @Nullable public synchronized Long getLoadLastModified(@Nonnull final String key) {
+ return loadLastModified.get(key);
+ }
+
+ /** {@inheritDoc} */
+ @Nullable public synchronized Long clearLoadLastModified(@Nonnull final String key) {
+ final Long prev = loadLastModified.get(key);
+ loadLastModified.remove(key);
+ return prev;
+ }
+
+ /** {@inheritDoc} */
+ public void clearAllLoadLastModified() {
+ loadLastModified.clear();
+ }
+
+ /**
+ * Update the cached modified time for the specified key with the current time.
+ *
+ * @param key the target key
+ * @return the previously cached modified time, or null if did not exist
+ */
+ protected synchronized Long updateLoadLastModified(@Nonnull final String key) {
+ return updateLoadLastModified(key, System.currentTimeMillis());
+ }
+
+ /**
+ * Update the cached modified time for the specified key with the specified time.
+ *
+ * @param key the target key
+ * @param modified the new cached modified time
+ * @return the previously cached modified time, or null if did not exist
+ */
+ protected synchronized Long updateLoadLastModified(@Nonnull final String key, @Nullable final Long modified) {
+ if (modified == null) {
+ return null;
+ }
+ final Long prev = loadLastModified.get(key);
+ loadLastModified.put(key, modified);
+ return prev;
+ }
+
+ /**
+ * Check whether the data corresponding to the specified key has been modified since the last time
+ * {@link #load(String)} was called for that key.
+ *
+ * @param key the data key
+ * @return true if the corresponding data has been modified since the last load, false otherwise
+ * @throws IOException if there is a fatal error evaluating the last modified status
+ */
+ protected abstract boolean isUnmodifiedSinceLastLoad(@Nonnull final String key) throws IOException;
+
+}
diff --git a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/ConditionalLoadXMLObjectLoadSaveManager.java b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/ConditionalLoadXMLObjectLoadSaveManager.java
new file mode 100644
index 0000000..5117e20
--- /dev/null
+++ b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/ConditionalLoadXMLObjectLoadSaveManager.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You under the Apache
+ * License, Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.opensaml.core.xml.persist;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.core.xml.XMLObject;
+
+/**
+ * Interface for specialization of {@link XMLObjectLoadSaveManager} implementations which
+ * track the modify times of requested data such that {@link #load(String)} returns
+ * data only if the data associated with the key has been modified since the last
+ * request.
+ *
+ * @param <T> the base type of XML objects being managed
+ */
+public interface ConditionalLoadXMLObjectLoadSaveManager<T extends XMLObject> extends XMLObjectLoadSaveManager<T> {
+
+ /**
+ * Get the configuration flag for whether {@link #load(String)} will check and return data only if modified
+ * since the last request for that data.
+ *
+ * @return true if data modify time check is enabled, false if not
+ */
+ public boolean isLoadConditionally();
+
+ /**
+ * Set the configuration flag for whether {@link #load(String)} will check and return data only if modified
+ * since the last request for that data.
+ *
+ * @param flag true if data modify time check should be enabled, false if not
+ */
+ public void setLoadConditionally(final boolean flag);
+
+ /**
+ * Retrieve the cached modified time for the last load of the specified key.
+ *
+ * <p>
+ * Note that this will be null if {@link #load(String)} has not been called
+ * for the specified key since construction or since the last call to
+ * {@link #clearLoadLastModified(String)}.
+ * </p>
+ *
+ * @param key the target key
+ * @return the current cached modified time in milliseconds since the epoch, may be null
+ */
+ @Nullable public Long getLoadLastModified(@Nonnull final String key);
+
+ /**
+ * Clear the cached modified time for the last load of the specified key.
+ *
+ * @param key the target key
+ * @return the previously cached modified time in milliseconds since the epoch, or null if did not exist
+ */
+ @Nullable public Long clearLoadLastModified(@Nonnull final String key);
+
+ /**
+ * Clear the cached modified times for the last load for all keys.
+ */
+ public void clearAllLoadLastModified();
+
+}
diff --git a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManager.java b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManager.java
index 62106b8..6dfb9a8 100644
--- a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManager.java
+++ b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManager.java
@@ -25,11 +25,9 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
-import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
@@ -71,7 +69,7 @@ import net.shibboleth.utilities.java.support.xml.XMLParserException;
* @param <T> the specific base XML object type being managed
*/
@NotThreadSafe
-public class FilesystemLoadSaveManager<T extends XMLObject> implements XMLObjectLoadSaveManager<T> {
+public class FilesystemLoadSaveManager<T extends XMLObject> extends AbstractConditionalLoadXMLObjectLoadSaveManager<T> {
/** Logger. */
private Logger log = LoggerFactory.getLogger(FilesystemLoadSaveManager.class);
@@ -85,13 +83,6 @@ public class FilesystemLoadSaveManager<T extends XMLObject> implements XMLObject
/** File filter used in filtering files in {@link #listKeys()} and {@link #listAll()}. */
private FileFilter fileFilter;
- /** Configuration flag for whether {@link #load(String)} will check and return data only if modified
- * since the last request for that data. */
- private boolean checkModifyTime;
-
- /** Storage for last modified time of previously requested files. */
- private Map<String, Long> lastModified;
-
/**
* Constructor.
*
@@ -131,6 +122,7 @@ public class FilesystemLoadSaveManager<T extends XMLObject> implements XMLObject
* @param pp the parser pool instance to use
*/
public FilesystemLoadSaveManager(@Nonnull final File baseDir, @Nullable final ParserPool pp) {
+ super();
baseDirectory = Constraint.isNotNull(baseDir, "Base directory File instance was null");
Constraint.isTrue(baseDirectory.isAbsolute(), "Base directory specified was not an absolute path");
if (baseDirectory.exists()) {
@@ -146,28 +138,6 @@ public class FilesystemLoadSaveManager<T extends XMLObject> implements XMLObject
}
fileFilter = new DefaultFileFilter();
-
- lastModified = new HashMap<>();
- }
-
- /**
- * Get the configuration flag for whether {@link #load(String)} will check and return data only if modified
- * since the last request for that data.
- *
- * @return true if file modify time check is enabled, false if not
- */
- public boolean isCheckModifyTime() {
- return checkModifyTime;
- }
-
- /**
- * Set the configuration flag for whether {@link #load(String)} will check and return data only if modified
- * since the last request for that data.
- *
- * @param flag true if file modify time check should be enabled, false if not
- */
- public void setCheckModifyTime(final boolean flag) {
- checkModifyTime = flag;
}
/** {@inheritDoc} */
@@ -195,10 +165,10 @@ public class FilesystemLoadSaveManager<T extends XMLObject> implements XMLObject
final File file = buildFile(key);
if (!file.exists()) {
log.debug("Target file with key '{}' does not exist, path: {}", key, file.getAbsolutePath());
- clearCachedModified(key);
+ clearLoadLastModified(key);
return null;
}
- if (isCheckModifyTime() && isUnmodifiedSinceLastRequest(key)) {
+ if (isLoadConditionally() && isUnmodifiedSinceLastLoad(key)) {
log.debug("Target file with key '{}' has not been modified since the last request, returning null: {}",
key, file.getAbsolutePath());
return null;
@@ -208,7 +178,7 @@ public class FilesystemLoadSaveManager<T extends XMLObject> implements XMLObject
try (final ByteArrayInputStream bais = new ByteArrayInputStream(source)) {
final XMLObject xmlObject = XMLObjectSupport.unmarshallFromInputStream(parserPool, bais);
xmlObject.getObjectMetadata().put(new XMLObjectSource(source));
- updateCachedModified(key, file.lastModified());
+ updateLoadLastModified(key, file.lastModified());
//TODO via ctor, etc, does caller need to supply a Class so we can can test and throw an IOException,
// rather than an unchecked ClassCastException?
return (T) xmlObject;
@@ -218,62 +188,12 @@ public class FilesystemLoadSaveManager<T extends XMLObject> implements XMLObject
}
}
- /**
- * Check whether the file corresponding to the specified key has been modified since the last time it
- * was requested.
- *
- * @param key the file key
- * @return true if the corresponding file has been modified since the last request for it, false otherwise
- * @throws IOException if there is a fatal error constructing or evaluating the candidate target path
- */
- protected synchronized boolean isUnmodifiedSinceLastRequest(@Nonnull final String key) throws IOException {
+ /** {@inheritDoc} */
+ protected synchronized boolean isUnmodifiedSinceLastLoad(@Nonnull final String key) throws IOException {
final File file = buildFile(key);
- log.trace("File '{}' last modified was: {}", file.getAbsolutePath(), file.lastModified());
- return getCachedModified(key) != null && file.lastModified() <= getCachedModified(key);
- }
-
- /**
- * Retrieve the current cached modified time for the specified key.
- * @param key the target key
- * @return the current cached modified time, may be null
- */
- protected synchronized Long getCachedModified(@Nonnull final String key) {
- return lastModified.get(key);
- }
-
- /**
- * Update the cached modified time for the specified key with the current time.
- * @param key the target key
- * @return the previously cached modified time, or null if did not exist
- */
- protected synchronized Long updateCachedModified(@Nonnull final String key) {
- return updateCachedModified(key, System.currentTimeMillis());
- }
-
- /**
- * Update the cached modified time for the specified key with the specified time.
- * @param key the target key
- * @param modified the new cached modified time
- * @return the previously cached modified time, or null if did not exist
- */
- protected synchronized Long updateCachedModified(@Nonnull final String key, @Nullable final Long modified) {
- if (modified == null) {
- return null;
- }
- final Long prev = lastModified.get(key);
- lastModified.put(key, modified);
- return prev;
- }
-
- /**
- * Clear the current cached modified time for the specified key.
- * @param key the target key
- * @return the previously cached modified time, or null if did not exist
- */
- protected synchronized Long clearCachedModified(@Nonnull final String key) {
- final Long prev = lastModified.get(key);
- lastModified.remove(key);
- return prev;
+ final long lastModified = file.lastModified();
+ log.trace("File '{}' last modified was: {}", file.getAbsolutePath(), lastModified);
+ return getLoadLastModified(key) != null && lastModified <= getLoadLastModified(key);
}
/** {@inheritDoc} */
@@ -315,7 +235,7 @@ public class FilesystemLoadSaveManager<T extends XMLObject> implements XMLObject
if (file.exists()) {
final boolean success = file.delete();
if (success) {
- clearCachedModified(key);
+ clearLoadLastModified(key);
return true;
} else {
throw new IOException(String.format("Error removing target file: %s", file.getAbsolutePath()));
@@ -337,8 +257,8 @@ public class FilesystemLoadSaveManager<T extends XMLObject> implements XMLObject
throw new IOException(String.format("Specified new key already exists: %s", newKey));
} else {
Files.move(currentFile, newFile);
- updateCachedModified(newKey, getCachedModified(currentKey));
- clearCachedModified(currentKey);
+ updateLoadLastModified(newKey, getLoadLastModified(currentKey));
+ clearLoadLastModified(currentKey);
return true;
}
}
diff --git a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/MapLoadSaveManager.java b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/MapLoadSaveManager.java
index 81149cc..b1fe17e 100644
--- a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/MapLoadSaveManager.java
+++ b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/MapLoadSaveManager.java
@@ -27,6 +27,8 @@ import javax.annotation.Nonnull;
import javax.annotation.concurrent.NotThreadSafe;
import org.opensaml.core.xml.XMLObject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import net.shibboleth.utilities.java.support.collection.Pair;
import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -37,11 +39,17 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
* @param <T> the specific base XML object type being managed
*/
@NotThreadSafe
-public class MapLoadSaveManager<T extends XMLObject> implements XMLObjectLoadSaveManager<T> {
+public class MapLoadSaveManager<T extends XMLObject> extends AbstractConditionalLoadXMLObjectLoadSaveManager<T> {
+
+ /** Logger. */
+ private Logger log = LoggerFactory.getLogger(MapLoadSaveManager.class);
/** The backing map. */
private Map<String,T> backingMap;
+ /** Storage to track last modified time of data. */
+ private Map<String,Long> dataLastModified;
+
/** Constructor. */
public MapLoadSaveManager() {
this(new HashMap<String,T>());
@@ -54,6 +62,7 @@ public class MapLoadSaveManager<T extends XMLObject> implements XMLObjectLoadSav
*/
public MapLoadSaveManager(@Nonnull final Map<String, T> map) {
backingMap = Constraint.isNotNull(map, "Backing map was null");
+ dataLastModified = new HashMap<>();
}
/** {@inheritDoc} */
@@ -77,6 +86,16 @@ public class MapLoadSaveManager<T extends XMLObject> implements XMLObjectLoadSav
/** {@inheritDoc} */
public T load(final String key) throws IOException {
+ if (!exists(key)) {
+ log.debug("Target data with key '{}' does not exist", key);
+ clearLoadLastModified(key);
+ return null;
+ }
+ if (isLoadConditionally() && isUnmodifiedSinceLastLoad(key)) {
+ log.debug("Target data with key '{}' has not been modified since the last request, returning null", key);
+ return null;
+ }
+ updateLoadLastModified(key, dataLastModified.get(key));
return backingMap.get(key);
}
@@ -91,24 +110,44 @@ public class MapLoadSaveManager<T extends XMLObject> implements XMLObjectLoadSav
throw new IOException(String.format("Value already exists for key '%s'", key));
} else {
backingMap.put(key, xmlObject);
+ dataLastModified.put(key, System.currentTimeMillis());
}
}
/** {@inheritDoc} */
public boolean remove(final String key) throws IOException {
- return backingMap.remove(key) != null;
+ final T removed = backingMap.remove(key);
+ dataLastModified.remove(key);
+ clearLoadLastModified(key);
+ return removed != null;
}
/** {@inheritDoc} */
public boolean updateKey(final String currentKey, final String newKey) throws IOException {
- final T value = load(currentKey);
- if (value != null) {
- save(newKey, value, false);
- remove(currentKey);
- return true;
- } else {
+ final T value = backingMap.get(currentKey);
+ if (value == null) {
return false;
}
+ if (backingMap.containsKey(newKey)) {
+ throw new IOException(String.format("Specified new key already exists: %s", newKey));
+ } else {
+ backingMap.put(newKey, value);
+ backingMap.remove(currentKey);
+
+ dataLastModified.put(newKey, dataLastModified.get(currentKey));
+ dataLastModified.remove(currentKey);
+
+ updateLoadLastModified(newKey, getLoadLastModified(currentKey));
+ clearLoadLastModified(currentKey);
+ return true;
+ }
+ }
+
+ /** {@inheritDoc} */
+ protected boolean isUnmodifiedSinceLastLoad(@Nonnull final String key) throws IOException {
+ final Long lastModified = dataLastModified.get(key);
+ log.trace("Key '{}' last modified was: {}", key, lastModified);
+ return getLoadLastModified(key) != null && lastModified != null && lastModified <= getLoadLastModified(key);
}
}
diff --git a/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java b/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java
index 3deea5e..3159b7d 100644
--- a/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java
+++ b/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java
@@ -125,7 +125,7 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
// Can't update to an existing name
try {
manager.updateKey("bar", "baz");
- Assert.fail("updateKey should have filed to due existing new key name");
+ Assert.fail("updateKey should have failed to due existing new key name");
} catch (IOException e) {
// expected, do nothing
}
@@ -145,20 +145,20 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
@Test
public void checkCheckModifyTimeTracking() throws IOException {
- manager.setCheckModifyTime(true);
+ manager.setLoadConditionally(true);
Assert.assertNull(manager.load("foo"));
- Assert.assertNull(manager.getCachedModified("foo"));
+ Assert.assertNull(manager.getLoadLastModified("foo"));
manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, true));
Assert.assertNotNull(manager.load("foo"));
- Long initialCachedModified = manager.getCachedModified("foo");
+ Long initialCachedModified = manager.getLoadLastModified("foo");
Assert.assertNotNull(initialCachedModified);
// Hasn't changed
Assert.assertNull(manager.load("foo"));
- Assert.assertEquals(manager.getCachedModified("foo"), initialCachedModified);
+ Assert.assertEquals(manager.getLoadLastModified("foo"), initialCachedModified);
// We have to sleep a little to get an updated timestamp when we save a new one,
// since filesystem mtime granularity is only seconds.
@@ -168,25 +168,25 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, true), true);
Assert.assertNotNull(manager.load("foo"));
- Long updatedCachedModified = manager.getCachedModified("foo");
+ Long updatedCachedModified = manager.getLoadLastModified("foo");
Assert.assertNotNull(updatedCachedModified);
Assert.assertNotEquals(updatedCachedModified, initialCachedModified);
// Hasn't changed (again)
Assert.assertNull(manager.load("foo"));
- Assert.assertEquals(manager.getCachedModified("foo"), updatedCachedModified);
+ Assert.assertEquals(manager.getLoadLastModified("foo"), updatedCachedModified);
// Test update of key
manager.updateKey("foo", "bar");
Assert.assertNull(manager.load("foo"));
Assert.assertNull(manager.load("bar"));
- Assert.assertNull(manager.getCachedModified("foo"));
- Assert.assertNotNull(manager.getCachedModified("bar"));
- Assert.assertEquals(manager.getCachedModified("bar"), updatedCachedModified);
+ Assert.assertNull(manager.getLoadLastModified("foo"));
+ Assert.assertNotNull(manager.getLoadLastModified("bar"));
+ Assert.assertEquals(manager.getLoadLastModified("bar"), updatedCachedModified);
// Test removal of key
manager.remove("bar");
- Assert.assertNull(manager.getCachedModified("bar"));
+ Assert.assertNull(manager.getLoadLastModified("bar"));
}
@Test
diff --git a/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java b/opensaml-core/src/test/java/org/opensaml/core/xml/persist/MapLoadSaveManagerTest.java
similarity index 53%
copy from opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java
copy to opensaml-core/src/test/java/org/opensaml/core/xml/persist/MapLoadSaveManagerTest.java
index 3deea5e..022abba 100644
--- a/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java
+++ b/opensaml-core/src/test/java/org/opensaml/core/xml/persist/MapLoadSaveManagerTest.java
@@ -17,95 +17,60 @@
package org.opensaml.core.xml.persist;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
import java.io.IOException;
-import java.nio.file.Files;
import java.util.Iterator;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.concurrent.TimeUnit;
-import javax.xml.namespace.QName;
-
-import org.opensaml.core.xml.XMLObject;
import org.opensaml.core.xml.XMLObjectBaseTestCase;
-import org.opensaml.core.xml.XMLRuntimeException;
-import org.opensaml.core.xml.io.MarshallingException;
import org.opensaml.core.xml.mock.SimpleXMLObject;
-import org.opensaml.core.xml.util.XMLObjectSource;
-import org.opensaml.core.xml.util.XMLObjectSupport;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import org.testng.Assert;
-import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.Uninterruptibles;
import net.shibboleth.utilities.java.support.collection.Pair;
-import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
-public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
-
- private Logger log = LoggerFactory.getLogger(FilesystemLoadSaveManagerTest.class);
-
- private File baseDir;
+/**
+ *
+ */
+public class MapLoadSaveManagerTest extends XMLObjectBaseTestCase {
- private FilesystemLoadSaveManager<SimpleXMLObject> manager;
+ private MapLoadSaveManager<SimpleXMLObject> manager;
@BeforeMethod
- public void setUp() throws IOException {
- baseDir = new File(System.getProperty("java.io.tmpdir"), "load-save-manager-test");
- baseDir.deleteOnExit();
- log.debug("Using base directory: {}", baseDir.getAbsolutePath());
- resetBaseDir();
- Assert.assertTrue(baseDir.mkdirs());
-
- manager = new FilesystemLoadSaveManager<>(baseDir);
- }
-
- @AfterMethod
- public void tearDown() throws IOException {
- resetBaseDir();
+ public void setup() {
+ manager = new MapLoadSaveManager<>();
}
@Test
- public void emptyDir() throws IOException {
+ public void emptyMap() throws IOException {
testState(Sets.<String>newHashSet());
}
- @DataProvider
- public Object[][] saveLoadUpdateRemoveParams() {
- return new Object[][] {
- new Object[] { Boolean.FALSE},
- new Object[] { Boolean.TRUE },
- };
- }
-
- @Test(dataProvider="saveLoadUpdateRemoveParams")
- public void saveLoadUpdateRemove(Boolean buildWithObjectSourceByteArray) throws IOException {
+ @Test
+ public void saveLoadUpdateRemove() throws IOException {
testState(Sets.<String>newHashSet());
Assert.assertNull(manager.load("bogus"));
- manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, buildWithObjectSourceByteArray));
+ manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME));
testState(Sets.newHashSet("foo"));
- manager.save("bar", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, buildWithObjectSourceByteArray));
- manager.save("baz", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, buildWithObjectSourceByteArray));
+ manager.save("bar", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME));
+ manager.save("baz", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME));
testState(Sets.newHashSet("foo", "bar", "baz"));
// Duplicate with overwrite
- manager.save("bar", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, buildWithObjectSourceByteArray), true);
+ manager.save("bar", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME), true);
testState(Sets.newHashSet("foo", "bar", "baz"));
// Duplicate without overwrite
try {
- manager.save("bar", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, buildWithObjectSourceByteArray), false);
+ manager.save("bar", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME), false);
Assert.fail("Should have failed on duplicate save without overwrite");
} catch (IOException e) {
// expected, do nothing
@@ -125,7 +90,7 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
// Can't update to an existing name
try {
manager.updateKey("bar", "baz");
- Assert.fail("updateKey should have filed to due existing new key name");
+ Assert.fail("updateKey should have failed to due existing new key name");
} catch (IOException e) {
// expected, do nothing
}
@@ -145,118 +110,46 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
@Test
public void checkCheckModifyTimeTracking() throws IOException {
- manager.setCheckModifyTime(true);
+ manager.setLoadConditionally(true);
Assert.assertNull(manager.load("foo"));
- Assert.assertNull(manager.getCachedModified("foo"));
+ Assert.assertNull(manager.getLoadLastModified("foo"));
- manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, true));
+ manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME));
Assert.assertNotNull(manager.load("foo"));
- Long initialCachedModified = manager.getCachedModified("foo");
+ Long initialCachedModified = manager.getLoadLastModified("foo");
Assert.assertNotNull(initialCachedModified);
// Hasn't changed
Assert.assertNull(manager.load("foo"));
- Assert.assertEquals(manager.getCachedModified("foo"), initialCachedModified);
+ Assert.assertEquals(manager.getLoadLastModified("foo"), initialCachedModified);
- // We have to sleep a little to get an updated timestamp when we save a new one,
- // since filesystem mtime granularity is only seconds.
- Uninterruptibles.sleepUninterruptibly(2, TimeUnit.SECONDS);
+ Uninterruptibles.sleepUninterruptibly(1, TimeUnit.SECONDS);
// Change it
- manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, true), true);
+ manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME), true);
Assert.assertNotNull(manager.load("foo"));
- Long updatedCachedModified = manager.getCachedModified("foo");
+ Long updatedCachedModified = manager.getLoadLastModified("foo");
Assert.assertNotNull(updatedCachedModified);
Assert.assertNotEquals(updatedCachedModified, initialCachedModified);
// Hasn't changed (again)
Assert.assertNull(manager.load("foo"));
- Assert.assertEquals(manager.getCachedModified("foo"), updatedCachedModified);
+ Assert.assertEquals(manager.getLoadLastModified("foo"), updatedCachedModified);
// Test update of key
manager.updateKey("foo", "bar");
Assert.assertNull(manager.load("foo"));
Assert.assertNull(manager.load("bar"));
- Assert.assertNull(manager.getCachedModified("foo"));
- Assert.assertNotNull(manager.getCachedModified("bar"));
- Assert.assertEquals(manager.getCachedModified("bar"), updatedCachedModified);
+ Assert.assertNull(manager.getLoadLastModified("foo"));
+ Assert.assertNotNull(manager.getLoadLastModified("bar"));
+ Assert.assertEquals(manager.getLoadLastModified("bar"), updatedCachedModified);
// Test removal of key
manager.remove("bar");
- Assert.assertNull(manager.getCachedModified("bar"));
- }
-
- @Test
- public void buildTargetFileFromKey() throws IOException {
- File target = manager.buildFile("abc");
- Assert.assertEquals(target, new File(baseDir, "abc"));
- }
-
- @Test(expectedExceptions=IOException.class)
- public void targetExistsButIsNotAFile() throws IOException {
- File target = new File(baseDir, "abc");
- Assert.assertFalse(target.exists());
- target.mkdir();
- try {
- manager.buildFile("abc");
- } finally {
- if (target.exists()) {
- Files.delete(target.toPath());
- }
- }
- }
-
- @Test(expectedExceptions=ConstraintViolationException.class)
- public void targetKeyIsNull() throws IOException {
- manager.buildFile(null);
- }
-
- @Test(expectedExceptions=ConstraintViolationException.class)
- public void targetKeyIsEmpty() throws IOException {
- manager.buildFile(" ");
- }
-
- @Test
- public void ctorCreateDirectory() throws IOException {
- resetBaseDir();
- Assert.assertFalse(baseDir.exists());
- new FilesystemLoadSaveManager<>(baseDir);
- Assert.assertTrue(baseDir.exists());
- }
-
- @Test
- public void ctorPathTrimming() throws IOException {
- new FilesystemLoadSaveManager<>(String.format(" %s ", baseDir.getAbsolutePath()));
- File target = manager.buildFile("abc");
- Assert.assertEquals(target.getParentFile(), baseDir);
- Assert.assertEquals(target.getParent(), baseDir.getAbsolutePath());
- Assert.assertFalse(target.getParent().startsWith(" "));
- Assert.assertFalse(target.getParent().endsWith(" "));
- }
-
- @Test(expectedExceptions=ConstraintViolationException.class)
- public void ctorEmptyPathString() {
- new FilesystemLoadSaveManager<>(" ");
- }
-
- @Test(expectedExceptions=ConstraintViolationException.class)
- public void ctorNullFile() {
- new FilesystemLoadSaveManager<>((File)null);
- }
-
- @Test(expectedExceptions=ConstraintViolationException.class)
- public void ctorRelativeDir() {
- new FilesystemLoadSaveManager<>("my/relative/dir");
- }
-
- @Test(expectedExceptions=ConstraintViolationException.class)
- public void ctorBaseDirPathExistsButNotADirectory() throws IOException {
- resetBaseDir();
- Files.createFile(baseDir.toPath());
- new FilesystemLoadSaveManager<>(baseDir);
+ Assert.assertNull(manager.getLoadLastModified("bar"));
}
@Test
@@ -326,18 +219,11 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
//expected, do nothing
}
- // Test when file is removed after iterator is created
- manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME));
- Assert.assertTrue(manager.exists("foo"));
- Assert.assertNotNull(manager.load("foo"));
- iterator = manager.listAll().iterator();
- manager.remove("foo");
- Assert.assertFalse(iterator.hasNext());
-
}
+
// Helpers
private void testState(Set<String> expectedKeys) throws IOException {
@@ -347,7 +233,6 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
Assert.assertTrue(manager.exists(expectedKey));
SimpleXMLObject sxo = manager.load(expectedKey);
Assert.assertNotNull(sxo);
- Assert.assertEquals(sxo.getObjectMetadata().get(XMLObjectSource.class).size(), 1);
}
Assert.assertEquals(manager.listAll().iterator().hasNext(), expectedKeys.isEmpty() ? false: true);
@@ -360,32 +245,5 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
}
Assert.assertEquals(sawCount, expectedKeys.size());
}
-
- private void resetBaseDir() throws IOException {
- if (baseDir.exists()) {
- if (baseDir.isDirectory()) {
- for (File child : baseDir.listFiles()) {
- Files.delete(child.toPath());
- }
- }
- Files.delete(baseDir.toPath());
- }
- }
-
- // It's hard to actually test that we're writing the existing byte[], but by doing this
- // we can at least visually inspect the logs for save() ops and see that it logs as expected.
- protected <T extends XMLObject> T buildXMLObject(QName name, boolean withObjectSource) {
- T xmlObject = super.buildXMLObject(name);
- if (withObjectSource) {
- try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
- XMLObjectSupport.marshallToOutputStream(xmlObject, baos);
- xmlObject.getObjectMetadata().put(new XMLObjectSource(baos.toByteArray()));
- } catch (MarshallingException | IOException e) {
- throw new XMLRuntimeException("Error marshalling XMLObject", e);
- }
- }
- return xmlObject;
- }
-
}
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/LocalDynamicMetadataResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/LocalDynamicMetadataResolver.java
index d1b4722..fca963b 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/LocalDynamicMetadataResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/LocalDynamicMetadataResolver.java
@@ -23,7 +23,9 @@ import java.util.Timer;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import org.opensaml.core.criterion.EntityIdCriterion;
import org.opensaml.core.xml.XMLObject;
+import org.opensaml.core.xml.persist.ConditionalLoadXMLObjectLoadSaveManager;
import org.opensaml.core.xml.persist.XMLObjectLoadSaveManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -99,6 +101,18 @@ public class LocalDynamicMetadataResolver extends AbstractDynamicMetadataResolve
}
/** {@inheritDoc} */
+ protected void removeByEntityID(final String entityID, final EntityBackingStore backingStore) {
+ if (sourceManager instanceof ConditionalLoadXMLObjectLoadSaveManager) {
+ final String key = sourceKeyGenerator.apply(new CriteriaSet(new EntityIdCriterion(entityID)));
+ if (key != null) {
+ ((ConditionalLoadXMLObjectLoadSaveManager)sourceManager).clearLoadLastModified(key);
+ }
+ }
+
+ super.removeByEntityID(entityID, backingStore);
+ }
+
+ /** {@inheritDoc} */
@Override
protected XMLObject fetchFromOriginSource(final CriteriaSet criteria) throws IOException {
final String key = sourceKeyGenerator.apply(criteria);
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/LocalDynamicMetadataResolverTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/LocalDynamicMetadataResolverTest.java
index 5bdbb03..ad3a2bb 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/LocalDynamicMetadataResolverTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/LocalDynamicMetadataResolverTest.java
@@ -121,6 +121,31 @@ public class LocalDynamicMetadataResolverTest extends XMLObjectBaseTestCase {
}
@Test
+ public void testConditionalLoadManagerWithClearEntityID() throws IOException, ResolverException {
+ sourceManager.setLoadConditionally(true);
+
+ sourceManager.save(sha1Digester.apply(entityID1), entity1);
+
+ // This will resolve from source manager directly
+ Assert.assertSame(resolver.resolveSingle(new CriteriaSet(new EntityIdCriterion(entityID1))), entity1);
+
+ // This will be from in-memory cache
+ Assert.assertSame(resolver.resolveSingle(new CriteriaSet(new EntityIdCriterion(entityID1))), entity1);
+
+ // Clear from in-memory cache
+ resolver.clear(entityID1);
+ Assert.assertNull(resolver.getBackingStore().getIndexedDescriptors().get(entityID1));
+ Assert.assertFalse(resolver.getBackingStore().getOrderedDescriptors().contains(entity1));
+
+ // This should re-resolve from source manager directly
+ Assert.assertSame(resolver.resolveSingle(new CriteriaSet(new EntityIdCriterion(entityID1))), entity1);
+
+ // This will be from in-memory cache (again)
+ Assert.assertSame(resolver.resolveSingle(new CriteriaSet(new EntityIdCriterion(entityID1))), entity1);
+
+ }
+
+ @Test
public void testCtorSourceKeyGenerator() throws ComponentInitializationException, IOException, ResolverException {
resolver.destroy();
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list