Refavamp and factor away need for "context artifact" parameters.

Description

Throughout the mummifiers and Mummy context, the methods have a contextArtifact parameter:

The artifact in which context the artifact is being generated, which may or may not be the same as the artifact being generated.

The impetus for all this overhead is the index.html file. It is an implementation detail: when we have some /folder/, the /folder/index.html file is only there to provide content for /folder/.

  • If there are links to /folder/index.html, we want them really to be to /folder/.

  • If there are links from /folder/index, we want the links to be calculated in terms of /folder/ as the source. (This usually only makes a difference if the page is linking to itself.)

  • When determining navigation links to /folder/index.html, we want to really determine them for /folder/, and ignore /folder/index.html altogether.

The Mummy context (now the Mummy plan as of ) contains a special map for for finding artifacts by reference that recognize both /folder/ and /folder/index.html as referencing the main /folder/. However we still pass the "context artifact" for /folder/ around through almost all the methods of the mummifiers, Mummy context, navigation manager, and related classes. This is a lot of cruft for little benefit. In addition we have some instances (e.g. the new artifact queries in ) in which we don't even have access to the mummifier or the Mummy context (e.g. ArtifactQuery.fromSiblingsOf(Artifact)) so we can't even get this "context artifact" and thus can't provide the correct answer.

There needs to be a better way to find and work with these "context artifacts". Here is a tentative approach:

  • Have a new category of artifacts called "subsumed" ("subordinate"? "subsidiary"?) artifacts, indicating they are implementation details—if nothing else than for a way to talk about them.

  • Have a means for a subsumed artifact to return its "principal artifact" (the old "context artifact"). One problem is that the "principal artifact" is not available when the subsumed artifact is created. We could have a way to set it later, or we could provide a method on the artifact that looks up the Mummy context using Csar (implemented in a separate ticket).

In any case, we need to remove all the "context artifact" parameters and simplify this before it keeps spreading.

Environment

None

Activity

Show:
Garret Wilson
January 9, 2021, 10:14 PM

Wow, it was obvious but still amazing how much this "context artifact" parameter cluttered the API. It was used in few places (pretty much for normalizing the source of links for "index" files), but it was passed around in loads of methods to make sure it got to where it needed to go, and sometimes apparently "just in case".

Having the "principal artifact" (the new and improved name of the "context artifact") automatically be determined in the MummyPlan methods and removing contextArtifact as a parameter in the API really cleans things up.

Garret Wilson
January 9, 2021, 3:53 PM

Further insight: the S3 code that called relativizeTargetReference(Artifact contextArtifact, Artifact referentArtifact) was for a slightly different purpose. That is, its needs semantically didn't match this method. The S3 code needed to find a relative path to each artifact, even foo/index.html. It didn't want to find a "reference to the best resource foo/index.html represents". That means if we ever updated the planning reference generation methods to detect foo/index.html as a reference target and normalize it to foo/, this code wouldn't work anymore. So the S3 code shouldn't even be calling method to generate references. Thus I've changed the S3 code to generate the S3 object names in the S3 code itself.

Garret Wilson
January 7, 2021, 2:53 PM
Edited

The terms "source" and "target" (referring to references in the source tree or the target tree) can be confusing in terms of references, because references themselves have "sources" and "targets". The thinking now is to put methods in the MummyPlan named referenceInSource(Artifact, Artifact) and referenceInTarget(Artifact, Artifact). A convenience method reference(Artifact, Artifact) can be added as an alias to referenceInSource(Artifact, Artifact) to be used with MEXL.

We might as well inform the plan of the source and target roots, and check those during the reference generation. We can take it out later if needed, if it provides too much overhead or we don't feel we need the safety. Actually if the plan has the root artifact, doesn't it know the root source and root target of site from the root and target paths of the root artifact? And semantically doesn't that even match better the checks we are trying to make?

Garret Wilson
January 6, 2021, 4:35 PM
Edited

These are the MummyContext reference generation methods that seem to be really used:

  • relativizeSourceReference(Artifact contextArtifact, Artifact referentArtifact) - The default navigation and the directory widget; basically general navigation generation at the source level before relocation.

  • relativizeTargetReference(Artifact contextArtifact, Artifact referentArtifact) - S3 determining the bucket path of each artifact by calculating its reference from the root. (This is done during "planning" but planning during deployment, which means the Mummy planning page is finished and the MummyPlan should be available.)

  • relativizeResourceReference(Path baseTargetPath, Path referenceTargetPath, boolean forceCollection) - 1) Relocating a template within the source tree, 2) relocating page links from source to target, and 2) generating source navigation links in the new navigation manager.

