Skip to content

Commit 6896260

Browse files
jpobstjpobst
committed
Address review feedback.
1 parent f938d56 commit 6896260

File tree

14 files changed

+137
-151
lines changed

14 files changed

+137
-151
lines changed

‎src/Java.Interop.Tools.Maven/DefaultPomResolver.cs‎ renamed to ‎src/Java.Interop.Tools.Maven/DefaultProjectResolver.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,18 @@
44

55
namespace Java.Interop.Tools.Maven;
66

7-
public class DefaultPomResolver : IPomResolver
7+
public class DefaultProjectResolver : IProjectResolver
88
{
99
readonly Dictionary<string, Project> poms = new ();
1010

1111
public void Register (Project project)
1212
{
13-
poms.Add (project.ToString (), project);
13+
poms.Add (project.VersionedArtifactString, project);
1414
}
1515

16-
public virtual Project ResolveRawProject (Artifact artifact)
16+
public virtual Project Resolve (Artifact artifact)
1717
{
18-
if (poms.TryGetValue (artifact.ToString (), out var project))
18+
if (poms.TryGetValue (artifact.VersionedArtifactString, out var project))
1919
return project;
2020

2121
throw new InvalidOperationException ($"No POM registered for {artifact}");

‎src/Java.Interop.Tools.Maven/Extensions/MavenNetExtensions.cs‎

Lines changed: 0 additions & 72 deletions
This file was deleted.

‎src/Java.Interop.Tools.Maven/IPomResolver.cs‎ renamed to ‎src/Java.Interop.Tools.Maven/IProjectResolver.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
namespace Java.Interop.Tools.Maven;
44

5-
public interface IPomResolver
5+
public interface IProjectResolver
66
{
7-
Project ResolveRawProject (Artifact artifact);
7+
Project Resolve (Artifact artifact);
88
}

‎src/Java.Interop.Tools.Maven/Models/Artifact.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ public class Artifact
1111

1212
public string Version { get; }
1313

14+
public string ArtifactString => $"{GroupId}:{Id}";
15+
16+
// Format should match Project.ArtifactString for comparisons.
17+
public string VersionedArtifactString => $"{GroupId}:{Id}:{Version}";
18+
1419
public Artifact (string groupId, string artifactId, string version)
1520
{
1621
Id = artifactId;
@@ -40,6 +45,5 @@ public static bool TryParse (string value, [NotNullWhen (true)]out Artifact? art
4045
return true;
4146
}
4247

43-
// This is a "well-known" format we use, it should not be changed.
44-
public override string ToString () => $"{GroupId}:{Id}:{Version}";
48+
public override string ToString () => VersionedArtifactString;
4549
}

‎src/Java.Interop.Tools.Maven/Models/Dependency.Partial.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
using Java.Interop.Tools.Maven.Extensions;
2+
3+
namespace Java.Interop.Tools.Maven.Models;
4+
5+
public partial class Dependency
6+
{
7+
public Artifact ToArtifact ()
8+
=> new Artifact (GroupId.OrEmpty (), ArtifactId.OrEmpty (), Version.OrEmpty ());
9+
}

‎src/Java.Interop.Tools.Maven/Models/Project.Partial.cs‎

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,32 +8,22 @@ namespace Java.Interop.Tools.Maven.Models;
88

99
public partial class Project
1010
{
11-
public static Project Parse (Stream stream)
12-
{
13-
Project? result = null;
14-
15-
var serializer = new XmlSerializer (typeof (Project));
16-
17-
using (var sr = new StreamReader (stream))
18-
result = (Project) serializer.Deserialize (new XmlTextReader (sr) {
19-
Namespaces = false,
20-
});
21-
22-
return result;
23-
}
11+
static readonly XmlSerializer xml_serializer = new (typeof (Project));
2412

25-
public static Project ParseXml (string xml)
13+
public static Project Load (Stream stream)
2614
{
27-
Project? result = null;
15+
using (var xr = new XmlTextReader (stream) { Namespaces = false })
16+
return Load (xr);
17+
}
2818

29-
var serializer = new XmlSerializer (typeof (Project));
19+
public static Project Load (XmlReader reader)
20+
=> (Project) xml_serializer.Deserialize (reader);
3021

22+
public static Project Parse (string xml)
23+
{
3124
using (var sr = new StringReader (xml))
32-
result = (Project) serializer.Deserialize (new XmlTextReader (sr) {
33-
Namespaces = false,
34-
});
35-
36-
return result;
25+
using (var xr = new XmlTextReader (sr) { Namespaces = false })
26+
return Load (xr);
3727
}
3828

3929
public bool TryGetParentPomArtifact ([NotNullWhen (true)] out Artifact? parent)
@@ -48,7 +38,7 @@ public bool TryGetParentPomArtifact ([NotNullWhen (true)] out Artifact? parent)
4838
return false;
4939
}
5040

51-
public override string ToString () => $"{GroupId}:{ArtifactId}:{Version}";
41+
public override string ToString () => VersionedArtifactString;
5242

5343
public string ToXml ()
5444
{
@@ -59,4 +49,7 @@ public string ToXml ()
5949
return sw.ToString ();
6050
}
6151
}
52+
53+
// Format should match Artifact.VersionedArtifactString for comparisons.
54+
public string VersionedArtifactString => $"{GroupId}:{ArtifactId}:{Version}";
6255
}

