Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251 #595

Closed
wants to merge 1 commit into from
Closed

[MNG-7312] Revert ThreadLocal approach from MNG-6843 and MNG-7251 #595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 48 additions & 54 deletions maven-core/src/main/java/org/apache/maven/project/MavenProject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,13 @@
public class MavenProject
implements Cloneable
{

private static final Logger LOGGER = LoggerFactory.getLogger( MavenProject.class );

public static final String EMPTY_PROJECT_GROUP_ID = "unknown";

public static final String EMPTY_PROJECT_ARTIFACT_ID = "empty-project";

public static final String EMPTY_PROJECT_VERSION = "0";

private Model model;
Expand All @@ -107,6 +111,10 @@ public class MavenProject

private Set<Artifact> resolvedArtifacts;

private ArtifactFilter artifactFilter;

private Set<Artifact> artifacts;

private Artifact parentArtifact;

private Set<Artifact> pluginArtifacts;
Expand Down Expand Up @@ -143,7 +151,8 @@ public class MavenProject

private Artifact artifact;

private ThreadLocal<ArtifactsHolder> threadLocalArtifactsHolder = newThreadLocalArtifactsHolder();
// calculated.
private Map<String, Artifact> artifactMap;

private Model originalModel;

Expand Down Expand Up @@ -176,21 +185,12 @@ public class MavenProject
public MavenProject()
{
Model model = new Model();

model.setGroupId( EMPTY_PROJECT_GROUP_ID );
model.setArtifactId( EMPTY_PROJECT_ARTIFACT_ID );
model.setVersion( EMPTY_PROJECT_VERSION );
setModel( model );
}

private static ThreadLocal<ArtifactsHolder> newThreadLocalArtifactsHolder()
{
return new ThreadLocal<ArtifactsHolder>()
{
protected ArtifactsHolder initialValue()
{
return new ArtifactsHolder();
}
};
setModel( model );
}

public MavenProject( Model model )
Expand Down Expand Up @@ -695,11 +695,10 @@ public void addLicense( License license )

public void setArtifacts( Set<Artifact> artifacts )
{
ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
artifactsHolder.artifacts = artifacts;
this.artifacts = artifacts;

// flush the calculated artifactMap
artifactsHolder.artifactMap = null;
artifactMap = null;
}

/**
Expand All @@ -712,36 +711,34 @@ public void setArtifacts( Set<Artifact> artifacts )
*/
public Set<Artifact> getArtifacts()
{
ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
if ( artifactsHolder.artifacts == null )
if ( artifacts == null )
{
if ( artifactsHolder.artifactFilter == null || resolvedArtifacts == null )
if ( artifactFilter == null || resolvedArtifacts == null )
{
artifactsHolder.artifacts = new LinkedHashSet<>();
artifacts = new LinkedHashSet<>();
}
else
{
artifactsHolder.artifacts = new LinkedHashSet<>( resolvedArtifacts.size() * 2 );
artifacts = new LinkedHashSet<>( resolvedArtifacts.size() * 2 );
for ( Artifact artifact : resolvedArtifacts )
{
if ( artifactsHolder.artifactFilter.include( artifact ) )
if ( artifactFilter.include( artifact ) )
{
artifactsHolder.artifacts.add( artifact );
artifacts.add( artifact );
}
}
}
}
return artifactsHolder.artifacts;
return artifacts;
}

public Map<String, Artifact> getArtifactMap()
{
ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
if ( artifactsHolder.artifactMap == null )
if ( artifactMap == null )
{
artifactsHolder.artifactMap = ArtifactUtils.artifactMapByVersionlessId( getArtifacts() );
artifactMap = ArtifactUtils.artifactMapByVersionlessId( getArtifacts() );
}
return artifactsHolder.artifactMap;
return artifactMap;
}

public void setPluginArtifacts( Set<Artifact> pluginArtifacts )
Expand Down Expand Up @@ -1186,8 +1183,7 @@ public MavenProject clone()
{
throw new UnsupportedOperationException( e );
}
// clone must have it's own TL, otherwise the artifacts are intermingled!
clone.threadLocalArtifactsHolder = newThreadLocalArtifactsHolder();

clone.deepCopy( this );

return clone;
Expand Down Expand Up @@ -1230,7 +1226,6 @@ private void deepCopy( MavenProject project )
// copy fields
file = project.file;
basedir = project.basedir;
threadLocalArtifactsHolder.set( project.threadLocalArtifactsHolder.get().copy() );

// don't need a deep copy, they don't get modified or added/removed to/from - but make them unmodifiable to be
// sure!
Expand All @@ -1239,6 +1234,11 @@ private void deepCopy( MavenProject project )
setDependencyArtifacts( Collections.unmodifiableSet( project.getDependencyArtifacts() ) );
}

if ( project.getArtifacts() != null )
{
setArtifacts( Collections.unmodifiableSet( project.getArtifacts() ) );
}

