Address the code review

cli/ballerina-cli/build.gradle

@@ -58,6 +58,7 @@ dependencies {
     implementation group: 'org.ow2.asm', name: 'asm', version: "${project.ow2AsmVersion}"
     implementation group: 'org.ow2.asm', name: 'asm-commons', version: "${project.ow2AsmCommonsVersion}"
     implementation group: 'org.ow2.asm', name: 'asm-tree', version: "${project.ow2AsmTreeVersion}"
+    implementation 'commons-io:commons-io'
 
     testImplementation "org.testng:testng:${project.testngVersion}"
     testImplementation "org.mockito:mockito-core:${project.mockitoCoreVersion}"

cli/ballerina-cli/src/main/java/io/ballerina/cli/cmd/PullCommand.java

@@ -40,13 +40,11 @@
 import picocli.CommandLine;
 
 import java.io.BufferedReader;
-import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Comparator;
 import java.util.List;
 
 import static io.ballerina.cli.cmd.Constants.PULL_COMMAND;
@@ -189,7 +187,7 @@ public void execute() {
         Repository targetRepository = null;
         if (repositoryName != null) {
             for (Repository repository : settings.getRepositories()) {
-                if (repository.id().equals(repositoryName)) {
+                if (repositoryName.equals(repository.id())) {
                     targetRepository = repository;
                     break;
                 }
@@ -221,33 +219,23 @@ public void execute() {
                     .resolve(ProjectConstants.BALA_DIR_NAME);
 
             try {
+                Path tmpDownloadDirectory = Files.createTempDirectory("ballerina-" + System.nanoTime());
                 mavenResolverClient.pullPackage(orgName, packageName, version,
-                        String.valueOf(mavenBalaCachePath.toAbsolutePath()));
-                Path balaDownloadPath = mavenBalaCachePath.resolve(orgName).resolve(packageName).resolve(version)
+                        String.valueOf(tmpDownloadDirectory.toAbsolutePath()));
+                Path balaDownloadPath = tmpDownloadDirectory.resolve(orgName).resolve(packageName).resolve(version)
                         .resolve(packageName + "-" + version + BALA_EXTENSION);
-                Path balaHashPath = mavenBalaCachePath.resolve(orgName).resolve(packageName).resolve(version)
-                        .resolve(packageName + "-" + version + BALA_EXTENSION + ".sha1");
-                Path temporaryExtractionPath = mavenBalaCachePath.resolve(orgName).resolve(packageName)
+                Path temporaryExtractionPath = tmpDownloadDirectory.resolve(orgName).resolve(packageName)
                         .resolve(version).resolve(PLATFORM);
                 ProjectUtils.extractBala(balaDownloadPath, temporaryExtractionPath);
                 Path packageJsonPath = temporaryExtractionPath.resolve("package.json");
-                BufferedReader bufferedReader = Files.newBufferedReader(packageJsonPath, StandardCharsets.UTF_8);
-                JsonObject resultObj = new Gson().fromJson(bufferedReader, JsonObject.class);
-                String platform = resultObj.get(PLATFORM).getAsString();
-                Path actualBalaPath = mavenBalaCachePath.resolve(orgName).resolve(packageName)
-                        .resolve(version).resolve(platform);
-                temporaryExtractionPath.toFile().renameTo(actualBalaPath.toFile());
-
-                Files.delete(balaDownloadPath);
-                Files.delete(balaHashPath);
-                Files.delete(balaDownloadPath.getParent().resolve("_remote.repositories"));
-                if (Files.exists(temporaryExtractionPath)) {
-                    Files.walk(temporaryExtractionPath)
-                            .sorted(Comparator.reverseOrder())
-                            .map(Path::toFile)
-                            .forEach(File::delete);
+                try (BufferedReader bufferedReader = Files.newBufferedReader(packageJsonPath, StandardCharsets.UTF_8)) {
+                    JsonObject resultObj = new Gson().fromJson(bufferedReader, JsonObject.class);
+                    String platform = resultObj.get(PLATFORM).getAsString();
+                    Path actualBalaPath = mavenBalaCachePath.resolve(orgName).resolve(packageName)
+                            .resolve(version).resolve(platform);
+                    org.apache.commons.io.FileUtils.copyDirectory(temporaryExtractionPath.toFile(),
+                            actualBalaPath.toFile());
                 }
-
             } catch (MavenResolverClientException e) {
                 errStream.println("unexpected error occurred while pulling package:" + e.getMessage());
                 CommandUtil.exitError(this.exitWhenFinish);

cli/ballerina-cli/src/main/java/io/ballerina/cli/cmd/PushCommand.java

@@ -52,7 +52,6 @@
 import java.nio.file.Paths;
 import java.nio.file.StandardOpenOption;
 import java.util.Arrays;
-import java.util.Comparator;
 import java.util.List;
 import java.util.stream.Collectors;
 import java.util.zip.ZipEntry;
@@ -66,7 +65,6 @@
 import static io.ballerina.projects.util.ProjectUtils.getAccessTokenOfCLI;
 import static io.ballerina.projects.util.ProjectUtils.initializeProxy;
 import static io.ballerina.runtime.api.constants.RuntimeConstants.SYSTEM_PROP_BAL_DEBUG;
-import static java.nio.file.Files.createDirectories;
 import static org.wso2.ballerinalang.programfile.ProgramFileConstants.SUPPORTED_PLATFORMS;
 
 /**
@@ -158,7 +156,7 @@ public void execute() {
                 boolean isCustomRepository = false;
                 Repository targetRepository = null;
                 for (Repository repository : settings.getRepositories()) {
-                    if (repository.id().equals(repositoryName)) {
+                    if (repositoryName.equals(repository.id())) {
                         isCustomRepository = true;
                         targetRepository = repository;
                         break;
@@ -199,15 +197,8 @@ public void execute() {
                 Proxy proxy = settings.getProxy();
                 mvnClient.setProxy(proxy.host(), proxy.port(), proxy.username(), proxy.password());
 
-                Path customRepoLocalCache = RepoUtils.createAndGetHomeReposPath()
-                        .resolve(ProjectConstants.REPOSITORIES_DIR)
-                        .resolve(targetRepository.id())
-                        .resolve(ProjectConstants.BALA_DIR_NAME)
-                        .resolve("temp");
-
-
                 if (balaPath == null && isCustomRepository) {
-                    pushPackage(project, mvnClient, customRepoLocalCache);
+                    pushPackage(project, mvnClient);
                 } else if (isCustomRepository) {
                     if (!balaPath.toFile().exists()) {
                         throw new ProjectException("path provided for the bala file does not exist: " + balaPath + ".");
@@ -216,7 +207,7 @@ public void execute() {
                         throw new ProjectException("file provided is not a bala file: " + balaPath + ".");
                     }
                     validatePackageMdAndBalToml(balaPath);
-                    pushBalaToCustomRepo(balaPath, mvnClient, customRepoLocalCache);
+                    pushBalaToCustomRepo(balaPath, mvnClient);
                 }
 
 
@@ -278,9 +269,9 @@ private void pushPackage(BuildProject project) {
         pushBalaToCustomRepo(balaFilePath);
     }
 
-    private void pushPackage(BuildProject project, MavenResolverClient client, Path customRepoPath) {
+    private void pushPackage(BuildProject project, MavenResolverClient client) {
         Path balaFilePath = validateBalaFile(project, this.balaPath);
-        pushBalaToCustomRepo(balaFilePath, client, customRepoPath);
+        pushBalaToCustomRepo(balaFilePath, client);
     }
 
     private void pushPackage(BuildProject project, CentralAPIClient client)
@@ -459,12 +450,7 @@ private void pushBalaToRemote(Path balaPath, CentralAPIClient client) {
         }
     }
 
-    private void pushBalaToCustomRepo(Path balaPath, MavenResolverClient client, Path customRepoPath) {
-        try {
-            createDirectories(customRepoPath);
-        } catch (IOException e) {
-            throw new ProjectException(e.getMessage());
-        }
+    private void pushBalaToCustomRepo(Path balaPath, MavenResolverClient client) {
         Path balaFileName = balaPath.getFileName();
         if (null != balaFileName) {
             ProjectEnvironmentBuilder defaultBuilder = ProjectEnvironmentBuilder.getDefaultBuilder();
@@ -476,13 +462,8 @@ private void pushBalaToCustomRepo(Path balaPath, MavenResolverClient client, Pat
             String version = balaProject.currentPackage().manifest().version().toString();
 
             try {
+                Path customRepoPath = Files.createTempDirectory("ballerina-" + System.nanoTime());
                 client.pushPackage(balaPath, org, name, version, customRepoPath);
-                if (Files.exists(customRepoPath)) {
-                    Files.walk(customRepoPath)
-                            .sorted(Comparator.reverseOrder())
-                            .map(Path::toFile)
-                            .forEach(File::delete);
-                }
             } catch (MavenResolverClientException | IOException e) {
                 throw new ProjectException(e.getMessage());
             }

cli/ballerina-cli/src/main/java/module-info.java

@@ -21,4 +21,5 @@
     requires io.ballerina.toml;
     requires io.ballerina.identifier;
     requires org.objectweb.asm;
+    requires org.apache.commons.io;
 }

compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java

@@ -95,7 +95,8 @@ private PackageResolution(PackageContext rootPackageContext, CompilationOptions
         this.resolutionOptions = getResolutionOptions(rootPackageContext, compilationOptions);
         ProjectEnvironment projectEnvContext = rootPackageContext.project().projectEnvironmentContext();
         this.packageResolver = projectEnvContext.getService(PackageResolver.class);
-        this.blendedManifest = createBlendedManifest(rootPackageContext, projectEnvContext);
+        this.blendedManifest = createBlendedManifest(rootPackageContext, projectEnvContext,
+                this.resolutionOptions.offline());
         diagnosticList.addAll(this.blendedManifest.diagnosticResult().allDiagnostics);
 
         this.moduleResolver = createModuleResolver(rootPackageContext, projectEnvContext);
@@ -481,12 +482,12 @@ private ModuleResolver createModuleResolver(PackageContext rootPackageContext,
     }
 
     private BlendedManifest createBlendedManifest(PackageContext rootPackageContext,
-                                                  ProjectEnvironment projectEnvContext) {
+                                                  ProjectEnvironment projectEnvContext, boolean offline) {
         Map<String, CustomPackageRepository> customPackageRepositoryMap =
                 projectEnvContext.getService(CustomPkgRepositoryContainer.class).getCustomPackageRepositories();
         return BlendedManifest.from(rootPackageContext.dependencyManifest(),
                 rootPackageContext.packageManifest(),
-                projectEnvContext.getService(LocalPackageRepository.class), customPackageRepositoryMap);
+                projectEnvContext.getService(LocalPackageRepository.class), customPackageRepositoryMap, offline);
     }
 
     private ResolutionOptions getResolutionOptions(PackageContext rootPackageContext,

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

@@ -63,7 +63,8 @@ private BlendedManifest(PackageContainer<Dependency> pkgContainer, DiagnosticRes
     public static BlendedManifest from(DependencyManifest dependencyManifest,
                                        PackageManifest packageManifest,
                                        AbstractPackageRepository localPackageRepository,
-                                       Map<String, CustomPackageRepository> customPackageRepositoryMap) {
+                                       Map<String, CustomPackageRepository> customPackageRepositoryMap,
+                                       boolean offline) {
         List<Diagnostic> diagnostics = new ArrayList<>();
         PackageContainer<Dependency> depContainer = new PackageContainer<>();
         for (DependencyManifest.Package pkgInDepManifest : dependencyManifest.packages()) {
@@ -114,10 +115,11 @@ public static BlendedManifest from(DependencyManifest dependencyManifest,
                     continue;
                 }
 
+                //TODO: check the behaviour for offline flag
                 if (!depInPkgManifest.repository().equals(ProjectConstants.LOCAL_REPOSITORY_NAME)) {
                     targettedRepository = customPackageRepositoryMap.get(depInPkgManifest.repository());
-                    if (!targettedRepository.isPackageExists(depInPkgManifest.org(), depInPkgManifest.name(),
-                            depInPkgManifest.version())) {
+                    if (!((CustomPackageRepository) targettedRepository).isPackageExists(depInPkgManifest.org(),
+                            depInPkgManifest.name(), depInPkgManifest.version(), offline)) {
                         var diagnosticInfo = new DiagnosticInfo(
                                 ProjectDiagnosticErrorCode.PACKAGE_NOT_FOUND.diagnosticId(),
                                 "Dependency version (" + depInPkgManifest.version() +

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

@@ -29,14 +29,6 @@ public CustomPackageRepository(Environment environment, Path cacheDirectory, Str
 
     @Override
     public Optional<Package> getPackage(ResolutionRequest request, ResolutionOptions options) {
-        String packageName = request.packageName().value();
-        String orgName = request.orgName().value();
-        String version = request.version().isPresent() ?
-                request.version().get().toString() : "0.0.0";
-        Path balaPath = this.fileSystemRepository.getPackagePath(orgName, packageName, version);
-        if (!Files.exists(balaPath)) {
-            getPackageFromRemoteRepo(orgName, packageName, version);
-        }
         return this.fileSystemRepository.getPackage(request, options);
     }
 
@@ -56,11 +48,18 @@ public Map<String, List<String>> getPackages() {
     public boolean isPackageExists(PackageOrg org,
                                    PackageName name,
                                    PackageVersion version) {
+        return this.fileSystemRepository.isPackageExists(org, name, version);
+    }
+
+    // This method should be called before calling other methods
+    public boolean isPackageExists(PackageOrg org,
+                                   PackageName name,
+                                   PackageVersion version, boolean offline) {
         boolean isPackageExist = this.fileSystemRepository.isPackageExists(org, name, version);
-        if (!isPackageExist) {
+        if (!isPackageExist && !offline) {
             return getPackageFromRemoteRepo(org.value(), name.value(), version.value().toString());
         }
-        return true;
+        return isPackageExist;
     }
 
     @Override
@@ -69,8 +68,6 @@ protected List<PackageVersion> getPackageVersions(PackageOrg org, PackageName na
             return Collections.emptyList();
         }
 
-        isPackageExists(org, name, version);
-
         Path balaPath = this.fileSystemRepository.getPackagePath(org.toString(), name.toString(), version.toString());
         if (Files.exists(balaPath)) {
             return Collections.singletonList(version);
@@ -82,10 +79,6 @@ protected List<PackageVersion> getPackageVersions(PackageOrg org, PackageName na
     @Override
     protected DependencyGraph<PackageDescriptor> getDependencyGraph(PackageOrg org, PackageName name,
                                                                     PackageVersion version) {
-        boolean packageExists = isPackageExists(org, name, version);
-        if (!packageExists) {
-            return DependencyGraph.emptyGraph();
-        }
         return this.fileSystemRepository.getDependencyGraph(org, name, version);
     }
 

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

@@ -9,19 +9,18 @@
 import io.ballerina.projects.internal.model.Proxy;
 import io.ballerina.projects.internal.model.Repository;
 import io.ballerina.projects.util.ProjectUtils;
+import org.apache.commons.io.FileUtils;
 import org.ballerinalang.maven.bala.client.MavenResolverClient;
 import org.ballerinalang.maven.bala.client.MavenResolverClientException;
 import org.ballerinalang.toml.exceptions.SettingsTomlException;
 import org.wso2.ballerinalang.util.RepoUtils;
 
 import java.io.BufferedReader;
-import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Comparator;
 
 import static io.ballerina.projects.util.ProjectConstants.BALA_EXTENSION;
 
@@ -75,34 +74,24 @@ public boolean getPackageFromRemoteRepo(String org,
                                                String name,
                                                String version) {
         try {
-            this.client.pullPackage(org, name, version, this.repoLocation);
-        } catch (MavenResolverClientException e) {
-            return false;
-        }
-
-        Path balaDownloadPath = Paths.get(this.repoLocation).resolve(org).resolve(name).resolve(version)
-                .resolve(name + "-" + version + BALA_EXTENSION);
-        Path balaHashPath = Paths.get(this.repoLocation).resolve(org).resolve(name).resolve(version)
-                .resolve(name + "-" + version + BALA_EXTENSION + ".sha1");
-        Path temporaryExtractionPath = Paths.get(this.repoLocation, org, name, version, PLATFORM);
-        try {
+            Path tmpDownloadDirectory = Files.createTempDirectory("ballerina-" + System.nanoTime());
+            client.pullPackage(org, name, version,
+                    String.valueOf(tmpDownloadDirectory.toAbsolutePath()));
+            Path balaDownloadPath = tmpDownloadDirectory.resolve(org).resolve(name).resolve(version)
+                    .resolve(name + "-" + version + BALA_EXTENSION);
+            Path temporaryExtractionPath = tmpDownloadDirectory.resolve(org).resolve(name)
+                    .resolve(version).resolve(PLATFORM);
             ProjectUtils.extractBala(balaDownloadPath, temporaryExtractionPath);
             Path packageJsonPath = temporaryExtractionPath.resolve("package.json");
-            BufferedReader bufferedReader = Files.newBufferedReader(packageJsonPath, StandardCharsets.UTF_8);
-            JsonObject resultObj = new Gson().fromJson(bufferedReader, JsonObject.class);
-            String platform = resultObj.get(PLATFORM).getAsString();
-            Path actualBalaPath = Paths.get(this.repoLocation, org, name, version, platform);
-            temporaryExtractionPath.toFile().renameTo(actualBalaPath.toFile());
-            Files.delete(balaDownloadPath);
-            Files.delete(balaHashPath);
-            Files.delete(balaDownloadPath.getParent().resolve("_remote.repositories"));
-            if (Files.exists(temporaryExtractionPath)) {
-                Files.walk(temporaryExtractionPath)
-                        .sorted(Comparator.reverseOrder())
-                        .map(Path::toFile)
-                        .forEach(File::delete);
+            try (BufferedReader bufferedReader = Files.newBufferedReader(packageJsonPath, StandardCharsets.UTF_8)) {
+                JsonObject resultObj = new Gson().fromJson(bufferedReader, JsonObject.class);
+                String platform = resultObj.get(PLATFORM).getAsString();
+                Path actualBalaPath = Paths.get(this.repoLocation).resolve(org).resolve(name)
+                        .resolve(version).resolve(platform);
+                FileUtils.copyDirectory(temporaryExtractionPath.toFile(),
+                        actualBalaPath.toFile());
             }
-        } catch (IOException e) {
+        } catch (IOException | MavenResolverClientException  e) {
             return false;
         }
         return true;