Skip to content

Commit

Permalink
Fix the semantics of the ModelCacheTag.RAW
Browse files Browse the repository at this point in the history 
The content of the tag has been slightly blurred by the fix for MNG-5669 while this is not necessary.
The RAW tag is used to store *raw* models, not *effective* models.
The introduction of a second level of cache using the ModelCacheTag.FILEMODEL is sufficient to avoid reading the raw model multiple times.
This also obscurs the usage of the cache a lot as it was not used inside a fromCache / intoCache pair.

# Conflicts:
#	maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
  • Loading branch information
@gnodet
gnodet committed Nov 27, 2020
1 parent 331e402 commit aa285f2
Showing 1 changed file with 41 additions and 66 deletions.
107 changes: 41 additions & 66 deletions maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import static org.apache.maven.model.building.Result.error;
import static org.apache.maven.model.building.Result.newResult;

import org.apache.maven.artifact.versioning.ArtifactVersion;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -432,16 +431,6 @@ else if ( !parentIds.add( parentData.getId() ) )
resultData.setArtifactId( resultModel.getArtifactId() );
resultData.setVersion( resultModel.getVersion() );

if ( request.getPomFile() != null )
{
intoCache( request.getModelCache(), new FileModelSource( request.getPomFile() ), ModelCacheTag.RAW,
resultData );
}
else
{
intoCache( request.getModelCache(), request.getModelSource(), ModelCacheTag.RAW, resultData );
}

result.setEffectiveModel( resultModel );

for ( ModelData currentData : lineage )
Expand Down Expand Up @@ -883,79 +872,65 @@ private ModelData readParent( Model childModel, ModelSource childSource, ModelBu
DefaultModelProblemCollector problems )
throws ModelBuildingException
{
ModelData parentData = null;
ModelData parentData;

Parent parent = childModel.getParent();

if ( parent != null )
{
ModelSource expectedParentSource = getParentPomFile( childModel, childSource );
String groupId = parent.getGroupId();
String artifactId = parent.getArtifactId();
String version = parent.getVersion();

if ( expectedParentSource != null )
{
ModelData candidateData = readParentLocally( childModel, childSource, request, problems );

if ( candidateData != null )
{
/*
* NOTE: This is a sanity check of the cache hit. If the cached parent POM was locally resolved,
* the child's GAV should match with that parent, too. If it doesn't, we ignore the cache and
* resolve externally, to mimic the behavior if the cache didn't exist in the first place.
* Otherwise, the cache would obscure a bad POM.
*/
try
{
VersionRange parentVersion = VersionRange.createFromVersionSpec( parent.getVersion() );

This comment has been minimized.

Sign in to view

Copy link
@rfscholte

rfscholte Jan 7, 2021

Contributor

I don't see this versionrange check anymore, what happened with it?
ArtifactVersion actualVersion = new DefaultArtifactVersion( candidateData.getVersion() );

if ( parent.getGroupId().equals( candidateData.getGroupId() )
&& parent.getArtifactId().equals( candidateData.getArtifactId() )
&& parentVersion.containsVersion( actualVersion ) )
{
parentData = candidateData;
}
}
catch ( InvalidVersionSpecificationException e )
{
// This should already been blocked during validation
}
}
}
parentData = fromCache( request.getModelCache(), groupId, artifactId, version, ModelCacheTag.RAW );

if ( parentData == null )
{
ModelData candidateData = fromCache( request.getModelCache(),
parent.getGroupId(), parent.getArtifactId(),
parent.getVersion(), ModelCacheTag.RAW );
parentData = readParentLocally( childModel, childSource, request, problems );


if ( candidateData != null && candidateData.getSource() instanceof ArtifactModelSource )
{
// ArtifactModelSource means repositorySource
parentData = candidateData;
}
else
if ( parentData == null )
{
parentData = readParentExternally( childModel, request, problems );

intoCache( request.getModelCache(),
parentData.getGroupId(), parentData.getArtifactId(),
parentData.getVersion(), ModelCacheTag.RAW, parentData );
}

intoCache( request.getModelCache(), groupId, artifactId, version, ModelCacheTag.RAW, parentData );
}

if ( parentData != null )
else
{
Model parentModel = parentData.getModel();

if ( !"pom".equals( parentModel.getPackaging() ) )
/*
* NOTE: This is a sanity check of the cache hit. If the cached parent POM was locally resolved, the
* child's <relativePath> should point at that parent, too. If it doesn't, we ignore the cache and
* resolve externally, to mimic the behavior if the cache didn't exist in the first place. Otherwise,
* the cache would obscure a bad POM.
*/

File pomFile = parentData.getModel().getPomFile();
if ( pomFile != null )
{
problems.add( new ModelProblemCollectorRequest( Severity.ERROR, Version.BASE )
.setMessage( "Invalid packaging for parent POM " + ModelProblemUtils.toSourceHint( parentModel )
+ ", must be \"pom\" but is \"" + parentModel.getPackaging() + "\"" )
.setLocation( parentModel.getLocation( "packaging" ) ) );
FileModelSource pomSource = new FileModelSource( pomFile );
ModelSource expectedParentSource = getParentPomFile( childModel, childSource );

if ( expectedParentSource == null || ( expectedParentSource instanceof ModelSource2
&& !pomSource.equals( expectedParentSource ) ) )
{
parentData = readParentExternally( childModel, request, problems );
}
}
}

Model parentModel = parentData.getModel();

if ( !"pom".equals( parentModel.getPackaging() ) )
{
problems.add( new ModelProblemCollectorRequest( Severity.ERROR, Version.BASE )
.setMessage( "Invalid packaging for parent POM " + ModelProblemUtils.toSourceHint( parentModel )
+ ", must be \"pom\" but is \"" + parentModel.getPackaging() + "\"" )
.setLocation( parentModel.getLocation( "packaging" ) ) );
}
}
else
{
parentData = null;
}

return parentData;
Expand Down

0 comments on commit aa285f2

Please sign in to comment.