if ( project.getParentFile() != null )
{
parentFile = new File( project.getParentFile().getAbsolutePath() );
Expand Down Expand Up @@ -1433,9 +1433,8 @@ public DependencyFilter getExtensionDependencyFilter()
public void setResolvedArtifacts( Set<Artifact> artifacts )
{
this.resolvedArtifacts = ( artifacts != null ) ? artifacts : Collections.<Artifact>emptySet();
ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
artifactsHolder.artifacts = null;
artifactsHolder.artifactMap = null;
this.artifacts = null;
this.artifactMap = null;
}

/**
Expand All @@ -1448,10 +1447,9 @@ public void setResolvedArtifacts( Set<Artifact> artifacts )
*/
public void setArtifactFilter( ArtifactFilter artifactFilter )
{
ArtifactsHolder artifactsHolder = threadLocalArtifactsHolder.get();
artifactsHolder.artifactFilter = artifactFilter;
artifactsHolder.artifacts = null;
artifactsHolder.artifactMap = null;
this.artifactFilter = artifactFilter;
this.artifacts = null;
this.artifactMap = null;
}

/**
Expand Down Expand Up @@ -1481,7 +1479,13 @@ public void addLifecyclePhase( String lifecyclePhase )

// ----------------------------------------------------------------------------------------------------------------
//
// D E P R E C A T E D - Everything below will be removed for Maven 4.0.0
//
// D E P R E C A T E D
//
//
// ----------------------------------------------------------------------------------------------------------------
//
// Everything below will be removed for Maven 4.0.0
//
// ----------------------------------------------------------------------------------------------------------------

Expand All @@ -1502,6 +1506,7 @@ public String getModulePathAdjustment( MavenProject moduleProject )
if ( moduleFile != null )
{
File moduleDir = moduleFile.getCanonicalFile().getParentFile();

module = moduleDir.getName();
}

Expand Down Expand Up @@ -1822,6 +1827,7 @@ public Reporting getReporting()
public void setReportArtifacts( Set<Artifact> reportArtifacts )
{
this.reportArtifacts = reportArtifacts;

reportArtifactMap = null;
}

Expand All @@ -1838,13 +1844,15 @@ public Map<String, Artifact> getReportArtifactMap()
{
reportArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getReportArtifacts() );
}

return reportArtifactMap;
}

@Deprecated
public void setExtensionArtifacts( Set<Artifact> extensionArtifacts )
{
this.extensionArtifacts = extensionArtifacts;

extensionArtifactMap = null;
}

Expand All @@ -1861,6 +1869,7 @@ public Map<String, Artifact> getExtensionArtifactMap()
{
extensionArtifactMap = ArtifactUtils.artifactMapByVersionlessId( getExtensionArtifacts() );
}

return extensionArtifactMap;
}

Expand All @@ -1878,6 +1887,7 @@ public List<ReportPlugin> getReportPlugins()
public Xpp3Dom getReportConfiguration( String pluginGroupId, String pluginArtifactId, String reportSetId )
{
Xpp3Dom dom = null;

// ----------------------------------------------------------------------
// I would like to be able to lookup the Mojo object using a key but
// we have a limitation in modello that will be remedied shortly. So
Expand Down Expand Up @@ -1981,20 +1991,4 @@ public void setProjectBuildingRequest( ProjectBuildingRequest projectBuildingReq
{
this.projectBuilderConfiguration = projectBuildingRequest;
}

private static class ArtifactsHolder
{
private ArtifactFilter artifactFilter;
private Set<Artifact> artifacts;
private Map<String, Artifact> artifactMap;

ArtifactsHolder copy()
{
ArtifactsHolder copy = new ArtifactsHolder();
copy.artifactFilter = artifactFilter;
copy.artifacts = artifacts != null ? new LinkedHashSet<>( artifacts ) : null;
copy.artifactMap = artifactMap != null ? new LinkedHashMap<>( artifactMap ) : null;
return copy;
}
}
}
43 changes: 0 additions & 43 deletions maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,13 @@

import java.io.File;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;

import org.apache.maven.artifact.Artifact;
import org.apache.maven.model.DependencyManagement;
import org.apache.maven.model.Model;
import org.apache.maven.model.Parent;
import org.apache.maven.model.Profile;
import org.mockito.Mockito;

public class MavenProjectTest
extends AbstractMavenProjectTestCase
Expand Down Expand Up @@ -193,44 +188,6 @@ public void testCloneWithBaseDir()
assertEquals( "Base directory is preserved across clone", projectToClone.getBasedir(), clonedProject.getBasedir() );
}

public void testCloneWithArtifacts()
throws InterruptedException
{
Artifact initialArtifact = Mockito.mock( Artifact.class, "initialArtifact" );
MavenProject originalProject = new MavenProject();
originalProject.setArtifacts( Collections.singleton( initialArtifact ) );
assertEquals( "Sanity check: originalProject returns artifact that has just been set",
Collections.singleton( initialArtifact ), originalProject.getArtifacts() );

final MavenProject clonedProject = originalProject.clone();

assertEquals( "Cloned project returns the artifact that was set for the original project",
Collections.singleton( initialArtifact ), clonedProject.getArtifacts() );

Artifact anotherArtifact = Mockito.mock( Artifact.class, "anotherArtifact" );
clonedProject.setArtifacts( Collections.singleton( anotherArtifact ) );
assertEquals( "Sanity check: clonedProject returns artifact that has just been set",
Collections.singleton( anotherArtifact ), clonedProject.getArtifacts() );

assertEquals( "Original project returns the artifact that was set initially (not the one for clonedProject)",
Collections.singleton( initialArtifact ), originalProject.getArtifacts() );

final AtomicReference<Set<Artifact>> artifactsFromThread = new AtomicReference<>();
Thread thread = new Thread( new Runnable()
{
@Override
public void run()
{
artifactsFromThread.set( clonedProject.getArtifacts() );
}
} );
thread.start();
thread.join();

assertEquals( "Another thread does not see the same artifacts",
Collections.emptySet(), artifactsFromThread.get() );
}

public void testUndefinedOutputDirectory()
throws Exception
{
Expand Down