Qs after reviewing Java dynamic metadata resolvers

Wed Sep 14 14:49:07 EDT 2016

So what I've come up with at this point for the caching support in the
dynamic providers turned out to be a little different than where I was
going originally.  And I think maybe different than what Scott may have
had in mind (I'm just inferring).  But I think the approach I've
devised both works acceptably and has the advantage of being "simple",
so smaller surface area for bugs, etc.

The idea is that the caching for the dynamic case really isn't
fundamentally different than than the caching we do for things like the
file-backed HTTP resolver:

1) at "fetch" time, you write the data to the cache.  (In the batch
ones this is once, at init time.  In the dynamic ones this is multiply,
at request time, once per EntityDescriptor.  But this is not a
significant difference).

2) at init time, you load (all) the cached data to make it "live".

The #2 I thought at first a little controversial.  But the more I
thought about it, this seems not only correct but probably what a
deployer might ideally want:  On a restart you probably want the state
of the new resolver instance to be the same, or as close as possible,
as before the restart.  You pay the loading cost upfront at
init/restart time, rather than paying it incrementally as you get
requests and delaying those requests while you load from the cache.
Basically: 1) you already had it 2) you know it was recently used (see
next paragraph) 3) a fair assumption is that it will be used again, so
4) go ahead and preemptively load it.

An initial concern with the preemptive load was the memory usage.  But
note that the dynamic case is really different than the batch case.  In
the latter, the concern is that you are potentially loading and keeping
very large sets, of which you use perhaps only a fraction.  With
dynamic, everything you've loaded was not only actively used, but used
"recently", as defined by the configuration and the metadata
validUntil, and enforced by the cleanup sweeper thread (which evicts
expired and/or idle metadata).  And the same is true with what is
loaded on restart.  Essentially you're really just going to be using
the same memory that you otherwise would have been using had you not
restarted.  So personally I don't see a problem there.  But let's
discuss more if I'm missing something.

So really, I think the use case of the dynamic caching is really
totally distinct from the "local dynamic" folder o'metadata use case. 
They can reuse some shared supporting code (see below), but they don't
fundamentally have anything to do with one another.

The load/save component interface was also written in such a way that
it is not specifically file-based.  So for example you could plug in a
JDBC or JPA-based one.  And so for the other type of concrete impl of
"local dynamic", if you had the latter, you could transparently support
dynamic resolution from a database also.

Of course let's discuss on Friday's call.

I've already provisionally implemented the dynamic resolver bits, just
working now on unit tests and the load/save manager impl (below).

More details below.

On 9/1/16 9:40 PM, Cantor, Scott wrote:
> On 9/1/16, 8:55 PM, "dev on behalf of Brent Putman" <dev-bounces at shibboleth.net on behalf of putmanb at georgetown.edu> wrote:
>
>> That's basically what I had in my mind to do.   The code to save and load metadata to/from
>> a file is not complicated.
> It's not, but synchronization is always subtle and I was looking for the best way to keep it factored out and not have to clutter all the core code with all that logic, which is something that I don't like about the C++ version of all this.

Given the approach above, I don't know if there is any (additional)
synchronization to do here.  The save ops are done under a write lock
over the entityID, from which the cache key is derived. Since resolvers
should not generally share a cache, there doesn't seem to be a need to
do explicit file (or database) locking at the load/save manager impl
level.  So I was thinking that the standard default file-based impl
wouldn't need locking, and therefore could be more performant. But I
suppose such locking could be an impl detail and we could do such a
locking impl at some point if necessary.

The same is also true of the propose concrete "local dynamic" impls. 
The load op is done under the same write lock over the entityID, and
therefore transitively under the cache key.

>
> Also, I was thinking about the work involved for the deployer in having to chain things, and the overlap of having multiple resolvers managing a backing store in each one for essentially the same data. It just seemed a little ugly.

Re-reading this, I realized I didn't really understand what you meant
here, what the use case was, etc.

>
>> If your reasoning is that you want to factor
>> out the file processing code so it can be reused for the separate use cases of the
>> backup files *and* the true file-based dynamic resolver - I think we could achieve the
>> same thing via either support classes, or even some sort of file load/save interface+impl
>> component that is plugged into both the AbstractDynamicMetadataResolver for the backing
>> file use, and the concrete file-based dynamic resolver.
> Yes, I am wanting to share that code.

