removed untested APIs and refactored.

vimanyu · vimanyu · commit f4b84d1eea8f · 2021-08-05T19:28:24.000-07:00
diff --git a/source/AndroidResolver/src/GradleResolver.cs b/source/AndroidResolver/src/GradleResolver.cs
@@ -106,50 +106,26 @@ private void LogMissingDependenciesError(List<string> missingArtifacts) {
         }
 
         /// <summary>
-        /// From a list of dependencies generate a list of Maven / Gradle / Ivy package spec
-        /// strings.
+        /// Get package spec from a dependency.
         /// </summary>
-        /// <param name="dependencies">Dependency instances to query for package specs.</param>
-        /// <returns>Dictionary of Dependency instances indexed by package spec strings.</returns>
-        internal static Dictionary<string, string> DependenciesToPackageSpecs(
-                IEnumerable<Dependency> dependencies) {
-            var sourcesByPackageSpec = new Dictionary<string, string>();
-            foreach (var dependency in dependencies) {
-                // Convert the legacy "LATEST" version spec to a Gradle version spec.
-                var packageSpec = dependency.Version.ToUpper() == "LATEST" ?
-                    dependency.VersionlessKey + ":+" : dependency.Key;
-                var source = CommandLine.SplitLines(dependency.CreatedBy)[0];
-                string sources;
-                if (sourcesByPackageSpec.TryGetValue(packageSpec, out sources)) {
-                    sources = sources + ", " + source;
-                } else {
-                    sources = source;
-                }
-                sourcesByPackageSpec[packageSpec] = sources;
-            }
-            return sourcesByPackageSpec;
+        /// <param name="dependency">Dependency instance to query for package spec.</param>
+        internal static string DependencyToPackageSpec(Dependency dependency) {
+            return dependency.Version.ToUpper() == "LATEST" ?
+                dependency.VersionlessKey + ":+" : dependency.Key;
         }
 
         /// <summary>
         /// From a list of dependencies generate a list of Maven / Gradle / Ivy package spec
-        /// strings along with their corresponding dependency versions.
+        /// strings.
         /// </summary>
         /// <param name="dependencies">Dependency instances to query for package specs.</param>
-        /// <returns>Dictionary where the key is a package spec string and value is a
-        /// 3 item list of sources string, dependency version less string,
-        /// and a dependency version string. </returns>
-        internal static Dictionary<string, List<string>> DependenciesToPackageSpecsWithVersions(
+        /// <returns>Dictionary of Dependency instances indexed by package spec strings.</returns>
+        internal static Dictionary<string, string> DependenciesToPackageSpecs(
                 IEnumerable<Dependency> dependencies) {
-            // To keep it simple to iterate and process, create 3 separate
-            // dictionaries to record required metadata about a package spec
-            // (version and versionless key).
             var sourcesByPackageSpec = new Dictionary<string, string>();
-            var versionsByPackageSpec = new Dictionary<string, string>();
-            var versionlessKeysByPackageSpec = new Dictionary<string, string>();
             foreach (var dependency in dependencies) {
                 // Convert the legacy "LATEST" version spec to a Gradle version spec.
-                var packageSpec = dependency.Version.ToUpper() == "LATEST" ?
-                    dependency.VersionlessKey + ":+" : dependency.Key;
+                var packageSpec = DependencyToPackageSpec(dependency);
                 var source = CommandLine.SplitLines(dependency.CreatedBy)[0];
                 string sources;
                 if (sourcesByPackageSpec.TryGetValue(packageSpec, out sources)) {
@@ -158,24 +134,8 @@ internal static Dictionary<string, List<string>> DependenciesToPackageSpecsWithV
                     sources = source;
                 }
                 sourcesByPackageSpec[packageSpec] = sources;
-                versionsByPackageSpec[packageSpec] = dependency.Version;
-                versionlessKeysByPackageSpec[packageSpec] = dependency.VersionlessKey;
-            }
-            // Create a new dictionary which combines the data from sources and
-            // versions dictionaries. Key is packageSpec string and value is a
-            // three-item list of strings of sources, version less string
-            // and dependency version.
-            // NOTE: tuples are a more elegant way of doing this but they work
-            // only in .NET frameworks >= 4.0.
-            var mergedByPackageSpec = new Dictionary<string, List<string>>();
-            foreach(var item in sourcesByPackageSpec) {
-                mergedByPackageSpec[item.Key] = new List<string>{
-                    sourcesByPackageSpec[item.Key],
-                    versionlessKeysByPackageSpec[item.Key],
-                    versionsByPackageSpec[item.Key]
-                };
             }
-            return mergedByPackageSpec;
+            return sourcesByPackageSpec;
         }
 
         /// <summary>
diff --git a/source/AndroidResolver/src/PlayServicesResolver.cs b/source/AndroidResolver/src/PlayServicesResolver.cs
@@ -2090,17 +2090,6 @@ public static IList<KeyValuePair<string, string>> GetPackageSpecs(
                 GradleResolver.DependenciesToPackageSpecs(GetOrReadDependencies(dependencies))));
         }
 