The two relocation use cases are interesting. When we apply a template, we have to move it within the source tree potentially from one level to another using artifact source paths for generating references. But when we relocate the entire document from the source to the target tree, we have to recalculate the references in terms of their target paths. These could in theory use something like the relativizeSourceReference() and relativizeTargetReference() methods in terms of resources, except the relocation methods are written in a general way so that they use method references to get Artifact::getSourcePath or Artifact::getTargetPath, breaking the method call down to its path-based invocation. That's apparently why relativizeResourceReference(Path baseTargetPath, Path referenceTargetPath, boolean forceCollection) needs to work with both source and target references.

It may be however that we could break down (build up?) the Function<Artifact, Path> referentArtifactPath even further so that it takes two artifacts. These methods already use contextArtifact.getSourcePath() and contextArtifact.getTargetPath() when the Function<Artifact, Path> is created. Instead of calling contextArtifact.getSourcePath() or contextArtifact.getTargetPath(), the same Function<Artifact, Path> referentArtifactPath could be called on the contextArtifact when needed, or a larger-grained Function<> could call relativizeSourceReference(contextArtifact, targetArtifact) or relativizeTargetReference(contextArtifact, targetArtifact).

What I'm getting at is that in the relocation code we may not need to break down the call to the path level, and still call the method in terms of the artifact. One goal here would be to get away from a relativization method that served double-duty for source paths and target paths—or at least it wouldn't have to check that both paths were in the same source or target, and could just concentrate on the paths. We could assume for example that all artifacts in the MummyPlan have correct source/target paths or they wouldn't be in the plan to begin with. (Although if we don't check, we don't know necessarily that the artifacts being passed in are in the plan or not.)

The NavigationManager calls directly come from using Artifact.getSourcePath() as well.

The conclusion I think I'm getting to here is that all of these actual use cases can be broken down to either relativizeSourceReference(Artifact contextArtifact, Artifact referentArtifact) or relativizeTargetReference(Artifact contextArtifact, Artifact referentArtifact) (with the latter version used only to relocate a page to the target tree, and to generate S3 paths). If we assume that the artifacts are from the same plan, perhaps we can trust that their source and target paths are in the same source/target tree? (In any case the relativization code will assure they have some common root path.) And if we then don't need to know the site source/target roots, these methods could go in MummyPlan and the plan would not need to store the rooth paths. In face we could simply have Artifact methods: Artifact.sourceReferenceTo(Artifact) and Artifact.targetReferenceTo(Artifact)! The calling code would need to call MummyPlan.getPrincipalArtifact(Artifact) first, though; this could be one stumbling block in using this directly from MEXL in mesh expressions, which is one of the end goals.

Garret Wilson
January 6, 2021, 4:06 PM

In MummyContext there seem to be three sets of reference-building methods:

  • Those for source references such as relativizeSourceReference(().

  • Those for target references such as relativizeTargetReference().

  • Those that work generally for references, both either in the source tree or in the target tree, such as relativizeResourceReference().

It looks like relativizeSourceReference(Path baseSourcePath,Path referenceSourcePath) and relativizeTargetReference(Path baseTargetPath, Path referenceTargetPath) are subsets of functionality of relativizeResourceReference(Path baseTargetPath, Path referenceTargetPath, boolean forceCollection). Furthermore the method variations delegating first two versions don't check the type of target artifact to see if it is a CollectionArtifact so as to force the target to be a collection.

Thus it would seem that several of the source and target reference methods could be consolidated into the general method, with the added benefit of better guaranteed collection references. (Currently the code that calls the existing methods may be generated from existing directories, meaning the paths would generate the collection reference slashes anyway. The new method with forceCollection was from configured navigation links to things that may not exist, I think.)

We would like to put these in the MummyPlan so that they can all be accessed from the plan, especially via Mesh MEXL expressions. That may mean we have to tell the plan about the source and target root paths.

Fixed

Assignee

Garret Wilson

Reporter

Garret Wilson

Labels

None

Components

Fix versions

Priority

Critical