So the shared code I have is for an interface for an XMLObject
load/save manager.  It is based on the (simple) idea of loading and
storing an XMLObject indexed under a String key.  The methods are the
obvious CRUD ones, including iteration of both keys and the managed
entries themselves. What I have right now is attached.

I considered directly using or basing this on the StorageService
interface.  But in the end decided to make this separate for a couple
of reasons. The main one was that the requirement to treat the stored
value as a String would mean that there would have to be some impl
inefficiencies, vs being able to directly load or save a file
input/output stream.  Also, there aren't any impls of StorageService
for files or databases, which I think is informative that the use cases
are kinda different.  However I think it would be possible to have an
impl of this load/save manager interface that just delegates to a
supplied StorageService, if that ever seems useful.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://shibboleth.net/pipermail/dev/attachments/20160914/e62b1d75/attachment-0001.html>
-------------- next part --------------
/*
 * 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.util;

import java.io.IOException;
import java.util.Set;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

import org.opensaml.core.xml.XMLObject;

import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.collection.Pair;

/**
 * Interface for a component which is capable of loading and saving instances of {@link XMLObject},
 * based on a string key.
 * 
 * <p>
 * The index key strategy used is determined by the caller.
 * </p>
 * 
 * @param <T> the base type of XML objects being managed
 */
public interface XMLObjectLoadSaveManager<T extends XMLObject> {

    /**
     * Return a set of the index keys of all objects under management.
     * 
     * @return a set of all indexed keys
     * @throws IOException if there is a fatal error obtaining the keys
     */
    @Nonnull @NonnullElements
    public Set<String> listKeys() throws IOException;

    /**
     * Return an iterable of all objects under management, along with their associated index key.
     * 
     * @return an iterable of all managed objects
     * @throws IOException if there is a fatal error loading the managed objects
     */
    @Nonnull @NonnullElements
    public Iterable<Pair<String,T>> listAll() throws IOException;

    /**
     * Evaluate whether an object already exists indexed by the supplied key.
     * 
     * @param key the key of the desired object
     * @return true if object exists, false otherwise
     * @throws IOException if there is a fatal error evaluating object existence
     */
    public boolean exists(@Nonnull @NotEmpty final String key) throws IOException;

    /**
     * Load a particular object based on the supplied key.
     * 
     * @param key the key of the desired object
     * @return the object saved under the specified key, or null if there is no such object
     * @throws IOException if there is a fatal error loading the object
     */
    @Nullable
    public T load(@Nonnull @NotEmpty final String key) throws IOException;

    /**
     * Save a particular object, indexed by the supplied key.
     * 
     * <p>
     * An existing object indexed by the supplied key will not be overwritten. 
     * Instead an {@link IOException} will be thrown.
     * For saving with the overwrite option, see {@link #save(String, XMLObject, boolean)}.
     * </p>
     * 
     * @param key the key under which to index the object
     * @param xmlObject the object to save
     * @throws IOException if there is a fatal error saving the object, or if an object already exists
     *          indexed by the supplied key
     */
    public void save(@Nonnull @NotEmpty final String key, @Nonnull final T xmlObject) throws IOException;

    /**
     * Save a particular object, indexed by the supplied key.
     * 
     * @param key the key under which to index the object
     * @param xmlObject the object to save
     * @param overwrite whether or not to overwrite any existing object indexed by the supplied key
     * @throws IOException if there is a fatal error saving the object, or if overwrite=false,
     *          or if an object already exists indexed by the supplied key
     */
    public void save(@Nonnull @NotEmpty final String key, @Nonnull final T xmlObject, boolean overwrite)
            throws IOException;

    /**
     * Remove the object indexed by the specified key.
     * 
     * @param key the key of the object to remove
     * @return true if the object was found and successfully removed, false if no such object was found
     * @throws IOException if there was a fatal error removing the object
     */
    public boolean remove(@Nonnull @NotEmpty final String key) throws IOException;

    /**
     * Update the key under which a particular object is stored.
     * 
     * @param currentKey the current key under which the object is stored
     * @param newKey the new key under which the object should be stored
     * @return true if the object was found under the current key and the key successfully updated, 
     *          false if no such object was found
     * @throws IOException if there was a fatal error updating the key
     */
    public boolean updateKey(@Nonnull @NotEmpty final String currentKey, @Nonnull @NotEmpty final String newKey)
            throws IOException;

}