Qs after reviewing Java dynamic metadata resolvers

[Cantor, Scott]

On 9/14/16, 2:49 PM, "dev on behalf of Brent Putman" <dev-bounces at shibboleth.net on behalf of putmanb at georgetown.edu> wrote:

> 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).

In batch, it's not just init, it's every load of a changed batch. Doesn't really change anything, just noting it.

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

I'm not opposed, but a simple Predicate optionally injected to control which entities get loaded seems like it would cover all bases, but defaulting that way doesn't bother me.

> An initial concern with the preemptive load was the memory usage.

More the init time, I would say. I don't think any amount of memory usage from this would come close to what the batch loads cost.

> 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.

As long as it's documented as non-threadsafe, no.

> 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.

Slight danger there if we re-introduce the problem of writing things out as XMLObjects, and risk the serializer breaking signatures, the bug we just fixed in the SP. I think the IdP currently backs up via streaming the data it gets directly, not with an XML roundtrip.

> 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.

Possibly, but I'm not sure I would advocate that we implement anything but file-backed to gain those efficiences. If we want something else, I would prefer that we use the storage API.

>  Also, there aren't any impls of StorageService for files or databases, which I think is 
> informative that the use cases are kinda different.

There certainly is one for databases. Not files, no, though it obviously could be done.

The use cases are always different. That isn't really the goal, to be optimal for any one case. The point is to prevent having to build multiple, complex persistence plugins because that's the hardest code there is to make reliable. If I thought we would ever build a file-based SS plugin, I would be against doing this as a one-off, regardless of the performance (within reason).

>  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.
    
I would prefer that we do that than ever build anything other than the file backed one, and yes, having additonal interfaces on top is fine, just like we do now for the replay cache, artifact map, session cache, etc. The complexity should be buried in those pieces, not in either the storage plugins or the code using those interfaces.

-- Scott