Merge pull request #43550 from gayaldassanayake/fix-hier-pkg-8.x

[2201.8.x] Add dep-manifest modules to the pkg-manifest dep

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;
@@ -133,11 +134,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

@@ -157,8 +157,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);
     }

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.
  *
@@ -48,6 +53,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

@@ -98,7 +98,6 @@
  *
  * @since 2.0.0
  */
-@Test(groups = "broken")
 public class PackageResolutionTests extends BaseTest {
     private static final Path RESOURCE_DIRECTORY = Paths.get(
             "src/test/resources/projects_for_resolution_tests").toAbsolutePath();
@@ -117,6 +116,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")
@@ -193,7 +193,6 @@ public void testDependencyGraphWithInvalidBuildFile() {
         BuildOptions.BuildOptionsBuilder buildOptionsBuilder = BuildOptions.builder();
         buildOptionsBuilder.setSticky(false);
         BuildOptions buildOptions = buildOptionsBuilder.build();
-
         Project loadProject = TestUtils.loadBuildProject(projectDirPath, buildOptions);
 
         PackageCompilation compilation = loadProject.currentPackage().getCompilation();
@@ -365,19 +364,16 @@ public void testClearingEnvironmentCache() throws IOException {
                 "projectB");
         FileUtils.deleteDirectory(projectBCachePath.toFile());
 
-
         // Step 1 : Build ProjectB1 and Cache
         CompileResult depCompileResult = BCompileUtil.compileAndCacheBala("projects_for_resolution_tests/projectB1");
         if (depCompileResult.getErrorCount() > 0) {
             Assert.fail("Package B contains compilations error");
         }
 
-
         // Step 2 : Build ProjectA with ProjectB as an import
         Path projectA = tempResourceDir.resolve("projectA");
         Project loadProjectA = TestUtils.loadBuildProject(projectA);
 
-
         // Step 3 : Get compilation of ProjectA and verify dependencies
         PackageCompilation compilation = loadProjectA.currentPackage().getCompilation();
         DependencyGraph<ResolvedPackageDependency> dependencyGraph = compilation.getResolution().dependencyGraph();
@@ -388,7 +384,6 @@ public void testClearingEnvironmentCache() throws IOException {
             }
         }
 
-
         // Step 4 : Modify projectA to be blank
 
         // - Step 4.1 : Get the main.bal file document for ProjectA
@@ -407,7 +402,6 @@ public void testClearingEnvironmentCache() throws IOException {
         Document document = defaultModuleProjectA.document(mainDocumentId.get());
         document.modify().withContent(newMainProjectAContent).apply();
 
-
         // Step 5 : Compile ProjectA and verify dependency
         compilation = loadProjectA.currentPackage().getCompilation();
         dependencyGraph = compilation.getResolution().dependencyGraph();
@@ -418,11 +412,9 @@ public void testClearingEnvironmentCache() throws IOException {
             }
         }
 
-
         // Step 6 : Clear ProjectB cache
         FileUtils.deleteDirectory(projectBCachePath.toFile());
 
-
         // Step 7 : Compile ProjectB2 and cache
         depCompileResult = BCompileUtil.compileAndCacheBala("projects_for_resolution_tests/projectB2");
         if (depCompileResult.getErrorCount() > 0) {
@@ -432,7 +424,6 @@ public void testClearingEnvironmentCache() throws IOException {
         // Step 8 : Clear ProjectB cache
         FileUtils.deleteDirectory(projectBCachePath.toFile());
 
-
         // Step 9 : Modify ProjectA again with the old content
 
         // - Step 9.1 : Get the main.bal file document
@@ -453,7 +444,6 @@ public void testClearingEnvironmentCache() throws IOException {
         document = defaultModuleProjectA.document(mainDocumentId.get());
         document.modify().withContent(oldMainProjectAContent).apply();
 
-
         // Step 10 : Compile ProjectA and verify dependency
         compilation = loadProjectA.currentPackage().getCompilation();
         Assert.assertNotEquals(compilation.diagnosticResult().errorCount(), 0, "Package A has compiled successfully " +
@@ -547,7 +537,6 @@ public void testProjectWithTransitiveTestDependencies() throws IOException {
         Assert.assertEquals(depGraphOfBala.getNodes().size(), 1);
     }
 
-    // TODO: enable after https://github.com/ballerina-platform/ballerina-lang/pull/31972 is merged
     @Test(description = "Ultimate test case")
     public void testProjectWithManyDependencies() {
         BCompileUtil.compileAndCacheBala(
@@ -563,12 +552,11 @@ public void testProjectWithManyDependencies() {
 
         OperatingSystemMXBean os = ManagementFactory.getOperatingSystemMXBean();
         long initialOpenCount = 0;
-        if (os instanceof UnixOperatingSystemMXBean) {
-            UnixOperatingSystemMXBean unixOperatingSystemMXBean = (UnixOperatingSystemMXBean) os;
+        if (os instanceof UnixOperatingSystemMXBean unixOperatingSystemMXBean) {
             initialOpenCount = unixOperatingSystemMXBean.getOpenFileDescriptorCount();
         }
-        Project project = BCompileUtil.loadProject(
-                "projects_for_resolution_tests/ultimate_package_resolution/package_http");
+        Project project = TestUtils.loadProject(tempResourceDir
+                        .resolve("ultimate_package_resolution/package_http"));
 
         PackageCompilation compilation = project.currentPackage().getCompilation();
         JBallerinaBackend jBallerinaBackend = JBallerinaBackend.from(compilation, JvmTarget.JAVA_17);
@@ -577,8 +565,7 @@ public void testProjectWithManyDependencies() {
         diagnosticResult.errors().forEach(OUT::println);
         Assert.assertEquals(diagnosticResult.diagnosticCount(), 0, "Unexpected compilation diagnostics");
 
-        if (os instanceof UnixOperatingSystemMXBean) {
-            UnixOperatingSystemMXBean unixOperatingSystemMXBean = (UnixOperatingSystemMXBean) os;
+        if (os instanceof UnixOperatingSystemMXBean unixOperatingSystemMXBean) {
             Assert.assertEquals(initialOpenCount, unixOperatingSystemMXBean.getOpenFileDescriptorCount());
         }
 
@@ -621,30 +608,36 @@ public void testBalaProjectDependencyResolution() {
         List<ResolvedPackageDependency> nodeInGraph = dependencyGraph.toTopologicallySortedList();
         Assert.assertEquals(nodeInGraph.size(), 2);
     }
-    // For this to be enabled, #31026 should be fixed.
-    @Test(enabled = false, dependsOnMethods = "testResolveDependencyFromUnsupportedCustomRepo")
-    public void testResolveDependencyFromCustomRepo() {
+
+    @Test
+    public void testResolveDependencyFromCustomRepo() throws IOException {
         Path projectDirPath = tempResourceDir.resolve("package_b");
-        String dependencyContent = "[[dependency]]\n" +
-                "org = \"samjs\"\n" +
-                "name = \"package_c\"\n" +
-                "version = \"0.1.0\"\n" +
-                "repository = \"local\"";
+        replaceDistributionVersionOfDependenciesToml(projectDirPath, RepoUtils.getBallerinaShortVersion());
+        String dependencyContent = """
+                
+                [[dependency]]
+                org = "samjs"
+                name = "package_c"
+                version = "0.1.0"
+                repository = "local\"""";
 
         // 1) load the build project
         Environment environment = EnvironmentBuilder.getBuilder().setUserHome(USER_HOME).build();
         ProjectEnvironmentBuilder projectEnvironmentBuilder = ProjectEnvironmentBuilder.getBuilder(environment);
         BuildProject project = TestUtils.loadBuildProject(projectEnvironmentBuilder, projectDirPath);
 
         // 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.
@@ -774,8 +767,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(
@@ -784,8 +777,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(),
@@ -880,6 +873,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"}
+]
+