‎src/Java.Interop.Tools.Maven/Models/ResolvedDependency.cs‎

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
using System;
2+
using System.Diagnostics.CodeAnalysis;
3+
using System.Linq;
14
using Java.Interop.Tools.Maven.Extensions;
25

36
namespace Java.Interop.Tools.Maven.Models;
@@ -13,6 +16,9 @@ public class ResolvedDependency
1316
public string? Type { get; }
1417
public string Version { get; }
1518

19+
public string ArtifactString => $"{GroupId}:{ArtifactId}";
20+
public string VersionedArtifactString => $"{GroupId}:{ArtifactId}:{Version}";
21+
1622
public ResolvedDependency (ResolvedProject project, Dependency dependency)
1723
: this (project, dependency, false)
1824
{ }
@@ -33,19 +39,19 @@ internal ResolvedDependency (ResolvedProject project, Dependency dependency, boo
3339
// If we're not shallow, fill in any still missing properties with parent values
3440
if (!shallow) {
3541
if (!Classifier.HasValue ())
36-
Classifier = this.GetInheritedProperty (project, d => d.Classifier);
42+
Classifier = GetInheritedProperty (this, project, d => d.Classifier);
3743

3844
if (!Optional.HasValue ())
39-
Optional = this.GetInheritedProperty (project, d => d.Optional);
45+
Optional = GetInheritedProperty (this, project, d => d.Optional);
4046

4147
if (!Scope.HasValue ())
42-
Scope = this.GetInheritedProperty (project, d => d.Scope);
48+
Scope = GetInheritedProperty (this, project, d => d.Scope);
4349

4450
if (!Type.HasValue ())
45-
Type = this.GetInheritedProperty (project, d => d.Type);
51+
Type = GetInheritedProperty (this, project, d => d.Type);
4652

4753
if (!Version.HasValue ())
48-
Version = this.GetInheritedProperty (project, d => d.Version);
54+
Version = GetInheritedProperty (this, project, d => d.Version);
4955
}
5056

5157
// Default scope to "compile" if not specified
@@ -57,12 +63,63 @@ internal ResolvedDependency (ResolvedProject project, Dependency dependency, boo
5763
Optional = "false";
5864
}
5965

60-
public string ToArtifactString (bool includeVersion = true)
66+
public override string ToString () => $"{VersionedArtifactString} - {Scope}";
67+
68+
static string GetInheritedProperty (ResolvedDependency dependency, ResolvedProject project, Func<ResolvedDependency, string?> property)
6169
{
62-
return includeVersion
63-
? $"{GroupId}:{ArtifactId}:{Version}"
64-
: $"{GroupId}:{ArtifactId}";
70+
// Check our <dependencyManagement> section
71+
if (CheckDependencyManagementSection (project, dependency, property, out var result))
72+
return result;
73+
74+
// Check imported POMs
75+
foreach (var imported in project.ImportedPomProjects) {
76+
var value = GetInheritedProperty (dependency, imported, property);
77+
78+
if (value.HasValue ())
79+
return value;
80+
}
81+
82+
// Check parent POM
83+
if (project.Parent is not null && !project.Parent.IsSuperPom)
84+
return GetInheritedProperty (dependency, project.Parent, property);
85+
86+
return string.Empty;
87+
}
88+
89+
static bool CheckImportedPoms (ResolvedDependency dependency, ResolvedProject project, Func<ResolvedDependency, string?> property, [NotNullWhen (true)] out string? result)
90+
{
91+
result = null;
92+
93+
foreach (var imported in project.ImportedPomProjects) {
94+
var imported_dep = imported.Resolved.DependencyManagement?.Dependencies.FirstOrDefault (x => x.ArtifactId == dependency.ArtifactId && x.GroupId == dependency.GroupId);
95+
96+
if (imported_dep != null) {
97+
result = property (new ResolvedDependency (imported, imported_dep, true));
98+
99+
if (result.HasValue ())
100+
return true;
101+
}
102+
103+
// Recurse, as imported POMs can also import POMs
104+
if (CheckImportedPoms (dependency, imported, property, out result))
105+
return true;
106+
}
107+
108+
return false;
65109
}
66110

67-
public override string ToString () => $"{GroupId}:{ArtifactId}:{Version} - {Scope}";
111+
static bool CheckDependencyManagementSection (ResolvedProject project, ResolvedDependency dependency, Func<ResolvedDependency, string?> property, [NotNullWhen (true)] out string? result)
112+
{
113+
result = null;
114+
115+
// Check <dependencyManagement>
116+
var dep_man = project.Resolved.DependencyManagement?.Dependencies.FirstOrDefault (x => x.ArtifactId == dependency.ArtifactId && x.GroupId == dependency.GroupId);
117+
118+
if (dep_man != null) {
119+
result = property (new ResolvedDependency (project, dep_man, true)) ?? string.Empty;
120+
return result.HasValue ();
121+
}
122+
123+
return false;
124+
}
68125
}

