Merge pull request #43524 from gayaldassanayake/fix-hierrarchical-pkg

Add DependencyManifest module to the PackageManifest dependency

compiler/ballerina-lang/src/main/java/io/ballerina/projects/internal/BlendedManifest.java

@@ -35,6 +35,7 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -132,11 +133,12 @@ public static BlendedManifest from(DependencyManifest dependencyManifest,
                     }
                 }
             } else {
+                Collection<String> moduleNames = existingDepOptional.isPresent() ?
+                        existingDepOptional.get().modules : Collections.emptyList();
                 depContainer.add(depInPkgManifest.org(), depInPkgManifest.name(), new Dependency(
                         depInPkgManifest.org(), depInPkgManifest.name(), depInPkgManifest.version(),
                         DependencyRelation.UNKNOWN, REPOSITORY_NOT_SPECIFIED,
-                        moduleNames(new DependencyManifest.Package(depInPkgManifest.name(), depInPkgManifest.org(),
-                                depInPkgManifest.version())), DependencyOrigin.USER_SPECIFIED));
+                        moduleNames, DependencyOrigin.USER_SPECIFIED));
                 continue;
             }
 

compiler/ballerina-lang/src/test/java/io/ballerina/projects/test/resolution/packages/ExistingProjectTests.java

@@ -60,9 +60,13 @@ public static Object[][] testCaseProvider() {
                 // 14. package contains 2 dependencies one of which is in Ballerina toml file thats not local
                 {"suite-existing_project", "case-0014", true},
                 {"suite-existing_project", "case-0014", false},
-                // 15. package updates transitive dependency from the Ballerian toml file that is not local
+                // 15. package updates transitive dependency from the Ballerina toml file that is not local
                 {"suite-existing_project", "case-0015", true},
-                {"suite-existing_project", "case-0015", false}
+                {"suite-existing_project", "case-0015", false},
+                // 16. package name is hierarchical, there are new versions in the central,
+                // and the older version is specified in Ballerina.toml and Dependencies.toml
+                {"suite-existing_project", "case-0016", true},
+                {"suite-existing_project", "case-0016", false}
         };
     }
 }

compiler/ballerina-lang/src/test/java/io/ballerina/projects/test/resolution/packages/internal/PackageResolutionTestCaseBuilder.java

@@ -156,8 +156,9 @@ private static DependencyManifest getDependencyManifest(Path dependenciesTomlPat
             }
 
             PackageDescriptor pkgDesc = Utils.getPkgDescFromNode(node.name().value(), null);
+            List<DependencyManifest.Module> modules = Utils.getDependencyModules(pkgDesc, attrs.get("modules"));
             recordedDeps.add(new DependencyManifest.Package(pkgDesc.name(), pkgDesc.org(), pkgDesc.version(),
-                    scope.getValue(), isTransitive, Collections.emptyList(), Collections.emptyList()));
+                    scope.getValue(), isTransitive, Collections.emptyList(), modules));
         }
         return DependencyManifest.from("2.0.0", null, recordedDeps, Collections.emptyList());
     }

compiler/ballerina-lang/src/test/java/io/ballerina/projects/test/resolution/packages/internal/Utils.java

@@ -17,6 +17,7 @@
  */
 package io.ballerina.projects.test.resolution.packages.internal;
 
+import io.ballerina.projects.DependencyManifest;
 import io.ballerina.projects.DependencyResolutionType;
 import io.ballerina.projects.ModuleName;
 import io.ballerina.projects.PackageDependencyScope;