-        /// <summary>
-        /// Get the list of Android package specs referenced by the project and the sources they're
-        /// loaded from along with their corresponding dependency versions.
-        /// </summary>
-        /// <returns>Dictionary of the form {<packageSpec>: [sourceString, dependencyVersion]}.</returns>
-        public static IList<KeyValuePair<string, List<string>>> GetPackageSpecsWithVersions(
-                IEnumerable<Dependency> dependencies = null) {
-            return new List<KeyValuePair<string, List<string>>>(new SortedList<string, List<string>>(
-            GradleResolver.DependenciesToPackageSpecsWithVersions(GetOrReadDependencies(dependencies))));
-        }
-
         /// <summary>
         /// Get the list of Maven repo URIs required for Android libraries in this project.
         /// </summary>
@@ -2213,40 +2202,45 @@ internal static IList<string> GradleDependenciesLines(
                 // If different packages have different versions of the same dependency,
                 // we want to activate only the highest version but still include the
                 // other versions as commented out dependency lines.
-                var dependenciesMaxVersions = new Dictionary<string, string>();
-                string dependencyMaxVersion;
+                var dependenciesMaxVersions = new Dictionary<string, Dependency>();
+                Dependency dependencyMaxVersion;
+                // Maintain a set of packages which we want to comment out.
+                HashSet<string> packageSpecStringsToComment = new HashSet<string>();
                 foreach( var dependency in dependencies) {
                     if (dependenciesMaxVersions.TryGetValue(dependency.VersionlessKey, out dependencyMaxVersion)){
-                        if(versionComparer.Compare(dependency.Version, dependencyMaxVersion) < 0) {
-                            dependenciesMaxVersions[dependency.VersionlessKey] = dependency.Version;
+                        if(versionComparer.Compare(dependency.Version, dependencyMaxVersion.Version) < 0) {
+                            dependenciesMaxVersions[dependency.VersionlessKey] = dependency;
+                            // We found a new larger version. Comment out older one
+                            // Build a single item list since `GetPackageSpecs`
+                            // accepts an IEnumerable type.
+                            var packageSpecString = GradleResolver.DependencyToPackageSpec(dependencyMaxVersion);
+                            packageSpecStringsToComment.Add(packageSpecString);
+                        }
+                        else {
+                            // Dependency version is smaller than current max.
+                            var packageSpecString = GradleResolver.DependencyToPackageSpec(dependency);
+                            packageSpecStringsToComment.Add(packageSpecString);
                         }
                     }
                     else {
-                        dependenciesMaxVersions[dependency.VersionlessKey] = dependency.Version;
+                        // First time encountering this dependency.
+                        dependenciesMaxVersions[dependency.VersionlessKey] = dependency;
                     }
                 }
-                foreach (var packageSpecAndSourcesWithVersions in GetPackageSpecsWithVersions(dependencies: dependencies)){
-                    var packageSpecString = packageSpecAndSourcesWithVersions.Key;
-                    var packageSourcesString = packageSpecAndSourcesWithVersions.Value[0];
-                    var packageVersionlessKey = packageSpecAndSourcesWithVersions.Value[1];
-                    var packageVersion = packageSpecAndSourcesWithVersions.Value[2];
-
+                foreach (var packageSpecAndSources in GetPackageSpecs(dependencies: dependencies)) {
                     string line = "    ";
-                    // If this is not the highest version of this dependency, add a line
-                    // but comment it out by adding leading slashes.
-                    if(dependenciesMaxVersions[packageVersionlessKey] != packageVersion) {
+                    if (packageSpecStringsToComment.Contains(packageSpecAndSources.Key)) {
                         PlayServicesResolver.Log(
                             String.Format(
                                 "Ignoring duplicate package {0} with older version.",
-                                 packageSpecString),
+                                 packageSpecAndSources.Key),
                                  level: LogLevel.Info
                             );
                         line += "// ";
                     }
-
                     line += String.Format(
-                            "{0} '{1}' // {2}", includeStatement, packageSpecString,
-                            packageSourcesString);
+                            "{0} '{1}' // {2}", includeStatement, packageSpecAndSources.Key,
+                            packageSpecAndSources.Value);
                     lines.Add(line);
                 }
                 if (includeDependenciesBlock) lines.Add("}");
diff --git a/source/AndroidResolver/test/AndroidResolverIntegrationTestsUnityProject/Assets/ExternalDependencyManager/Editor/TestAdditionalDuplicateDependencies.template b/source/AndroidResolver/test/AndroidResolverIntegrationTestsUnityProject/Assets/ExternalDependencyManager/Editor/TestAdditionalDuplicateDependencies.template
@@ -1,8 +1,11 @@
 <dependencies>
   <androidPackages>
-    <!-- Duplicate entry for package with a different version. -->
+    <!-- Duplicate entry for package with a different version (higher). -->
+    <androidPackage spec="com.google.android.gms:play-services-base:18.0.1" />
+    <!-- Duplicate entry for package with a different version (lower). -->
     <androidPackage spec="com.google.android.gms:play-services-base:12.0.1" />
     <!-- Duplicate entry for package with the exact same version. -->
     <androidPackage spec="com.android.support:support-annotations:26.1.0"/>
   </androidPackages>
 </dependencies>
+
diff --git a/source/AndroidResolver/test/AndroidResolverIntegrationTestsUnityProject/ExpectedArtifacts/NoExport/GradleTemplateDuplicatePackages/mainTemplate.gradle b/source/AndroidResolver/test/AndroidResolverIntegrationTestsUnityProject/ExpectedArtifacts/NoExport/GradleTemplateDuplicatePackages/mainTemplate.gradle
@@ -47,8 +47,9 @@ dependencies {
     implementation fileTree(dir: 'libs', include: ['*.jar'])
 // Android Resolver Dependencies Start
     compile 'com.android.support:support-annotations:26.1.0' // Assets/ExternalDependencyManager/Editor/TestDependencies.xml:4
-    // compile 'com.google.android.gms:play-services-base:12.0.1' // Assets/ExternalDependencyManager/Editor/TestAdditionalDuplicateDependencies.xml:4
-    compile 'com.google.android.gms:play-services-base:15.0.1' // Assets/ExternalDependencyManager/Editor/TestAdditionalDependencies.xml:3
+    // compile 'com.google.android.gms:play-services-base:12.0.1' // Assets/ExternalDependencyManager/Editor/TestAdditionalDuplicateDependencies.xml:6
+    // compile 'com.google.android.gms:play-services-base:15.0.1' // Assets/ExternalDependencyManager/Editor/TestAdditionalDependencies.xml:3
+    compile 'com.google.android.gms:play-services-base:18.0.1' // Assets/ExternalDependencyManager/Editor/TestAdditionalDuplicateDependencies.xml:4
     compile 'com.google.firebase:firebase-app-unity:5.1.1' // Assets/ExternalDependencyManager/Editor/TestDependencies.xml:10
     compile 'com.google.firebase:firebase-common:16.0.0' // Google.AndroidResolverIntegrationTests.SetupDependencies
     compile 'org.test.psr:classifier:1.0.1:foo@aar' // Assets/ExternalDependencyManager/Editor/TestDependencies.xml:12
