S-RAMP: New extensibility contracts

[brmeyer]

S-RAMP has a need for 3 different types of extensible actions:

1. Build up a primary artifact, generate its derived artifacts, and define relationships between them.  This is handled by the new ArtifactBuilder contract: s-ramp/ArtifactBuilder.java at master · Governance/s-ramp · GitHub
2. Expand an archive, creating artifacts from discovered items.  (This currently exists through s-ramp/s-ramp-atom/src/main/java/org/overlord/sramp/atom/archive at master · Governance/s-ramp · GitHub, but its re-design is the main purpose of this discussion.)
3. Identify an artifact type, using various rulesets (file contents, file names, other contexts).

Note that #1 is additive.  Each ArtifactBuilder is given the opportunity to "do its thing", independent of the others.

But note that #2 and #3 need to be exclusive.  The providers need to be ordered from most-specific down to most-generic.  Once an applicable provider is found, it's executed and the rest of the chain is ignored.

So, I'd love feedback from the community on the best way to set up these contracts.  Should all concepts be crammed into ArtifactBuilder?  Should ArtifactBuilder be left as-is, but #2 and #3 go together into one new contract?  Use separated contracts for all 3?

I think I lean towards the latter.  Each action is distinct and not much overlap exists (if at all).  Keeping the concepts separated may be a good idea.  Although, that's obviously at the expense of having multiple contracts to implement, rather than 1 central interface.

[eric.wittmann]

Might also be a good time to re-think derivation.  The reason s-ramp has the concept of an Expander at all is because we didn't want to derive content from a derived artifact.  But perhaps that was the wrong decision.  If a derived artifact could be further derived, then you only need to solve #1 and #3.  And #3 is only applicable when an artifact is added without an artifact type explicitly set.

[brmeyer]

The reason s-ramp has the concept of an Expander at all is because we didn't want to derive content from a derived artifact.

Really interesting thought.  So, "expanding" could be seen as simply deriving a file, but the difference is the first level of "derived" artifacts are primary artifacts/files themselves.  The ArtifactBuilder could simply be called iteratively w/ minimal extra logic.  That would also give an "easy" opportunity for clients to define relationships amongst the entire set.

[eric.wittmann]

Yes exactly.  It's unfortunate that the spec dictates that certain artifact types are always "derived" and others are not.  If that weren't the case then I would say "derived" would just be a flag on the artifact.  Sadly, if I remember the spec correctly, you're stuck.

So you might have to stick with "Expanding" as a concept. 

[objectiser]

I liked your original thought about treating them individually, as they seemed to do different tasks - but also see the benefit of Eric's suggestion.

My main question is how the distinction between primary and derived artifacts would be determined - would this be the responsibility of the ArtifactBuilder implementation? Or a property of the artifact being built?

[brmeyer]

My main question is how the distinction between primary and derived artifacts would be determined - would this be the responsibility of the ArtifactBuilder implementation? Or a property of the artifact being built?

Realistically, the ArtifactBuilder only needs to "generate additional artifacts from this given artifact", and that's it.  The spec's concept of "derived" is really just a set of relationships and constraints pegged onto specific artifact types.  So presumably, the needed logic would be fairly simple.

I liked your original thought about treating them individually, as they seemed to do different tasks

+1.  Originally, my concern was that #2 and #3 will both need to open an archive, #2 to expand it and #3 to inspect it.  Splitting into separate contracts may have meant duplication of efforts.  But if I crack open the archive once and provide it to both contracts through abstraction, there may be no downside.

[brmeyer]

Alright, here's what I had in mind.  Pardon the really, really rough diagram, but hopefully it helps: https://www.dropbox.com/s/h8akmwpl83zqsxv/ArtifactExpander.pdf?dl=0

1. A new REST endpoint would be created to accept a filename (slug), InputStream, and a Map<String, String> of "contextParameters" (client-specific "clues", such as the GAV from mvn:deploy).  Note that the model and type would not be included.  Explicitly identifying them would go through the existing endpoints.
2. From #1, create an "ArtifactContext" and set the original archive and its exploded contents
3. Give the original archive to the "ArtifactTypeDetector" chain.  The most specific detector will set the archive artifact type in the ArtifactContext.
4. Give the ArtifactContext to the "ArtifactExpander" chain.  Again, the most specific detector wins (but inheritance should be used whenever applicable).  This generates the full list of artifacts from the exploded archive contents, as well as expansion-specific relationships.
5. Run the entire set through ArtifactBuilder, which will be left as is: build up all properties, generate derived artifacts, and set additional relationships.  This step does not need the ArtifactContext, since all artifact types are already available.

ArtifactExpander would pull in much of the current ZipToSrampArchive functionality, generating an SrampArchive and re-using the existing batch-upload endpoint.

Any thoughts?

[objectiser]

Sounds good to me. Just wanted to check how "most specific" would be determined in #3 and #4?

[brmeyer]

Presumably, the ArtifactTypeDetector and ArtifactExpander contracts would both provide an integer order value