@@ -26,6 +27,10 @@
 import io.ballerina.projects.PackageVersion;
 import io.ballerina.projects.environment.ModuleLoadRequest;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
 /**
  * Contains utility methods used throughout the test framework.
  *
@@ -49,6 +54,19 @@ public static PackageDependencyScope getDependencyScope(Object scopeValue) {
         }
     }
 
+    public static List<DependencyManifest.Module> getDependencyModules(PackageDescriptor pkgDesc, Object modulesValue) {
+        if (modulesValue == null) {
+            return Collections.emptyList();
+        }
+        List<DependencyManifest.Module> modules = new ArrayList<>();
+        String modulesStr = modulesValue.toString();
+        String[] moduleNames = modulesStr.split(",");
+        for (String moduleName : moduleNames) {
+            modules.add(new DependencyManifest.Module(pkgDesc.org().value(), pkgDesc.name().value(), moduleName));
+        }
+        return modules;
+    }
+
     public static ModuleLoadRequest getModuleLoadRequest(String name,
                                                          PackageDependencyScope scope,
                                                          DependencyResolutionType resolutionType) {

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/Ballerina_toml.dot

@@ -0,0 +1,3 @@
+digraph "BallerinaToml" {
+    "samjs/qux.foo:1.0.2"
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/Dependencies_toml.dot

@@ -0,0 +1,5 @@
+digraph "example1" {
+    "samejs/app:0.1.0" -> "samjs/qux.foo:1.0.2"
+
+    "samjs/qux.foo:1.0.2" [modules = "qux.foo"]
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/app.dot

@@ -0,0 +1,3 @@
+digraph "samejs/app:0.1.0" {
+    "samjs/qux.foo"
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/case-description.md

@@ -0,0 +1,13 @@
+# Package name is hierarchical, there are new versions in the central, and the older version is specified in Ballerina.toml and Dependencies.toml
+
+1. User's package has `samjs/qux.foo:1.0.2` as a dependency in Dependencies.toml.
+2. A newer version `samjs/qux.foo:1.0.5` has been released to central.
+3. User specifies `samjs/qux.foo:1.0.2` in Ballerina.toml
+4. User now builds the package
+
+## Expected behavior
+
+### Sticky == true
+No changes to Dependency graph
+### Sticky == false
+Dependency graph should be updated to have `samjs/qux.foo:1.0.5`

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/expected-graph-nosticky.dot

@@ -0,0 +1,3 @@
+digraph "example1" {
+    "samejs/app:0.1.0" -> "samjs/qux.foo:1.0.5"
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/case-0016/expected-graph-sticky.dot

@@ -0,0 +1,3 @@
+digraph "example1" {
+    "samejs/app:0.1.0" -> "samjs/qux.foo:1.0.2"
+}

compiler/ballerina-lang/src/test/resources/package-resolution/suite-existing_project/repositories/central.dot

@@ -102,5 +102,11 @@ digraph central {
     subgraph "ballerina/http:1.4.0" {
         "ballerina/http:1.4.0" -> "ballerina/io:1.0.2"
     }
+
+    subgraph "samjs/qux.foo:1.0.2" {
+    }
+
+    subgraph "samjs/qux.foo:1.0.5" {
+    }
 }
 

project-api/project-api-test/src/test/java/io/ballerina/projects/test/PackageResolutionTests.java

@@ -96,7 +96,6 @@
  *
  * @since 2.0.0
  */
