<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 8/31/16 1:44 PM, Cantor, Scott
wrote:<br>
</div>
<blockquote cite="mid:F223E525-DC79-485E-BDAA-D7BCB6D1FA6E@osu.edu"
type="cite">
<pre wrap="">I've filed a couple of bugs after reviewing the code,</pre>
</blockquote>
<br>
I'll look at the OSJ-171 and OSJ-172 ones and respond in the
issues. Those two seem pretty minor.<br>
<br>
<br>
<blockquote cite="mid:F223E525-DC79-485E-BDAA-D7BCB6D1FA6E@osu.edu"
type="cite">
<pre wrap="">
I'm guessing maybe this is already known, but it seems like this design doesn't accomodate artifact resolution if the transformation of the URI is configured into the resolver "once" at wiring time, and then only works against an EntityIDCriterion.
There are a number of ways it could be enhanced I suppose, but I also found it a little strange that the Function interface it's using is String->String vs. CriteriaSet->String. The latter would accomodate an ArtifactSourceIDCriterion for example, and then a Function chain could be used to look for supported Criterion and handle the different cases. As it is, I'm not sure how it would work.</pre>
</blockquote>
<br>
Yeah, I think I didn't have the artifact use case in mind when I
originally did this work. Was that always in the MDQ spec - as in
specifically mentioned - or was it added later? I probably just
overlooked it.<br>
<br>
In any case, I agree the current impl is not correct for that use
case. However, I think it's trivial to fix. The
FunctionDrivenDynamicHTTPMetadataResolver is the (trivial) concrete
class here, and all it basically does is provide an impl of the
superclass abstract method:<br>
<br>
String buildRequestURL(CriteriaSet criteria)<br>
<br>
So the input there already is a CriteriaSet. The current
String->String function is just an impl detail. So it would be
trivial to fix things for this case: Just add a new
Function<CriteriaSet, String> property and if configured use
that in preference to the existing String->String one. Or
something like that. Details TBD. Maybe deprecate the
String->String one.<br>
<br>
And/or of course we could add a new similarly trivial concrete
subclass that has only a CriteriaSet->String function.<br>
<br>
Fixing up the Spring custom parsing to use either of those
approaches in the IdP doesn't seem that bad, it's all "under the
covers" kind of stuff.<br>
<br>
<blockquote cite="mid:F223E525-DC79-485E-BDAA-D7BCB6D1FA6E@osu.edu"
type="cite">
<pre wrap="">
Mostly I'm reviewing the code to see where the best place to add file-backed support would be, because we need to get that into 3.3.</pre>
</blockquote>
<br>
<br>
Read on, but I think it's AbstractDynamicMetadataResolver (NOT the
HTTP-specific subclass).<br>
<br>
<br>
<blockquote cite="mid:F223E525-DC79-485E-BDAA-D7BCB6D1FA6E@osu.edu"
type="cite">
<pre wrap="">
Does the HttpClient support file URIs by any random chance? Probably not I'm guessing.</pre>
</blockquote>
<br>
Not that I'm aware.<br>
<br>
<blockquote cite="mid:F223E525-DC79-485E-BDAA-D7BCB6D1FA6E@osu.edu"
type="cite">
<pre wrap="">
I guess there are a few ways to do this:
- extend AbstractDynamic[HTTP]MetadataResolver to support failing into backup files</pre>
</blockquote>
<br>
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.<br>
<br>
<br>
<blockquote cite="mid:F223E525-DC79-485E-BDAA-D7BCB6D1FA6E@osu.edu"
type="cite">
<pre wrap="">
- create a new resolver that's strictly file-based (largely duplicating some of the existing class hierarchy underneath AbstractDynamicHTTPMetadataResolver) and force the deployer to chain to that as a backup</pre>
</blockquote>
<br>
Well, as far as providing a totally file-based dynamic resolver (the
folder o' metadata idea), my plan was for a relatively trivial
concrete subclass of AbstractDynamicMetadataResolver. Note that
this class is where all of the "dynamic" bits live. That was
deliberate, making it easy to create new dynamic resolvers that are
not tied to HTTP. The AbstractDynamicHTTPMetadataResolver just has
the HTTP transport bits only.<br>
<br>
As we've previously discussed, I think the only trick for the
file-based one is devising the appropriate strategy for turning
entityIDs into usable file names. And providing a end-user tool to
let them convert/rename an EntityDescriptor using the same
strategy. Or something fancier, like an ingest queue processed by a
background (drop in the file, it gets processed and moved to the
runtime metadata dir with the correct name).<br>
<br>
<blockquote cite="mid:F223E525-DC79-485E-BDAA-D7BCB6D1FA6E@osu.edu"
type="cite">
<pre wrap="">
- pull the transport aspect of this out of the class hierarchy and deprecate what's there now</pre>
</blockquote>
<br>
Offhand, at the moment, I don't personally see a need to do this,
given the relative ease of adding backing file support to
AbstractDynamicMetadataResolver, which then covers all the dynamic
resolver cases.<br>
<br>
But if there's a good reason, or something I'm missing, of course
let's discuss.<br>
<br>
<blockquote cite="mid:F223E525-DC79-485E-BDAA-D7BCB6D1FA6E@osu.edu"
type="cite">
<pre wrap="">
I favor the latter. Basically, I think instead of baking HTTP-awareness into a metadata resolver,</pre>
</blockquote>
<br>
To re-iterate: The HTTP awareness is *not* in the dynamic resolver
code. We already have a transport/source-agnostic dynamic resolver
base class, the AbstractDynamicMetadataResolver. That was
deliberate.<br>
<br>
<br>
<br>
<blockquote cite="mid:F223E525-DC79-485E-BDAA-D7BCB6D1FA6E@osu.edu"
type="cite">
<pre wrap=""> we should plug in a new interface that handles metadata acquisition, and that interface can take over the HTTP client case, the existing or extended entityID -> URL transformation job, and include additional support for backing up to files, and simply reading from files.</pre>
</blockquote>
<br>
You may remember that early on in v3 Chad had proposed something
that sounds very much like that - a 2-part metadata resolver that
separated the "source" of the metadata from the "resolving" from
it. And in the end we rejected it because there was something about
it that didn't work. I'd have to go look at my email archive to
swap in exactly what it was, but I think in fact it was you who
originally pointed out the problem(s). Maybe that's not the same
thing as you are proposing here, though.<br>
<br>
But that in general sounds like a much more complicated design.
Reading/writing files isn't that hard, and I think (albeit not
having gotten into it yet) that that is easily done in
AbstractDynamicMetadataResolver. 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. The latter component approach
is kind of what I had been thinking, although I hadn't gotten to
anything concrete yet.<br>
<br>
<br>
<br>
</body>
</html>