‎src/Java.Interop.Tools.Maven/Models/ResolvedProject.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Java.Interop.Tools.Maven.Models;
99
public class ResolvedProject
1010
{
1111
readonly ResolvedProject? parent;
12-
readonly IPomResolver? resolver;
12+
readonly IProjectResolver? resolver;
1313

1414
Project? resolved_project;
1515

@@ -34,7 +34,7 @@ public ResolvedProject Parent {
3434
}
3535
}
3636

37-
public ResolvedProject (Project project, ResolvedProject parent, IPomResolver resolver)
37+
public ResolvedProject (Project project, ResolvedProject parent, IProjectResolver resolver)
3838
{
3939
Raw = project;
4040
this.parent = parent;
@@ -43,17 +43,17 @@ public ResolvedProject (Project project, ResolvedProject parent, IPomResolver re
4343

4444
public void Resolve () => ResolveCore (new PropertyStack ());
4545

46-
public static ResolvedProject FromArtifact (Artifact artifact, IPomResolver resolver)
46+
public static ResolvedProject FromArtifact (Artifact artifact, IProjectResolver resolver)
4747
{
4848
var project = FromArtifactCore (artifact, resolver);
4949
project.Resolve ();
5050

5151
return project;
5252
}
5353

54-
static ResolvedProject FromArtifactCore (Artifact artifact, IPomResolver resolver)
54+
static ResolvedProject FromArtifactCore (Artifact artifact, IProjectResolver resolver)
5555
{
56-
var raw = resolver.ResolveRawProject (artifact);
56+
var raw = resolver.Resolve (artifact);
5757

5858
// POM has a parent, resolve it
5959
if (raw.TryGetParentPomArtifact (out var parentArtifact)) {
@@ -79,7 +79,7 @@ void ResolveCore (PropertyStack properties)
7979
var xml = Raw.ToXml ();
8080
xml = ReplaceProperties (xml, this, properties);
8181

82-
resolved_project = Project.ParseXml (xml);
82+
resolved_project = Project.Parse (xml);
8383

8484
properties.Push (Raw.Properties);
8585
parent?.ResolveCore (properties);

‎src/Java.Interop.Tools.Maven/Repositories/MavenRepository.cs‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace Java.Interop.Tools.Maven.Repositories;
88
public class MavenRepository : IMavenRepository
99
{
1010
readonly string base_url;
11-
static readonly HttpClient client;
11+
static readonly HttpClient client = new HttpClient ();
1212

1313
public string Name { get; }
1414

@@ -18,11 +18,6 @@ public MavenRepository (string baseUrl, string name)
1818
base_url = baseUrl.TrimEnd ('/');
1919
}
2020

21-
static MavenRepository ()
22-
{
23-
client = new HttpClient ();
24-
}
25-
2621
public bool TryGetFile (Artifact artifact, string filename, [NotNullWhen (true)] out Stream? stream)
2722
{
2823
// ex: https://repo1.maven.org/maven2/dev/chrisbanes/snapper/snapper/0.3.0/{filename}

‎tests/Java.Interop.Tools.Maven-Tests/DependenciesTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ public class DependenciesTests
3737
[TestCase ("com.facebook.android:facebook-android-sdk:14.1.1", "com.facebook.android:facebook-applinks:14.1.1 - compile;com.facebook.android:facebook-common:14.1.1 - compile;com.facebook.android:facebook-core:14.1.1 - compile;com.facebook.android:facebook-gamingservices:14.1.1 - compile;com.facebook.android:facebook-login:14.1.1 - compile;com.facebook.android:facebook-messenger:14.1.1 - compile;com.facebook.android:facebook-share:14.1.1 - compile;org.jetbrains.kotlin:kotlin-stdlib:1.5.10 - compile")]
3838
[TestCase ("com.airbnb.android:lottie:5.2.0", "androidx.appcompat:appcompat:1.3.1 - runtime;com.squareup.okio:okio:1.17.4 - runtime")]
3939
public void TestMavenCentralResolvedDependencies (string artifact, string expected)
40-
=> TestResolvedDependencies (MavenPomResolver.Central, artifact, expected);
40+
=> TestResolvedDependencies (MavenProjectResolver.Central, artifact, expected);
4141

42-
void TestResolvedDependencies (MavenPomResolver resolver, string artifact, string expected)
42+
void TestResolvedDependencies (MavenProjectResolver resolver, string artifact, string expected)
4343
{
4444
var art = Artifact.Parse (artifact);
4545
var project = ResolvedProject.FromArtifact (art, resolver);

0 commit comments

Comments
 (0)