Resolve maven prop in dep coordinates for user features

[cherylking]

Fixes #1758

[scottkurz]

Question: is it correct that we don't have a similar problem in copyDependencies-related paths?

Just wondering why not...

Is the problem here that Maven only does property substitution for its built-in packaging types, like JAR/WAR, but not ESA?

[cherylking]

@scottkurz Good question. I will change one of the copyDependencies test cases to see if we need a similar change there.

[scottkurz]

I don't think there's much downside in doing an "extra" substitution...it's hard to imagine there's a valid "${abc}" value that you wouldn't want substituted...

So if doing it in PrepareFeatureSupport.downloadArtifact is easier than making a new abstract common method that seems OK. Maybe we could have a comment mentioning why this case needs resolution (because we parsed the pom.xml in DOM ourselves).

As far as the InstallFeatureSupport class...

I dont' think the new substitution is needed in getDependencyFeatures() .. since we got the artifact right from the project:
List<org.apache.maven.model.Dependency> dependencyArtifacts = project.getDependencies();

It might be slightly clearer reading the code to see that, in cases like this where we can use the project API we don't need to substitute...

As far as InstallFeatureSupport.downloadArtifact... I'm not clear.

It looks like there are paths like this where substitution is clearly not needed...

downloadArtifact(String, String, String, String)
  downloadEsaArtifact(String)
   downloadEsas(Collection<?>, Map<String, String>)
     installFeatures(boolean, List<String>)

since at the bottom of the stack we're starting already with a resolved feature.

But that method is called in 4 other ways.. do any of those others require it? Not sure.

I guess if you're fairly sure at least one path requires it then no big deal.. if we don't really have any evidence we need it and it passes then I'd probably leave it out but up to you