-@Test(groups = "broken")
 public class PackageResolutionTests extends BaseTest {
     private static final Path RESOURCE_DIRECTORY = Path.of(
             "src/test/resources/projects_for_resolution_tests").toAbsolutePath();
@@ -115,6 +114,7 @@ public void setup() throws IOException {
         replaceDependenciesTomlVersion(tempResourceDir.resolve("package_n"));
         replaceDependenciesTomlVersion(tempResourceDir.resolve("package_p_withDep"));
         replaceDependenciesTomlVersion(tempResourceDir.resolve("package_p_withoutDep"));
+        replaceDependenciesTomlVersion(tempResourceDir.resolve("package_z"));
     }
 
     @Test(description = "tests resolution with zero direct dependencies")
@@ -561,8 +561,8 @@ public void testProjectWithManyDependencies(boolean optimizeDependencyCompilatio
         if (os instanceof UnixOperatingSystemMXBean unixOperatingSystemMXBean) {
             initialOpenCount = unixOperatingSystemMXBean.getOpenFileDescriptorCount();
         }
-        Project project = TestUtils.loadProject(
-                Path.of("projects_for_resolution_tests/ultimate_package_resolution/package_http"),
+        Project project = TestUtils.loadProject(tempResourceDir
+                        .resolve("ultimate_package_resolution/package_http"),
                 BuildOptions.builder().setOptimizeDependencyCompilation(optimizeDependencyCompilation).build());
 
         PackageCompilation compilation = project.currentPackage().getCompilation();
@@ -617,9 +617,11 @@ public void testBalaProjectDependencyResolution() {
     }
 
     @Test (description = "Resolve a dependency from the local repo", dataProvider = "optimizeDependencyCompilation")
-    public void testResolveDependencyFromCustomRepo(boolean optimizeDependencyCompilation) {
+    public void testResolveDependencyFromCustomRepo(boolean optimizeDependencyCompilation) throws IOException {
         Path projectDirPath = tempResourceDir.resolve("package_b");
+        replaceDistributionVersionOfDependenciesToml(projectDirPath, RepoUtils.getBallerinaShortVersion());
         String dependencyContent = """
+                
                 [[dependency]]
                 org = "samjs"
                 name = "package_c"
@@ -634,14 +636,17 @@ public void testResolveDependencyFromCustomRepo(boolean optimizeDependencyCompil
         BuildProject project = TestUtils.loadBuildProject(projectEnvironmentBuilder, projectDirPath, buildOptions);
 
         // 2) set local repository to dependency
-        project.currentPackage().dependenciesToml().orElseThrow().modify().withContent(dependencyContent).apply();
+        String currentContent = project.currentPackage().ballerinaToml().get()
+                .tomlDocument().textDocument().toString();
+        String updatedContent = currentContent.concat(dependencyContent);
+        project.currentPackage().ballerinaToml().orElseThrow().modify().withContent(updatedContent).apply();
 
         // 3) Compile and check the diagnostics
         PackageCompilation compilation = project.currentPackage().getCompilation();
         DiagnosticResult diagnosticResult = compilation.diagnosticResult();
 
         // 4) The dependency is expected to load from distribution cache, hence zero diagnostics
-        Assert.assertEquals(diagnosticResult.errorCount(), 2);
+        Assert.assertEquals(diagnosticResult.errorCount(), 0);
     }
 
     // For this to be enabled, #31026 should be fixed.
@@ -772,8 +777,8 @@ public void testPackageResolutionOfDependencyInvalidRepository() {
         // Check whether there are any diagnostics
         DiagnosticResult diagnosticResult = compilation.diagnosticResult();
         diagnosticResult.diagnostics().forEach(OUT::println);
-        Assert.assertEquals(diagnosticResult.errorCount(), 4, "Unexpected compilation diagnostics");
-        Assert.assertEquals(diagnosticResult.warningCount(), 1, "Unexpected compilation diagnostics");
+        Assert.assertEquals(diagnosticResult.errorCount(), 3, "Unexpected compilation diagnostics");
+        Assert.assertEquals(diagnosticResult.warningCount(), 2, "Unexpected compilation diagnostics");
 
         Iterator<Diagnostic> diagnosticIterator = diagnosticResult.diagnostics().iterator();
         Assert.assertTrue(diagnosticIterator.next().toString().contains(
@@ -782,8 +787,8 @@ public void testPackageResolutionOfDependencyInvalidRepository() {
         // Check dependency cannot be resolved diagnostic
         Assert.assertEquals(
                 diagnosticIterator.next().toString(),
-                "ERROR [Ballerina.toml:(21:12,21:21)] invalid 'repository' under [dependency]: 'repository' " +
-                        "can only have the value 'local'");
+                "WARNING [Ballerina.toml:(17:1,21:21)] Provided custom repository (invalid) cannot be found in the " +
+                        "Settings.toml. ");
         Assert.assertEquals(diagnosticIterator.next().toString(),
                             "ERROR [fee.bal:(1:1,1:16)] cannot resolve module 'ccc/ddd'");
         Assert.assertEquals(diagnosticIterator.next().toString(),
@@ -878,6 +883,29 @@ public void testDependencyResolutionWithTransitiveDependencyBuiltFromHigherDistV
         Assert.assertEquals(diagnosticResult.diagnosticCount(), 0, "Unexpected compilation diagnostics");
     }
 
+    @Test(description = "Resolve dependencies when a dependency has a hierarchical name and is specified in " +
+            "the Ballerina.toml and the Dependencies.toml of the dependent")
+    public void testDependencyWithHierrarchicalNameInDepTomlAndBalToml() {
+        Path projectDirPath = tempResourceDir.resolve("package_z");
+
+        BCompileUtil.compileAndCacheBala("projects_for_resolution_tests/package_zz_1_0_0");
+        BCompileUtil.compileAndCacheBala("projects_for_resolution_tests/package_zz_1_0_2");
+
+        BuildOptions.BuildOptionsBuilder buildOptionsBuilder = BuildOptions.builder();
+        buildOptionsBuilder.setSticky(true);
+        BuildOptions buildOptions = buildOptionsBuilder.build();
+        Project loadProject = TestUtils.loadBuildProject(projectDirPath, buildOptions);
+
+        PackageCompilation compilation = loadProject.currentPackage().getCompilation();
+
+        DependencyGraph<ResolvedPackageDependency> dependencyGraph = compilation.getResolution().dependencyGraph();
+        ResolvedPackageDependency packageO =
+                dependencyGraph.getNodes().stream().filter(
+                        node -> node.packageInstance().manifest().name().toString().equals("foo.bar")
+                ).findFirst().orElseThrow();
+        Assert.assertEquals(packageO.packageInstance().manifest().version().toString(), "1.0.0");
+    }
+
     private void replaceDependenciesTomlVersion(Path projectPath) throws IOException {
         String currentDistrVersion = RepoUtils.getBallerinaShortVersion();
         Path dependenciesTomlTemplatePath = projectPath.resolve("Dependencies-template.toml");

project-api/project-api-test/src/test/resources/projects_for_resolution_tests/package_z/Ballerina.toml

@@ -0,0 +1,9 @@
+[package]
+org = "samjs"
+name = "package_z"
+version = "0.1.0"
+
+[[dependency]]
+org = "samjs"
+name = "foo.bar"
+version = "1.0.0"

project-api/project-api-test/src/test/resources/projects_for_resolution_tests/package_z/Dependencies-template.toml

@@ -0,0 +1,28 @@
+# AUTO-GENERATED FILE. DO NOT MODIFY.
+
+# This file is auto-generated by Ballerina for managing dependency versions.
+# It should not be modified by hand.
+
+[ballerina]
+dependencies-toml-version = "2"
+distribution-version = "**INSERT_DISTRIBUTION_VERSION_HERE**"
+
+[[package]]
+org = "samjs"
+name = "foo.bar"
+version = "1.0.0"
+modules = [
+    {org = "samjs", packageName = "foo.bar", moduleName = "foo.bar"}
+]
+
+[[package]]
+org = "samjs"
+name = "package_z"
+version = "0.1.0"
+dependencies = [
+    {org = "samjs", name = "foo.bar"}
+]
+modules = [
+    {org = "samjs", packageName = "package_z", moduleName = "package_z"}
+]
+

project-api/project-api-test/src/test/resources/projects_for_resolution_tests/package_z/main.bal

@@ -0,0 +1,4 @@
+import samjs/foo.bar as _;
+
+public function func_z() {
+}

project-api/project-api-test/src/test/resources/projects_for_resolution_tests/package_zz_1_0_0/Ballerina.toml

@@ -0,0 +1,4 @@
+[package]
+org = "samjs"
+name = "foo.bar"
+version = "1.0.0"

project-api/project-api-test/src/test/resources/projects_for_resolution_tests/package_zz_1_0_0/main.bal

@@ -0,0 +1,2 @@
+public function func() {
+}

project-api/project-api-test/src/test/resources/projects_for_resolution_tests/package_zz_1_0_2/Ballerina.toml

@@ -0,0 +1,4 @@
+[package]
+org = "samjs"
+name = "foo.bar"
+version = "1.0.2"

project-api/project-api-test/src/test/resources/projects_for_resolution_tests/package_zz_1_0_2/main.bal

@@ -0,0 +1,2 @@
+public function func() {
+}

project-api/project-api-test/src/test/resources/testng.xml

@@ -20,12 +20,7 @@ under the License.
 <!DOCTYPE suite SYSTEM "https://testng.org/testng-1.0.dtd" >
 
 <suite name="ballerina-projects-test-suite">
-    <test name="ballerina-projects-test" preserve-order="true">
-        <groups>
-            <run>
-                <exclude name="broken"/>
-            </run>
-        </groups>
+    <test name="ballerina-projects-test">
         <packages>
             <package name="io.ballerina.projects.test.*" />
         </packages>