From c77a3eaad3d411a3b7e9387b1aaef36c460fa7fd Mon Sep 17 00:00:00 2001 From: Fathima Dilhasha Date: Tue, 11 Jun 2024 21:36:14 +0530 Subject: [PATCH 01/17] Include configurable variables in imported packages to the schema --- .../io/ballerina/projects/ConfigReader.java | 231 ++++++++++++++---- .../configschema/ConfigSchemaBuilder.java | 8 +- 2 files changed, 190 insertions(+), 49 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/ConfigReader.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/ConfigReader.java index 68da8a05610c..79d94c92c050 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/ConfigReader.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/ConfigReader.java @@ -26,7 +26,6 @@ import io.ballerina.compiler.syntax.tree.ModulePartNode; import io.ballerina.compiler.syntax.tree.ModuleVariableDeclarationNode; import io.ballerina.compiler.syntax.tree.Node; -import io.ballerina.compiler.syntax.tree.NodeList; import io.ballerina.compiler.syntax.tree.SimpleNameReferenceNode; import io.ballerina.compiler.syntax.tree.SpecificFieldNode; import io.ballerina.compiler.syntax.tree.SyntaxKind; @@ -44,6 +43,7 @@ import org.wso2.ballerinalang.util.Flags; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -60,7 +60,7 @@ public class ConfigReader { /** - * Retrieve configurable variables in a package as a per module map. + * Retrieve all the configurable variables for a package. * * @param packageInstance to read configurable variables * @return A map with ConfigModuleDetails(module details) as key and @@ -68,19 +68,112 @@ public class ConfigReader { */ public static Map> getConfigVariables(Package packageInstance) { Map> configDetails = new HashMap<>(); + // Get configurable variables of the current package Set validConfigs = getValidConfigs(packageInstance.getDefaultModule().moduleContext(). bLangPackage().symbol); for (Module module : packageInstance.modules()) { getConfigs(module, module.moduleContext().bLangPackage(), configDetails, validConfigs); } + // Get configurable variables of the direct imports + Collection dependencies = new ArrayList<>(); + getValidDependencies(packageInstance, packageInstance.getDefaultModule(), dependencies); + getImportedConfigVars(packageInstance, dependencies, configDetails); return configDetails; } + /** + * Retrieve configurable variables for all the direct imports for a package. + * + * @param currentPackage Current package instance + * @param moduleDependencies Used dependencies of the package + * @param configDetails Map to store the configurable variables against module + */ + private static void getImportedConfigVars(Package currentPackage, + Collection moduleDependencies, + Map> configDetails) { + currentPackage.modules().forEach(module -> + module.moduleContext().bLangPackage().symbol.imports.stream() + .filter(importSymbol -> isDirectDependency( + moduleDependencies, + importSymbol.descriptor.org().value(), + importSymbol.descriptor.packageName().value(), + importSymbol.descriptor.name().moduleNamePart() + )) + .forEach(importSymbol -> { + String orgName = importSymbol.descriptor.org().value(); + String packageName = importSymbol.descriptor.packageName().value(); + String moduleName = importSymbol.descriptor.name().moduleNamePart(); + + // If default module + if (moduleName == null) { + moduleName = packageName; + } + + List configVariables = new ArrayList<>(); + getConfigVars(module, importSymbol.scope.entries.values(), null, configVariables); + + if (!configVariables.isEmpty()) { + configDetails.put( + new ConfigModuleDetails(orgName, packageName, moduleName, + ProjectKind.BALA_PROJECT), configVariables); + } + }) + ); + } + + /** + * Checks whether it is a direct dependency. + * + * @param moduleDependencies collection of module dependencies + * @param orgName org name + * @param packageName package name + * @param moduleName module name + * @return boolean value indicating whether it is a direct dependency + */ + private static boolean isDirectDependency(Collection moduleDependencies, String orgName, + String packageName, String moduleName) { + return moduleDependencies.stream().anyMatch(dependency -> + dependency.descriptor().org().value().equals(orgName) && + dependency.descriptor().packageName().value().equals(packageName) && + (moduleName == null + ? dependency.descriptor().name().moduleNamePart() == null + : dependency.descriptor().name().moduleNamePart().equals(moduleName)) + ); + } + + private static String getDescriptionValue(MetadataNode metadataNode) { + for (AnnotationNode annotation : metadataNode.annotations()) { + Node annotReference = annotation.annotReference(); + if (annotReference.kind() == SyntaxKind.SIMPLE_NAME_REFERENCE && + ((SimpleNameReferenceNode) annotReference).name().text().equals("display") && + annotation.annotValue().isPresent()) { + + for (MappingFieldNode fieldNode : annotation.annotValue().get().fields()) { + if (fieldNode.kind() == SyntaxKind.SPECIFIC_FIELD) { + SpecificFieldNode specificField = (SpecificFieldNode) fieldNode; + if (specificField.fieldName().kind() == SyntaxKind.IDENTIFIER_TOKEN && + ((IdentifierToken) specificField.fieldName()).text().equals("description") && + specificField.valueExpr().isPresent()) { + + ExpressionNode valueNode = specificField.valueExpr().get(); + if (valueNode instanceof BasicLiteralNode) { + return ((BasicLiteralNode) valueNode).literalToken().text(); + } + } + } + } + } + } + return ""; + } + + + /** * Update provided map with the configurable variable details for the given module. * - * @param module Module to retrieve module details - * @param bLangPackage to retrieve configurable variable details + * @param module Module to retrieve module details + * @param bLangPackage to retrieve configurable variable details * @param configDetails Map to store the configurable variables against module */ private static void getConfigs(Module module, @@ -88,32 +181,41 @@ private static void getConfigs(Module module, List> configDetails, Set validConfigs) { List configVariables = new ArrayList<>(); PackageID currentPkgId = bLangPackage.symbol.pkgID; - for (Scope.ScopeEntry entry : bLangPackage.symbol.scope.entries.values()) { + getConfigVars(module, bLangPackage.symbol.scope.entries.values(), validConfigs, configVariables); + if (!configVariables.isEmpty()) { + // Add configurable variable details for the current package + configDetails.put(getConfigModuleDetails(module.moduleName(), currentPkgId, + module.project().kind()), configVariables); + } + } + + private static void getConfigVars(Module module, Collection scopeEntries, + Set validConfigs, List configVariables) { + for (Scope.ScopeEntry entry : scopeEntries) { BSymbol symbol = entry.symbol; // Filter configurable variables if (symbol != null && symbol.tag == SymTag.VARIABLE && Symbols.isFlagOn(symbol.flags, Flags.CONFIGURABLE)) { if (symbol instanceof BVarSymbol) { BVarSymbol varSymbol = (BVarSymbol) symbol; - if (validConfigs.contains(varSymbol)) { + if ((validConfigs != null && validConfigs.contains(varSymbol)) || validConfigs == null) { // Get description - String description = getDescription(varSymbol, module); - if (description.startsWith("\"") && description.endsWith("\"")) { - description = description.substring(1, description.length() - 1); - } - configVariables.add(new ConfigVariable(varSymbol.name.value.replace("\\", ""), varSymbol.type, + String description = getDescriptionValue(varSymbol, module); + configVariables.add(new ConfigVariable( + varSymbol.name.value.replace("\\", ""), varSymbol.type, Symbols.isFlagOn(varSymbol.flags, Flags.REQUIRED), description)); } } } } - if (!configVariables.isEmpty()) { - // Add configurable variable details for the current package - configDetails.put(getConfigModuleDetails(module.moduleName(), currentPkgId, - module.project().kind()), configVariables); - } } + /** + * Get the used configurable variables of the package. + * + * @param packageSymbol ballerina package symbol + * @return set of valid configurable variable symbols + */ private static Set getValidConfigs(BPackageSymbol packageSymbol) { Set configVars = new HashSet<>(); populateConfigVars(packageSymbol, configVars); @@ -125,6 +227,12 @@ private static Set getValidConfigs(BPackageSymbol packageSymbol) { return configVars; } + /** + * Populate the configurable variables for the package. + * + * @param pkgSymbol ballerina package symbol + * @param configVars set of configurable variable symbols + */ private static void populateConfigVars(BPackageSymbol pkgSymbol, Set configVars) { for (Scope.ScopeEntry entry : pkgSymbol.scope.entries.values()) { BSymbol symbol = entry.symbol; @@ -143,8 +251,8 @@ private static void populateConfigVars(BPackageSymbol pkgSymbol, Set /** * Get module details to store the configurable variables against. * - * @param moduleName to retrieve module details - * @param packageID to retrieve package details + * @param moduleName to retrieve module details + * @param packageID to retrieve package details * @param projectKind to retrieve information about the type of project * @return module details stored in object ConfigModuleDetails */ @@ -163,40 +271,19 @@ private static ConfigModuleDetails getConfigModuleDetails(ModuleName moduleName, * @param module to retrieve module details * @return configurable variable description */ - private static String getDescription(BVarSymbol symbol, Module module) { + private static String getDescriptionValue(BVarSymbol symbol, Module module) { Map syntaxTreeMap = getSyntaxTreeMap(module); Node variableNode = getVariableNode(symbol.getPosition().lineRange().startLine().line(), syntaxTreeMap); if (variableNode != null) { Optional optionalMetadataNode = ((ModuleVariableDeclarationNode) variableNode).metadata(); if (optionalMetadataNode.isPresent()) { - NodeList annotations = optionalMetadataNode.get().annotations(); - for (AnnotationNode annotation : annotations) { - Node annotReference = annotation.annotReference(); - if (annotReference.kind() == SyntaxKind.SIMPLE_NAME_REFERENCE) { - SimpleNameReferenceNode simpleNameRef = (SimpleNameReferenceNode) annotReference; - if (simpleNameRef.name().text().equals("display") && annotation.annotValue().isPresent()) { - for (MappingFieldNode fieldNode : annotation.annotValue().get().fields()) { - if (fieldNode.kind() == SyntaxKind.SPECIFIC_FIELD) { - SpecificFieldNode specificField = (SpecificFieldNode) fieldNode; - if (specificField.fieldName().kind() == SyntaxKind.IDENTIFIER_TOKEN) { - if (((IdentifierToken) specificField.fieldName()).text(). - equals("description")) { - if (((SpecificFieldNode) fieldNode).valueExpr().isPresent()) { - ExpressionNode valueNode = - ((SpecificFieldNode) fieldNode).valueExpr().get(); - if (valueNode instanceof BasicLiteralNode) { - return ((BasicLiteralNode) valueNode).literalToken().text(); - } - } - } - } - } - - } - } - } + String description = getDescriptionValue(optionalMetadataNode.get()); + // Remove enclosing quotes if present + if (description.startsWith("\"") && description.endsWith("\"")) { + description = description.substring(1, description.length() - 1); } + return description; } } return ""; @@ -205,7 +292,7 @@ private static String getDescription(BVarSymbol symbol, Module module) { /** * Get Syntax tree node for the configurable variable in given position. * - * @param position position of configurable BVarSymbol + * @param position position of configurable BVarSymbol * @param syntaxTreeMap Syntax tree map for the specific module * @return Relevant syntax tree node for the variable */ @@ -243,4 +330,56 @@ private static Map getSyntaxTreeMap(Module module) { return syntaxTreeMap; } + /** + * Get all the valid dependencies for the package. + * + * @param packageInstance Package instance + * @param module module instance + * @param dependencies Collection of module dependencies + */ + private static void getValidDependencies(Package packageInstance, Module module, + Collection dependencies) { + Collection directDependencies = module.moduleContext().dependencies(); + for (ModuleDependency moduleDependency : directDependencies) { + if (!isDefaultScope(moduleDependency)) { + continue; + } + dependencies.add(moduleDependency); + if (isSamePackage(packageInstance, moduleDependency)) { + for (Module mod : packageInstance.modules()) { + String modName = mod.descriptor().name().moduleNamePart(); + if (modName != null && modName.equals( + moduleDependency.descriptor().name().moduleNamePart())) { + getValidDependencies(packageInstance, mod, dependencies); + } + } + } + } + } + + /** + * Check if the dependency has the default scope. + * + * @param moduleDependency Module dependency + * @return boolean value indicating whether the dependency has default scope or not + */ + private static boolean isDefaultScope(ModuleDependency moduleDependency) { + return moduleDependency.packageDependency().scope().getValue().equals( + PlatformLibraryScope.DEFAULT.getStringValue()); + } + + /** + * Check if the dependency is From the same package. + * + * @param packageInstance package instance + * @param moduleDependency Module dependency + * @return boolean value indicating whether the dependency is from the same package or not + */ + private static boolean isSamePackage(Package packageInstance, ModuleDependency moduleDependency) { + String orgValue = moduleDependency.descriptor().org().value(); + String packageVal = moduleDependency.descriptor().packageName().value(); + return orgValue.equals(packageInstance.packageOrg().value()) && + packageVal.equals(packageInstance.packageName().value()); + } + } diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/internal/configschema/ConfigSchemaBuilder.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/internal/configschema/ConfigSchemaBuilder.java index f3ddba9a6b10..dcd4ae2ae3db 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/internal/configschema/ConfigSchemaBuilder.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/internal/configschema/ConfigSchemaBuilder.java @@ -147,9 +147,11 @@ private static JsonArray getRequiredConfigs(List configVariables private JsonObject setConfigVariables(List configVariables, JsonObject node) { for (ConfigVariable configVariable : configVariables) { - JsonObject typeNode = new TypeConverter().getType(configVariable.type()); - typeNode.addProperty("description", configVariable.description()); - node.add(configVariable.name(), typeNode); + if (configVariable.type() != null) { + JsonObject typeNode = new TypeConverter().getType(configVariable.type()); + typeNode.addProperty("description", configVariable.description()); + node.add(configVariable.name(), typeNode); + } } return node; } From ed51a7fc04292c96ee2a6c237940240a894f4087 Mon Sep 17 00:00:00 2001 From: Thevakumar-Luheerathan Date: Thu, 23 May 2024 11:25:41 +0530 Subject: [PATCH 02/17] Reuse Package resolution with zero import change --- .../java/io/ballerina/projects/Package.java | 8 +- .../io/ballerina/projects/PackageContext.java | 11 ++- .../ballerina/projects/PackageResolution.java | 83 +++++++++++++++---- .../workspace/BallerinaWorkspaceManager.java | 6 +- 4 files changed, 90 insertions(+), 18 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/Package.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/Package.java index 36a25764db7c..64f015c2d318 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/Package.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/Package.java @@ -5,6 +5,7 @@ import io.ballerina.projects.internal.DependencyManifestBuilder; import io.ballerina.projects.internal.ManifestBuilder; import io.ballerina.projects.internal.model.CompilerPluginDescriptor; +import io.ballerina.projects.util.ProjectUtils; import io.ballerina.tools.diagnostics.Diagnostic; import org.ballerinalang.model.elements.PackageID; import org.wso2.ballerinalang.compiler.PackageCache; @@ -624,13 +625,18 @@ private Map copyModules(Package oldPackage) { } private Package createNewPackage() { + Set oldPackageImports = ProjectUtils.getPackageImports(this.project.currentPackage()); + PackageResolution oldResolution = this.project.currentPackage().getResolution();; PackageContext newPackageContext = new PackageContext(this.project, this.packageId, this.packageManifest, this.dependencyManifest, this.ballerinaTomlContext, this.dependenciesTomlContext, this.cloudTomlContext, this.compilerPluginTomlContext, this.balToolTomlContext, this.packageMdContext, this.compilationOptions, this.moduleContextMap, DependencyGraph.emptyGraph()); this.project.setCurrentPackage(new Package(newPackageContext, this.project)); - + Set currentPackageImports = ProjectUtils.getPackageImports(this.project.currentPackage()); + if (oldPackageImports.equals(currentPackageImports)) { + this.project.currentPackage().packageContext().getResolution(oldResolution); + } CompilationOptions offlineCompOptions = CompilationOptions.builder().setOffline(true).build(); offlineCompOptions = offlineCompOptions.acceptTheirs(project.currentPackage().compilationOptions()); DependencyGraph newDepGraph = this.project.currentPackage().packageContext() diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageContext.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageContext.java index 57efbc9093bc..aa2d317c623c 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageContext.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageContext.java @@ -58,8 +58,8 @@ class PackageContext { */ private final DependencyGraph pkgDescDependencyGraph; - private Set packageDependencies; - private DependencyGraph moduleDependencyGraph; + private Set packageDependencies; // This is added during getResolution() method + private DependencyGraph moduleDependencyGraph; // This is added during resolveResolution() method private PackageResolution packageResolution; private PackageCompilation packageCompilation; @@ -265,6 +265,13 @@ PackageResolution getResolution(CompilationOptions compilationOptions, boolean i return packageResolution; } + PackageResolution getResolution(PackageResolution oldResolution) { + if (packageResolution == null) { + packageResolution = PackageResolution.from(oldResolution, this, this.compilationOptions); + } + return packageResolution; + } + Collection packageDependencies() { return packageDependencies; } diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java index 880784353517..66d0ef4f11a7 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java @@ -76,6 +76,7 @@ public class PackageResolution { private final PackageContext rootPackageContext; private final BlendedManifest blendedManifest; private final DependencyGraph dependencyGraph; + private final DependencyGraph dependencyNodeGraph; private final CompilationOptions compilationOptions; private final ResolutionOptions resolutionOptions; private final PackageResolver packageResolver; @@ -100,16 +101,68 @@ private PackageResolution(PackageContext rootPackageContext, CompilationOptions diagnosticList.addAll(this.blendedManifest.diagnosticResult().allDiagnostics); this.moduleResolver = createModuleResolver(rootPackageContext, projectEnvContext); - this.dependencyGraph = buildDependencyGraph(); + this.dependencyNodeGraph = createDependencyNodeGraph(); + this.dependencyGraph = buildDependencyGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), + packageResolver); DependencyResolution dependencyResolution = new DependencyResolution( projectEnvContext.getService(PackageCache.class), moduleResolver, dependencyGraph); resolveDependencies(dependencyResolution); } + private PackageResolution(PackageResolution packageResolution, PackageContext rootPackageContext, + CompilationOptions compilationOptions) { + this.rootPackageContext = rootPackageContext; + this.diagnosticList = new ArrayList<>(); + this.compilationOptions = compilationOptions; + this.resolutionOptions = getResolutionOptions(rootPackageContext, compilationOptions); + ProjectEnvironment projectEnvContext = rootPackageContext.project().projectEnvironmentContext(); + this.packageResolver = projectEnvContext.getService(PackageResolver.class); + this.blendedManifest = createBlendedManifest(rootPackageContext, projectEnvContext, + this.resolutionOptions.offline()); + diagnosticList.addAll(this.blendedManifest.diagnosticResult().allDiagnostics); + + this.moduleResolver = createModuleResolver(rootPackageContext, projectEnvContext); + getModuleLoadRequestsOfDirectDependencies(); + this.dependencyNodeGraph = packageResolution.dependencyNodeGraph; + this.dependencyGraph = cloneDependencyGraphReplacingRoot(packageResolution.dependencyGraph, + rootPackageContext.project().currentPackage()); + DependencyResolution dependencyResolution = new DependencyResolution( + projectEnvContext.getService(PackageCache.class), moduleResolver, dependencyGraph); + resolveDependencies(dependencyResolution); + } + + private DependencyGraph cloneDependencyGraphReplacingRoot + (DependencyGraph depGraph, Package rootPackage) { + ResolvedPackageDependency oldRoot = depGraph.getRoot(); + ResolvedPackageDependency newRoot = new ResolvedPackageDependency(rootPackage, + oldRoot.scope(), oldRoot.dependencyResolvedType()); + DependencyGraphBuilder depGraphBuilder = + DependencyGraphBuilder.getBuilder(newRoot); + for (ResolvedPackageDependency depNode : depGraph.getNodes()) { + if (depNode == oldRoot) { + depGraphBuilder.add(newRoot); + } else { + depGraphBuilder.add(depNode); + } + List directPkgDependencies = + depGraph.getDirectDependencies(depNode) + .stream() + .map(directDepNode -> directDepNode == oldRoot ? newRoot : directDepNode) + .collect(Collectors.toList()); + depGraphBuilder.addDependencies(depNode, directPkgDependencies); + } + return depGraphBuilder.build(); + } + static PackageResolution from(PackageContext rootPackageContext, CompilationOptions compilationOptions) { return new PackageResolution(rootPackageContext, compilationOptions); } + static PackageResolution from(PackageResolution packageResolution, PackageContext + packageContext, CompilationOptions compilationOptions) { + return new PackageResolution(packageResolution, packageContext, compilationOptions); + } + /** * Returns the package dependency graph of this package. * @@ -224,12 +277,13 @@ private boolean getSticky(PackageContext rootPackageContext) { * * @return package dependency graph of this package */ - private DependencyGraph buildDependencyGraph() { + private DependencyGraph createDependencyNodeGraph() { // TODO We should get diagnostics as well. Need to design that contract if (rootPackageContext.project().kind() == ProjectKind.BALA_PROJECT) { - return resolveBALADependencies(); + return createDependencyNodeGraphForBala( + rootPackageContext.dependencyGraph()); } else { - return resolveSourceDependencies(); + return createDependencyNodeGraphForSource(); } } @@ -271,15 +325,15 @@ private LinkedHashSet getModuleLoadRequestsOfDirectDependenci private DependencyGraph resolveBALADependencies() { // 1) Convert package descriptor graph to DependencyNode graph - DependencyGraph dependencyNodeGraph = createDependencyNodeGraph( + DependencyGraph dependencyNodeGraph = createDependencyNodeGraphForBala( rootPackageContext.dependencyGraph()); //2 ) Create the package dependency graph by downloading packages if necessary. - return buildPackageGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), + return buildDependencyGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), packageResolver); } - private DependencyGraph resolveSourceDependencies() { + private DependencyGraph createDependencyNodeGraphForSource() { // 1) Get PackageLoadRequests for all the direct dependencies of this package LinkedHashSet moduleLoadRequests = getModuleLoadRequestsOfDirectDependencies(); @@ -291,10 +345,11 @@ private DependencyGraph resolveSourceDependencies() { this.dependencyGraphDump = resolutionEngine.dumpGraphs(); diagnosticList.addAll(resolutionEngine.diagnosticResult().allDiagnostics); + return dependencyNodeGraph; - //3 ) Create the package dependency graph by downloading packages if necessary. - return buildPackageGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), - packageResolver); +// //3 ) Create the package dependency graph by downloading packages if necessary. +// return buildPackageGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), +// packageResolver); } static Optional findModuleInPackage(PackageContext resolvedPackage, String moduleNameStr) { @@ -318,9 +373,9 @@ static Optional findModuleInPackage(PackageContext resolvedPackag return Optional.of(resolvedModule); } - private DependencyGraph buildPackageGraph(DependencyGraph depGraph, - Package rootPackage, - PackageResolver packageResolver) { + private DependencyGraph buildDependencyGraph(DependencyGraph depGraph, + Package rootPackage, + PackageResolver packageResolver) { PackageContainer resolvedPkgContainer = new PackageContainer<>(); // Add root node to the container @@ -393,7 +448,7 @@ private ResolutionRequest createFromDepNode(DependencyNode depNode) { resolutionOptions.packageLockingMode()); } - private DependencyGraph createDependencyNodeGraph( + private DependencyGraph createDependencyNodeGraphForBala( DependencyGraph pkgDescDepGraph) { DependencyNode rootNode = new DependencyNode(rootPackageContext.descriptor()); diff --git a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java index 61935d570e4e..54ed1a1a169c 100644 --- a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java +++ b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java @@ -1268,7 +1268,11 @@ private void updateBalDocument(Path filePath, String content, ProjectContext pro // Update project instance projectContext.setProject(updatedDoc.module().project()); } - } finally { + } catch (Exception e) { + String message = e.getMessage(); + throw new WorkspaceDocumentException("Error occurred while updating document: " + filePath.toString(), e); + } + finally { // Unlock Project Instance lock.unlock(); } From 31d112f3155e11eb8f10bb9cf9bd6399f5bb3b7f Mon Sep 17 00:00:00 2001 From: Thevakumar-Luheerathan Date: Mon, 27 May 2024 17:50:47 +0530 Subject: [PATCH 03/17] Fix test failures --- .../io/ballerina/projects/PackageContext.java | 4 +- .../ballerina/projects/PackageResolution.java | 38 +++++++++---------- .../workspace/BallerinaWorkspaceManager.java | 6 +-- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageContext.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageContext.java index aa2d317c623c..4962f258d3ff 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageContext.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageContext.java @@ -58,8 +58,8 @@ class PackageContext { */ private final DependencyGraph pkgDescDependencyGraph; - private Set packageDependencies; // This is added during getResolution() method - private DependencyGraph moduleDependencyGraph; // This is added during resolveResolution() method + private Set packageDependencies; + private DependencyGraph moduleDependencyGraph; private PackageResolution packageResolution; private PackageCompilation packageCompilation; diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java index 66d0ef4f11a7..61af036e0c09 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java @@ -76,7 +76,6 @@ public class PackageResolution { private final PackageContext rootPackageContext; private final BlendedManifest blendedManifest; private final DependencyGraph dependencyGraph; - private final DependencyGraph dependencyNodeGraph; private final CompilationOptions compilationOptions; private final ResolutionOptions resolutionOptions; private final PackageResolver packageResolver; @@ -101,9 +100,7 @@ private PackageResolution(PackageContext rootPackageContext, CompilationOptions diagnosticList.addAll(this.blendedManifest.diagnosticResult().allDiagnostics); this.moduleResolver = createModuleResolver(rootPackageContext, projectEnvContext); - this.dependencyNodeGraph = createDependencyNodeGraph(); - this.dependencyGraph = buildDependencyGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), - packageResolver); + this.dependencyGraph = buildDependencyGraph(); DependencyResolution dependencyResolution = new DependencyResolution( projectEnvContext.getService(PackageCache.class), moduleResolver, dependencyGraph); resolveDependencies(dependencyResolution); @@ -122,10 +119,11 @@ private PackageResolution(PackageResolution packageResolution, PackageContext ro diagnosticList.addAll(this.blendedManifest.diagnosticResult().allDiagnostics); this.moduleResolver = createModuleResolver(rootPackageContext, projectEnvContext); - getModuleLoadRequestsOfDirectDependencies(); - this.dependencyNodeGraph = packageResolution.dependencyNodeGraph; + LinkedHashSet moduleLoadRequests = getModuleLoadRequestsOfDirectDependencies(); + moduleResolver.resolveModuleLoadRequests(moduleLoadRequests); this.dependencyGraph = cloneDependencyGraphReplacingRoot(packageResolution.dependencyGraph, rootPackageContext.project().currentPackage()); + this.dependencyGraphDump = packageResolution.dependencyGraphDump; DependencyResolution dependencyResolution = new DependencyResolution( projectEnvContext.getService(PackageCache.class), moduleResolver, dependencyGraph); resolveDependencies(dependencyResolution); @@ -277,13 +275,12 @@ private boolean getSticky(PackageContext rootPackageContext) { * * @return package dependency graph of this package */ - private DependencyGraph createDependencyNodeGraph() { + private DependencyGraph buildDependencyGraph() { // TODO We should get diagnostics as well. Need to design that contract if (rootPackageContext.project().kind() == ProjectKind.BALA_PROJECT) { - return createDependencyNodeGraphForBala( - rootPackageContext.dependencyGraph()); + return resolveBALADependencies(); } else { - return createDependencyNodeGraphForSource(); + return resolveSourceDependencies(); } } @@ -325,15 +322,15 @@ private LinkedHashSet getModuleLoadRequestsOfDirectDependenci private DependencyGraph resolveBALADependencies() { // 1) Convert package descriptor graph to DependencyNode graph - DependencyGraph dependencyNodeGraph = createDependencyNodeGraphForBala( + DependencyGraph dependencyNodeGraph = createDependencyNodeGraph( rootPackageContext.dependencyGraph()); //2 ) Create the package dependency graph by downloading packages if necessary. - return buildDependencyGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), + return buildPackageGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), packageResolver); } - private DependencyGraph createDependencyNodeGraphForSource() { + private DependencyGraph resolveSourceDependencies() { // 1) Get PackageLoadRequests for all the direct dependencies of this package LinkedHashSet moduleLoadRequests = getModuleLoadRequestsOfDirectDependencies(); @@ -345,11 +342,10 @@ private DependencyGraph createDependencyNodeGraphForSource() { this.dependencyGraphDump = resolutionEngine.dumpGraphs(); diagnosticList.addAll(resolutionEngine.diagnosticResult().allDiagnostics); - return dependencyNodeGraph; -// //3 ) Create the package dependency graph by downloading packages if necessary. -// return buildPackageGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), -// packageResolver); + //3 ) Create the package dependency graph by downloading packages if necessary. + return buildPackageGraph(dependencyNodeGraph, rootPackageContext.project().currentPackage(), + packageResolver); } static Optional findModuleInPackage(PackageContext resolvedPackage, String moduleNameStr) { @@ -373,9 +369,9 @@ static Optional findModuleInPackage(PackageContext resolvedPackag return Optional.of(resolvedModule); } - private DependencyGraph buildDependencyGraph(DependencyGraph depGraph, - Package rootPackage, - PackageResolver packageResolver) { + private DependencyGraph buildPackageGraph(DependencyGraph depGraph, + Package rootPackage, + PackageResolver packageResolver) { PackageContainer resolvedPkgContainer = new PackageContainer<>(); // Add root node to the container @@ -448,7 +444,7 @@ private ResolutionRequest createFromDepNode(DependencyNode depNode) { resolutionOptions.packageLockingMode()); } - private DependencyGraph createDependencyNodeGraphForBala( + private DependencyGraph createDependencyNodeGraph( DependencyGraph pkgDescDepGraph) { DependencyNode rootNode = new DependencyNode(rootPackageContext.descriptor()); diff --git a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java index 54ed1a1a169c..61935d570e4e 100644 --- a/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java +++ b/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/workspace/BallerinaWorkspaceManager.java @@ -1268,11 +1268,7 @@ private void updateBalDocument(Path filePath, String content, ProjectContext pro // Update project instance projectContext.setProject(updatedDoc.module().project()); } - } catch (Exception e) { - String message = e.getMessage(); - throw new WorkspaceDocumentException("Error occurred while updating document: " + filePath.toString(), e); - } - finally { + } finally { // Unlock Project Instance lock.unlock(); } From 8c080d043ec5273c50ef0d27f94010cf502c8da9 Mon Sep 17 00:00:00 2001 From: Thevakumar-Luheerathan Date: Wed, 29 May 2024 14:02:06 +0530 Subject: [PATCH 04/17] Discard resolution with Ballerina.toml,Dependencies.toml changes --- .../java/io/ballerina/projects/Package.java | 23 +++++++++++++++---- .../ballerina/projects/PackageResolution.java | 6 ++--- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/Package.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/Package.java index 64f015c2d318..4c64364f66f2 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/Package.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/Package.java @@ -625,16 +625,15 @@ private Map copyModules(Package oldPackage) { } private Package createNewPackage() { - Set oldPackageImports = ProjectUtils.getPackageImports(this.project.currentPackage()); - PackageResolution oldResolution = this.project.currentPackage().getResolution();; + Package oldPackage = this.project.currentPackage(); + PackageResolution oldResolution = oldPackage.getResolution();; PackageContext newPackageContext = new PackageContext(this.project, this.packageId, this.packageManifest, this.dependencyManifest, this.ballerinaTomlContext, this.dependenciesTomlContext, this.cloudTomlContext, this.compilerPluginTomlContext, this.balToolTomlContext, this.packageMdContext, this.compilationOptions, this.moduleContextMap, DependencyGraph.emptyGraph()); this.project.setCurrentPackage(new Package(newPackageContext, this.project)); - Set currentPackageImports = ProjectUtils.getPackageImports(this.project.currentPackage()); - if (oldPackageImports.equals(currentPackageImports)) { + if (isOldDependencyGraphValid(oldPackage, this.project.currentPackage())) { this.project.currentPackage().packageContext().getResolution(oldResolution); } CompilationOptions offlineCompOptions = CompilationOptions.builder().setOffline(true).build(); @@ -645,6 +644,22 @@ private Package createNewPackage() { return this.project.currentPackage(); } + private static boolean isOldDependencyGraphValid(Package oldPackage, Package currentPackage) { + Set oldPackageImports = ProjectUtils.getPackageImports(oldPackage); + Set currentPackageImports = ProjectUtils.getPackageImports(currentPackage); + String oldDependencyTomlContent = oldPackage.packageContext.dependenciesTomlContext() + .map(d -> d.tomlDocument().textDocument().toString()).orElse(""); + String currentDependencyTomlContent = currentPackage.packageContext.dependenciesTomlContext() + .map(d -> d.tomlDocument().textDocument().toString()).orElse(""); + String oldBallerinaTomlContent = oldPackage.packageContext.ballerinaTomlContext() + .map(d -> d.tomlDocument().textDocument().toString()).orElse(""); + String currentBallerinaTomlContent = currentPackage.packageContext.ballerinaTomlContext() + .map(d -> d.tomlDocument().textDocument().toString()).orElse(""); + return oldPackageImports.equals(currentPackageImports) && + oldDependencyTomlContent.equals(currentDependencyTomlContent) && + oldBallerinaTomlContent.equals(currentBallerinaTomlContent); + } + private void cleanPackageCache(DependencyGraph oldGraph, DependencyGraph newGraph) { io.ballerina.projects.environment.PackageCache environmentPackageCache = diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java index 61af036e0c09..f977b02d219e 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/PackageResolution.java @@ -98,7 +98,6 @@ private PackageResolution(PackageContext rootPackageContext, CompilationOptions this.blendedManifest = createBlendedManifest(rootPackageContext, projectEnvContext, this.resolutionOptions.offline()); diagnosticList.addAll(this.blendedManifest.diagnosticResult().allDiagnostics); - this.moduleResolver = createModuleResolver(rootPackageContext, projectEnvContext); this.dependencyGraph = buildDependencyGraph(); DependencyResolution dependencyResolution = new DependencyResolution( @@ -117,11 +116,10 @@ private PackageResolution(PackageResolution packageResolution, PackageContext ro this.blendedManifest = createBlendedManifest(rootPackageContext, projectEnvContext, this.resolutionOptions.offline()); diagnosticList.addAll(this.blendedManifest.diagnosticResult().allDiagnostics); - this.moduleResolver = createModuleResolver(rootPackageContext, projectEnvContext); LinkedHashSet moduleLoadRequests = getModuleLoadRequestsOfDirectDependencies(); moduleResolver.resolveModuleLoadRequests(moduleLoadRequests); - this.dependencyGraph = cloneDependencyGraphReplacingRoot(packageResolution.dependencyGraph, + this.dependencyGraph = cloneDependencyGraphNewRoot(packageResolution.dependencyGraph, rootPackageContext.project().currentPackage()); this.dependencyGraphDump = packageResolution.dependencyGraphDump; DependencyResolution dependencyResolution = new DependencyResolution( @@ -129,7 +127,7 @@ private PackageResolution(PackageResolution packageResolution, PackageContext ro resolveDependencies(dependencyResolution); } - private DependencyGraph cloneDependencyGraphReplacingRoot + private DependencyGraph cloneDependencyGraphNewRoot (DependencyGraph depGraph, Package rootPackage) { ResolvedPackageDependency oldRoot = depGraph.getRoot(); ResolvedPackageDependency newRoot = new ResolvedPackageDependency(rootPackage, From 1e6461973bcd7f69f77cb10c79e7c71780c07749 Mon Sep 17 00:00:00 2001 From: Thevakumar-Luheerathan Date: Mon, 3 Jun 2024 15:26:53 +0530 Subject: [PATCH 05/17] Restrict imports with syntax error for package resolution --- .../io/ballerina/projects/util/ProjectUtils.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/util/ProjectUtils.java b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/util/ProjectUtils.java index 6148904ced9f..d4439fee7bf7 100644 --- a/compiler/ballerina-lang/src/main/java/io/ballerina/projects/util/ProjectUtils.java +++ b/compiler/ballerina-lang/src/main/java/io/ballerina/projects/util/ProjectUtils.java @@ -46,6 +46,8 @@ import io.ballerina.projects.Settings; import io.ballerina.projects.internal.model.BuildJson; import io.ballerina.projects.internal.model.Dependency; +import io.ballerina.tools.diagnostics.Diagnostic; +import io.ballerina.tools.diagnostics.DiagnosticSeverity; import org.apache.commons.compress.archivers.jar.JarArchiveEntry; import org.apache.commons.compress.archivers.zip.ZipArchiveEntryPredicate; import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream; @@ -194,10 +196,18 @@ public static Set getPackageImports(Package pkg) { private static void getPackageImports(Set imports, Module module, Collection documentIds) { for (DocumentId docId : documentIds) { Document document = module.document(docId); - ModulePartNode modulePartNode = document.syntaxTree().rootNode(); - for (ImportDeclarationNode importDcl : modulePartNode.imports()) { + boolean isErrorInImport = false; + for (Diagnostic diagnostic : importDcl.diagnostics()) { + if (diagnostic.diagnosticInfo().severity() == DiagnosticSeverity.ERROR) { + isErrorInImport = true; + break; + } + } + if (isErrorInImport) { + continue; + } String orgName = ""; if (importDcl.orgName().isPresent()) { orgName = importDcl.orgName().get().orgName().text(); From 2cafdb89b27e1ce5adbf31cbbe13f8b726d118f3 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Tue, 18 Jun 2024 12:28:57 +0530 Subject: [PATCH 06/17] Add new transaction configuration values This commit introduces two new configurations values for transactions, `transactionAutoCommitTimeout` and `transactionCleanupTimeout` --- .../transactions/TransactionConstants.java | 2 + .../TransactionResourceManager.java | 58 +++++++++++++++++++ .../src/main/ballerina/transaction.bal | 4 ++ 3 files changed, 64 insertions(+) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionConstants.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionConstants.java index 345b8f82cac1..f7ee5d9e9d8a 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionConstants.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionConstants.java @@ -70,4 +70,6 @@ public class TransactionConstants { public static final String ANN_NAME_TRX_PARTICIPANT_CONFIG = "Participant"; public static final String TIMESTAMP_OBJECT_VALUE_FIELD = "timeValue"; + public static final int DEFAULT_TRX_AUTO_COMMIT_TIMEOUT = 120; + public static final int DEFAULT_TRX_CLEANUP_TIMEOUT = 600; } diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java index 739c34fdf6aa..c378bb7c7a8b 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java @@ -59,6 +59,8 @@ import javax.transaction.xa.Xid; import static io.ballerina.runtime.api.constants.RuntimeConstants.BALLERINA_BUILTIN_PKG_PREFIX; +import static io.ballerina.runtime.transactions.TransactionConstants.DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; +import static io.ballerina.runtime.transactions.TransactionConstants.DEFAULT_TRX_CLEANUP_TIMEOUT; import static io.ballerina.runtime.transactions.TransactionConstants.TRANSACTION_PACKAGE_ID; import static io.ballerina.runtime.transactions.TransactionConstants.TRANSACTION_PACKAGE_NAME; import static io.ballerina.runtime.transactions.TransactionConstants.TRANSACTION_PACKAGE_VERSION; @@ -186,6 +188,62 @@ private String getTransactionLogDirectory() { } } + /** + * This method gets the user specified config for the transaction auto commit timeout. Default is 120. + * + * @return int transaction auto commit timeout value + */ + public int getTransactionAutoCommitTimeout() { + VariableKey transactionAutoCommitTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, + "transactionAutoCommitTimeout", PredefinedTypes.TYPE_INT, false); + if (!ConfigMap.containsKey(transactionAutoCommitTimeoutKey)) { + return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; + } else { + int transactionAutoCommitTimeout; + Object value = ConfigMap.get(transactionAutoCommitTimeoutKey); + if (value instanceof Long) { + transactionAutoCommitTimeout = ((Long) value).intValue(); + } else if (value instanceof Integer) { + transactionAutoCommitTimeout = (Integer) value; + } else { + return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; + } + if (transactionAutoCommitTimeout < DEFAULT_TRX_AUTO_COMMIT_TIMEOUT) { + return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; + } else { + return transactionAutoCommitTimeout; + } + } + } + + /** + * This method gets the user specified config for cleaning up dead transactions. Default is 600. + * + * @return int transaction cleanup after value + */ + public int getTransactionCleanupTimeout() { + VariableKey transactionCleanupTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, "transactionCleanupTimeout", + PredefinedTypes.TYPE_INT, false); + if (!ConfigMap.containsKey(transactionCleanupTimeoutKey)) { + return DEFAULT_TRX_CLEANUP_TIMEOUT; + } else { + int transactionCleanupTimeout; + Object value = ConfigMap.get(transactionCleanupTimeoutKey); + if (value instanceof Long) { + transactionCleanupTimeout = ((Long) value).intValue(); + } else if (value instanceof Integer) { + transactionCleanupTimeout = (Integer) value; + } else { + return DEFAULT_TRX_CLEANUP_TIMEOUT; + } + if (transactionCleanupTimeout < DEFAULT_TRX_CLEANUP_TIMEOUT) { + return DEFAULT_TRX_CLEANUP_TIMEOUT; + } else { + return transactionCleanupTimeout; + } + } + } + /** * This method will register connection resources with a particular transaction. * diff --git a/langlib/lang.transaction/src/main/ballerina/transaction.bal b/langlib/lang.transaction/src/main/ballerina/transaction.bal index 0e8bd367b713..ab62be24e215 100644 --- a/langlib/lang.transaction/src/main/ballerina/transaction.bal +++ b/langlib/lang.transaction/src/main/ballerina/transaction.bal @@ -20,6 +20,10 @@ import ballerina/jballerina.java; configurable boolean managerEnabled = false; # Config to specify transaction log directory. configurable string logBase = "transaction_log_dir"; +# Config to specify the timeout for auto commit. +configurable int transactionAutoCommitTimeout = 120; +# Config to specify the timeout for cleaning up dead transactions. +configurable int transactionCleanupTimeout = 600; //TODO: remove this in Beta2 and use an anonymous record instead # Internally used record to hold information about a transaction. From 24f9085575caed2fcf0faea7edb7ea6f1d6a0608 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Tue, 18 Jun 2024 17:42:22 +0530 Subject: [PATCH 07/17] Rework config default value --- .../runtime/transactions/TransactionResourceManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java index c378bb7c7a8b..ead9f3fc32df 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java @@ -208,7 +208,7 @@ public int getTransactionAutoCommitTimeout() { } else { return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; } - if (transactionAutoCommitTimeout < DEFAULT_TRX_AUTO_COMMIT_TIMEOUT) { + if (transactionAutoCommitTimeout < 0) { return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; } else { return transactionAutoCommitTimeout; @@ -236,7 +236,7 @@ public int getTransactionCleanupTimeout() { } else { return DEFAULT_TRX_CLEANUP_TIMEOUT; } - if (transactionCleanupTimeout < DEFAULT_TRX_CLEANUP_TIMEOUT) { + if (transactionCleanupTimeout < 0) { return DEFAULT_TRX_CLEANUP_TIMEOUT; } else { return transactionCleanupTimeout; From fb41a31bbcb1c0d75f71eda1e8628e6bec859533 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Tue, 18 Jun 2024 18:52:18 +0530 Subject: [PATCH 08/17] Add runtime warnings for invalid config values --- .../runtime/internal/errors/ErrorCodes.java | 4 ++- .../TransactionResourceManager.java | 35 +++++++++++++++---- .../main/resources/MessagesBundle.properties | 5 +++ 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java index a969630c6a83..a0e465542db6 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java @@ -149,7 +149,9 @@ public enum ErrorCodes implements DiagnosticCode { REGEXP_INVALID_HEX_DIGIT("regexp.invalid.hex.digit", "RUNTIME_0120"), CONFIG_TOML_INVALID_MODULE_STRUCTURE_WITH_VARIABLE("config.toml.invalid.module.structure.with.variable", "RUNTIME_0121"), - EMPTY_XML_SEQUENCE_HAS_NO_ATTRIBUTES("empty.xml.sequence.no.attributes", "RUNTIME_0122"); + EMPTY_XML_SEQUENCE_HAS_NO_ATTRIBUTES("empty.xml.sequence.no.attributes", "RUNTIME_0122"), + INVALID_TRANSACTION_AUTO_COMMIT_TIMEOUT("invalid.transaction.auto.commit.value", "RUNTIME_0131"), + INVALID_TRANSACTION_CLEANUP_TIMEOUT("invalid.transaction.cleanup.timeout", "RUNTIME_0132"); private final String errorMsgKey; private final String errorCode; diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java index ead9f3fc32df..e20e7455689b 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java @@ -27,6 +27,8 @@ import io.ballerina.runtime.api.values.BString; import io.ballerina.runtime.internal.configurable.ConfigMap; import io.ballerina.runtime.internal.configurable.VariableKey; +import io.ballerina.runtime.internal.diagnostics.RuntimeDiagnosticLog; +import io.ballerina.runtime.internal.errors.ErrorCodes; import io.ballerina.runtime.internal.scheduling.Scheduler; import io.ballerina.runtime.internal.scheduling.Strand; import io.ballerina.runtime.internal.util.RuntimeUtils; @@ -101,6 +103,7 @@ public class TransactionResourceManager { private boolean transactionManagerEnabled; private static final PrintStream stderr = System.err; + RuntimeDiagnosticLog diagnosticLog = new RuntimeDiagnosticLog(); Map transactionInfoMap; @@ -196,19 +199,29 @@ private String getTransactionLogDirectory() { public int getTransactionAutoCommitTimeout() { VariableKey transactionAutoCommitTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, "transactionAutoCommitTimeout", PredefinedTypes.TYPE_INT, false); - if (!ConfigMap.containsKey(transactionAutoCommitTimeoutKey)) { + Object value = ConfigMap.get(transactionAutoCommitTimeoutKey); + if (value == null) { return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; } else { int transactionAutoCommitTimeout; - Object value = ConfigMap.get(transactionAutoCommitTimeoutKey); if (value instanceof Long) { transactionAutoCommitTimeout = ((Long) value).intValue(); } else if (value instanceof Integer) { transactionAutoCommitTimeout = (Integer) value; } else { + diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_AUTO_COMMIT_TIMEOUT, null, value, + DEFAULT_TRX_AUTO_COMMIT_TIMEOUT); + if (!diagnosticLog.getDiagnosticList().isEmpty()) { + RuntimeUtils.handleDiagnosticErrors(diagnosticLog); + } return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; } - if (transactionAutoCommitTimeout < 0) { + if (transactionAutoCommitTimeout <= 0) { + diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_CLEANUP_TIMEOUT, null, + transactionAutoCommitTimeout, DEFAULT_TRX_AUTO_COMMIT_TIMEOUT); + if (!diagnosticLog.getDiagnosticList().isEmpty()) { + RuntimeUtils.handleDiagnosticErrors(diagnosticLog); + } return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; } else { return transactionAutoCommitTimeout; @@ -224,19 +237,29 @@ public int getTransactionAutoCommitTimeout() { public int getTransactionCleanupTimeout() { VariableKey transactionCleanupTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, "transactionCleanupTimeout", PredefinedTypes.TYPE_INT, false); - if (!ConfigMap.containsKey(transactionCleanupTimeoutKey)) { + Object value = ConfigMap.get(transactionCleanupTimeoutKey); + if (value == null) { return DEFAULT_TRX_CLEANUP_TIMEOUT; } else { int transactionCleanupTimeout; - Object value = ConfigMap.get(transactionCleanupTimeoutKey); if (value instanceof Long) { transactionCleanupTimeout = ((Long) value).intValue(); } else if (value instanceof Integer) { transactionCleanupTimeout = (Integer) value; } else { + diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_CLEANUP_TIMEOUT, null, value, + DEFAULT_TRX_CLEANUP_TIMEOUT); + if (!diagnosticLog.getDiagnosticList().isEmpty()) { + RuntimeUtils.handleDiagnosticErrors(diagnosticLog); + } return DEFAULT_TRX_CLEANUP_TIMEOUT; } - if (transactionCleanupTimeout < 0) { + if (transactionCleanupTimeout <= 0) { + diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_CLEANUP_TIMEOUT, null, + transactionCleanupTimeout, DEFAULT_TRX_CLEANUP_TIMEOUT); + if (!diagnosticLog.getDiagnosticList().isEmpty()) { + RuntimeUtils.handleDiagnosticErrors(diagnosticLog); + } return DEFAULT_TRX_CLEANUP_TIMEOUT; } else { return transactionCleanupTimeout; diff --git a/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties b/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties index 297d6b945c74..ac1281830e2f 100644 --- a/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties +++ b/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties @@ -196,6 +196,11 @@ merge.json.error = cannot merge JSON values of types ''{0}'' and ''{1}'' field.removal.not.allowed = failed to remove field: ''{0}'' is a required field in ''{1}'' operation.not.supported = {0} not supported on type ''{1}'' invalid.fraction.digits = fraction digits cannot be less than '0' +invalid.transaction.auto.commit.timeout = provided transaction auto commit timeout of ''{0}'' is invalid, default \ + value of ''{1}'' will be used instead. +invalid.transaction.cleanup.timeout = provided transaction cleanup timeout of ''{0}'' is invalid, default value of \ + ''{1}'' will be used instead. + #configurations config.type.not.supported = configurable variable ''{0}'' with type ''{1}'' is not supported config.invalid.byte.range = [{0}] value provided for byte variable ''{1}'' is out of range. Expected range is \ From 48001ef9d83e8cb8c5380b2131d26c505b0dda60 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Tue, 18 Jun 2024 18:58:20 +0530 Subject: [PATCH 09/17] Make config keys constants --- .../runtime/transactions/TransactionResourceManager.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java index e20e7455689b..3ffe5727f4e8 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java @@ -88,6 +88,8 @@ public class TransactionResourceManager { private static final String ATOMIKOS_LOG_BASE_PROPERTY = "com.atomikos.icatch.log_base_dir"; private static final String ATOMIKOS_LOG_NAME_PROPERTY = "com.atomikos.icatch.log_base_name"; private static final String ATOMIKOS_REGISTERED_PROPERTY = "com.atomikos.icatch.registered"; + public static final String TRANSACTION_AUTO_COMMIT_TIMEOUT_KEY = "transactionAutoCommitTimeout"; + public static final String TRANSACTION_CLEANUP_TIMEOUT_KEY = "transactionCleanupTimeout"; private static final Logger log = LoggerFactory.getLogger(TransactionResourceManager.class); private Map> resourceRegistry; @@ -198,7 +200,7 @@ private String getTransactionLogDirectory() { */ public int getTransactionAutoCommitTimeout() { VariableKey transactionAutoCommitTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, - "transactionAutoCommitTimeout", PredefinedTypes.TYPE_INT, false); + TRANSACTION_AUTO_COMMIT_TIMEOUT_KEY, PredefinedTypes.TYPE_INT, false); Object value = ConfigMap.get(transactionAutoCommitTimeoutKey); if (value == null) { return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; @@ -235,7 +237,8 @@ public int getTransactionAutoCommitTimeout() { * @return int transaction cleanup after value */ public int getTransactionCleanupTimeout() { - VariableKey transactionCleanupTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, "transactionCleanupTimeout", + VariableKey transactionCleanupTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, + TRANSACTION_CLEANUP_TIMEOUT_KEY, PredefinedTypes.TYPE_INT, false); Object value = ConfigMap.get(transactionCleanupTimeoutKey); if (value == null) { From 557985aea859fb4df1c2931610914bf9970b3230 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Wed, 19 Jun 2024 11:01:11 +0530 Subject: [PATCH 10/17] Address review suggestions --- .../runtime/internal/errors/ErrorCodes.java | 4 +- .../TransactionResourceManager.java | 67 ++++++------------- .../main/resources/MessagesBundle.properties | 6 +- 3 files changed, 24 insertions(+), 53 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java index a0e465542db6..1b01390acfc1 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java @@ -150,8 +150,8 @@ public enum ErrorCodes implements DiagnosticCode { CONFIG_TOML_INVALID_MODULE_STRUCTURE_WITH_VARIABLE("config.toml.invalid.module.structure.with.variable", "RUNTIME_0121"), EMPTY_XML_SEQUENCE_HAS_NO_ATTRIBUTES("empty.xml.sequence.no.attributes", "RUNTIME_0122"), - INVALID_TRANSACTION_AUTO_COMMIT_TIMEOUT("invalid.transaction.auto.commit.value", "RUNTIME_0131"), - INVALID_TRANSACTION_CLEANUP_TIMEOUT("invalid.transaction.cleanup.timeout", "RUNTIME_0132"); + INVALID_FUNCTION_INVOCATION("invalid.function.invocation.call", "RUNTIME_0130"), + INVALID_TRANSACTION_TIMEOUT("invalid.transaction.value", "RUNTIME_0131"); private final String errorMsgKey; private final String errorCode; diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java index 3ffe5727f4e8..90822011e4b8 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java @@ -204,31 +204,8 @@ public int getTransactionAutoCommitTimeout() { Object value = ConfigMap.get(transactionAutoCommitTimeoutKey); if (value == null) { return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; - } else { - int transactionAutoCommitTimeout; - if (value instanceof Long) { - transactionAutoCommitTimeout = ((Long) value).intValue(); - } else if (value instanceof Integer) { - transactionAutoCommitTimeout = (Integer) value; - } else { - diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_AUTO_COMMIT_TIMEOUT, null, value, - DEFAULT_TRX_AUTO_COMMIT_TIMEOUT); - if (!diagnosticLog.getDiagnosticList().isEmpty()) { - RuntimeUtils.handleDiagnosticErrors(diagnosticLog); - } - return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; - } - if (transactionAutoCommitTimeout <= 0) { - diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_CLEANUP_TIMEOUT, null, - transactionAutoCommitTimeout, DEFAULT_TRX_AUTO_COMMIT_TIMEOUT); - if (!diagnosticLog.getDiagnosticList().isEmpty()) { - RuntimeUtils.handleDiagnosticErrors(diagnosticLog); - } - return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; - } else { - return transactionAutoCommitTimeout; - } } + return parseTimeoutValue(value, TRANSACTION_AUTO_COMMIT_TIMEOUT_KEY, DEFAULT_TRX_AUTO_COMMIT_TIMEOUT); } /** @@ -243,31 +220,27 @@ public int getTransactionCleanupTimeout() { Object value = ConfigMap.get(transactionCleanupTimeoutKey); if (value == null) { return DEFAULT_TRX_CLEANUP_TIMEOUT; + } + return parseTimeoutValue(value, TRANSACTION_CLEANUP_TIMEOUT_KEY, DEFAULT_TRX_CLEANUP_TIMEOUT); + } + + private int parseTimeoutValue(Object value, String key, int defaultValue) { + int timeoutValue; + if (value instanceof Long longVal) { + timeoutValue = longVal.intValue(); + } else if (value instanceof Integer intVal) { + timeoutValue = intVal; } else { - int transactionCleanupTimeout; - if (value instanceof Long) { - transactionCleanupTimeout = ((Long) value).intValue(); - } else if (value instanceof Integer) { - transactionCleanupTimeout = (Integer) value; - } else { - diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_CLEANUP_TIMEOUT, null, value, - DEFAULT_TRX_CLEANUP_TIMEOUT); - if (!diagnosticLog.getDiagnosticList().isEmpty()) { - RuntimeUtils.handleDiagnosticErrors(diagnosticLog); - } - return DEFAULT_TRX_CLEANUP_TIMEOUT; - } - if (transactionCleanupTimeout <= 0) { - diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_CLEANUP_TIMEOUT, null, - transactionCleanupTimeout, DEFAULT_TRX_CLEANUP_TIMEOUT); - if (!diagnosticLog.getDiagnosticList().isEmpty()) { - RuntimeUtils.handleDiagnosticErrors(diagnosticLog); - } - return DEFAULT_TRX_CLEANUP_TIMEOUT; - } else { - return transactionCleanupTimeout; - } + diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_TIMEOUT, null, key, value, defaultValue); + RuntimeUtils.handleDiagnosticErrors(diagnosticLog); + return defaultValue; + } + if (timeoutValue <= 0) { + diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_TIMEOUT, null, key, value, defaultValue); + RuntimeUtils.handleDiagnosticErrors(diagnosticLog); + return defaultValue; } + return timeoutValue; } /** diff --git a/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties b/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties index ac1281830e2f..f93cbd441d99 100644 --- a/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties +++ b/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties @@ -196,10 +196,8 @@ merge.json.error = cannot merge JSON values of types ''{0}'' and ''{1}'' field.removal.not.allowed = failed to remove field: ''{0}'' is a required field in ''{1}'' operation.not.supported = {0} not supported on type ''{1}'' invalid.fraction.digits = fraction digits cannot be less than '0' -invalid.transaction.auto.commit.timeout = provided transaction auto commit timeout of ''{0}'' is invalid, default \ - value of ''{1}'' will be used instead. -invalid.transaction.cleanup.timeout = provided transaction cleanup timeout of ''{0}'' is invalid, default value of \ - ''{1}'' will be used instead. +invalid.transaction.timeout = provided ''{0}'' of ''{1}'' is invalid, default \ + value of ''{2}'' will be used instead. #configurations config.type.not.supported = configurable variable ''{0}'' with type ''{1}'' is not supported From fdd943714fbaf62977abf617cc1e70cfca38f8d4 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Wed, 19 Jun 2024 11:50:01 +0530 Subject: [PATCH 11/17] Fix typo --- .../java/io/ballerina/runtime/internal/errors/ErrorCodes.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java index 1b01390acfc1..2cd4e7c21d9a 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java @@ -151,7 +151,7 @@ public enum ErrorCodes implements DiagnosticCode { "RUNTIME_0121"), EMPTY_XML_SEQUENCE_HAS_NO_ATTRIBUTES("empty.xml.sequence.no.attributes", "RUNTIME_0122"), INVALID_FUNCTION_INVOCATION("invalid.function.invocation.call", "RUNTIME_0130"), - INVALID_TRANSACTION_TIMEOUT("invalid.transaction.value", "RUNTIME_0131"); + INVALID_TRANSACTION_TIMEOUT("invalid.transaction.timeout", "RUNTIME_0131"); private final String errorMsgKey; private final String errorCode; From c56005ff583dca5444f1cb5d88d6b9236cd819b7 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Wed, 19 Jun 2024 14:33:51 +0530 Subject: [PATCH 12/17] Remove unnecessary warning messages --- .../io/ballerina/runtime/internal/errors/ErrorCodes.java | 4 +--- .../runtime/transactions/TransactionResourceManager.java | 7 ------- .../src/main/resources/MessagesBundle.properties | 3 --- 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java index 2cd4e7c21d9a..a969630c6a83 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/errors/ErrorCodes.java @@ -149,9 +149,7 @@ public enum ErrorCodes implements DiagnosticCode { REGEXP_INVALID_HEX_DIGIT("regexp.invalid.hex.digit", "RUNTIME_0120"), CONFIG_TOML_INVALID_MODULE_STRUCTURE_WITH_VARIABLE("config.toml.invalid.module.structure.with.variable", "RUNTIME_0121"), - EMPTY_XML_SEQUENCE_HAS_NO_ATTRIBUTES("empty.xml.sequence.no.attributes", "RUNTIME_0122"), - INVALID_FUNCTION_INVOCATION("invalid.function.invocation.call", "RUNTIME_0130"), - INVALID_TRANSACTION_TIMEOUT("invalid.transaction.timeout", "RUNTIME_0131"); + EMPTY_XML_SEQUENCE_HAS_NO_ATTRIBUTES("empty.xml.sequence.no.attributes", "RUNTIME_0122"); private final String errorMsgKey; private final String errorCode; diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java index 90822011e4b8..ea0d9889d327 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java @@ -27,8 +27,6 @@ import io.ballerina.runtime.api.values.BString; import io.ballerina.runtime.internal.configurable.ConfigMap; import io.ballerina.runtime.internal.configurable.VariableKey; -import io.ballerina.runtime.internal.diagnostics.RuntimeDiagnosticLog; -import io.ballerina.runtime.internal.errors.ErrorCodes; import io.ballerina.runtime.internal.scheduling.Scheduler; import io.ballerina.runtime.internal.scheduling.Strand; import io.ballerina.runtime.internal.util.RuntimeUtils; @@ -105,7 +103,6 @@ public class TransactionResourceManager { private boolean transactionManagerEnabled; private static final PrintStream stderr = System.err; - RuntimeDiagnosticLog diagnosticLog = new RuntimeDiagnosticLog(); Map transactionInfoMap; @@ -231,13 +228,9 @@ private int parseTimeoutValue(Object value, String key, int defaultValue) { } else if (value instanceof Integer intVal) { timeoutValue = intVal; } else { - diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_TIMEOUT, null, key, value, defaultValue); - RuntimeUtils.handleDiagnosticErrors(diagnosticLog); return defaultValue; } if (timeoutValue <= 0) { - diagnosticLog.warn(ErrorCodes.INVALID_TRANSACTION_TIMEOUT, null, key, value, defaultValue); - RuntimeUtils.handleDiagnosticErrors(diagnosticLog); return defaultValue; } return timeoutValue; diff --git a/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties b/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties index f93cbd441d99..297d6b945c74 100644 --- a/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties +++ b/bvm/ballerina-runtime/src/main/resources/MessagesBundle.properties @@ -196,9 +196,6 @@ merge.json.error = cannot merge JSON values of types ''{0}'' and ''{1}'' field.removal.not.allowed = failed to remove field: ''{0}'' is a required field in ''{1}'' operation.not.supported = {0} not supported on type ''{1}'' invalid.fraction.digits = fraction digits cannot be less than '0' -invalid.transaction.timeout = provided ''{0}'' of ''{1}'' is invalid, default \ - value of ''{2}'' will be used instead. - #configurations config.type.not.supported = configurable variable ''{0}'' with type ''{1}'' is not supported config.invalid.byte.range = [{0}] value provided for byte variable ''{1}'' is out of range. Expected range is \ From 6a11f6656b0a7011c19f2673ae7ce8d5feb81664 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Wed, 19 Jun 2024 16:42:34 +0530 Subject: [PATCH 13/17] Refactor parseTimeoutValue --- .../TransactionResourceManager.java | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java index ea0d9889d327..ad5f391b9577 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java @@ -195,14 +195,14 @@ private String getTransactionLogDirectory() { * * @return int transaction auto commit timeout value */ - public int getTransactionAutoCommitTimeout() { + public static int getTransactionAutoCommitTimeout() { VariableKey transactionAutoCommitTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, TRANSACTION_AUTO_COMMIT_TIMEOUT_KEY, PredefinedTypes.TYPE_INT, false); Object value = ConfigMap.get(transactionAutoCommitTimeoutKey); if (value == null) { return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; } - return parseTimeoutValue(value, TRANSACTION_AUTO_COMMIT_TIMEOUT_KEY, DEFAULT_TRX_AUTO_COMMIT_TIMEOUT); + return parseTimeoutValue(value, DEFAULT_TRX_AUTO_COMMIT_TIMEOUT); } /** @@ -210,7 +210,7 @@ public int getTransactionAutoCommitTimeout() { * * @return int transaction cleanup after value */ - public int getTransactionCleanupTimeout() { + public static int getTransactionCleanupTimeout() { VariableKey transactionCleanupTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, TRANSACTION_CLEANUP_TIMEOUT_KEY, PredefinedTypes.TYPE_INT, false); @@ -218,18 +218,14 @@ public int getTransactionCleanupTimeout() { if (value == null) { return DEFAULT_TRX_CLEANUP_TIMEOUT; } - return parseTimeoutValue(value, TRANSACTION_CLEANUP_TIMEOUT_KEY, DEFAULT_TRX_CLEANUP_TIMEOUT); + return parseTimeoutValue(value, DEFAULT_TRX_CLEANUP_TIMEOUT); } - private int parseTimeoutValue(Object value, String key, int defaultValue) { - int timeoutValue; - if (value instanceof Long longVal) { - timeoutValue = longVal.intValue(); - } else if (value instanceof Integer intVal) { - timeoutValue = intVal; - } else { + private static int parseTimeoutValue(Object value, int defaultValue) { + if (!(value instanceof Number number)) { return defaultValue; } + int timeoutValue = number.intValue(); if (timeoutValue <= 0) { return defaultValue; } From f62a945c45177afde9fb1312dbd730fd09c14979 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Fri, 21 Jun 2024 12:14:56 +0530 Subject: [PATCH 14/17] Address review suggestions --- .../TransactionResourceManager.java | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java index ad5f391b9577..ef8a06d2d54a 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java @@ -198,11 +198,15 @@ private String getTransactionLogDirectory() { public static int getTransactionAutoCommitTimeout() { VariableKey transactionAutoCommitTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, TRANSACTION_AUTO_COMMIT_TIMEOUT_KEY, PredefinedTypes.TYPE_INT, false); - Object value = ConfigMap.get(transactionAutoCommitTimeoutKey); - if (value == null) { + if (!ConfigMap.containsKey(transactionAutoCommitTimeoutKey)) { return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; + } else { + Object configValue = ConfigMap.get(transactionAutoCommitTimeoutKey); + if (configValue == null) { + return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; + } + return parseTimeoutValue(configValue, DEFAULT_TRX_AUTO_COMMIT_TIMEOUT); } - return parseTimeoutValue(value, DEFAULT_TRX_AUTO_COMMIT_TIMEOUT); } /** @@ -214,15 +218,19 @@ public static int getTransactionCleanupTimeout() { VariableKey transactionCleanupTimeoutKey = new VariableKey(TRANSACTION_PACKAGE_ID, TRANSACTION_CLEANUP_TIMEOUT_KEY, PredefinedTypes.TYPE_INT, false); - Object value = ConfigMap.get(transactionCleanupTimeoutKey); - if (value == null) { - return DEFAULT_TRX_CLEANUP_TIMEOUT; + if (!ConfigMap.containsKey(transactionCleanupTimeoutKey)) { + return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; + } else { + Object configValue = ConfigMap.get(transactionCleanupTimeoutKey); + if (configValue == null) { + return DEFAULT_TRX_CLEANUP_TIMEOUT; + } + return parseTimeoutValue(configValue, DEFAULT_TRX_CLEANUP_TIMEOUT); } - return parseTimeoutValue(value, DEFAULT_TRX_CLEANUP_TIMEOUT); } - private static int parseTimeoutValue(Object value, int defaultValue) { - if (!(value instanceof Number number)) { + private static int parseTimeoutValue(Object configValue, int defaultValue) { + if (!(configValue instanceof Number number)) { return defaultValue; } int timeoutValue = number.intValue(); From 3a32d364c827f8d3b84e3b4ad658d6d3e18073a0 Mon Sep 17 00:00:00 2001 From: dsplayerX Date: Fri, 21 Jun 2024 20:16:23 +0530 Subject: [PATCH 15/17] Fix default return type of getTransactionCleanupTimeout --- .../runtime/transactions/TransactionResourceManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java index ef8a06d2d54a..bdec6396502a 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/transactions/TransactionResourceManager.java @@ -219,7 +219,7 @@ public static int getTransactionCleanupTimeout() { TRANSACTION_CLEANUP_TIMEOUT_KEY, PredefinedTypes.TYPE_INT, false); if (!ConfigMap.containsKey(transactionCleanupTimeoutKey)) { - return DEFAULT_TRX_AUTO_COMMIT_TIMEOUT; + return DEFAULT_TRX_CLEANUP_TIMEOUT; } else { Object configValue = ConfigMap.get(transactionCleanupTimeoutKey); if (configValue == null) { From 1d374a38b51efffb0d91952adf2a130250c7bca4 Mon Sep 17 00:00:00 2001 From: ballerina-bot Date: Thu, 27 Jun 2024 16:28:52 +0000 Subject: [PATCH 16/17] [Gradle Release Plugin] - pre tag commit: 'v2201.8.7'. --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 6acca048fb7f..9e469725036f 100644 --- a/gradle.properties +++ b/gradle.properties @@ -6,7 +6,7 @@ org.gradle.vfs.watch=false systemProp.scan.capture-build-logging=false systemProp.scan.capture-test-logging=false -version=2201.8.7-SNAPSHOT +version=2201.8.7 group=org.ballerinalang bootstrappedOn=1.1.0-alpha specVersion=2023R1 From d61748b819d6fdcc2a86efe93e1d05108755c8fb Mon Sep 17 00:00:00 2001 From: ballerina-bot Date: Thu, 27 Jun 2024 16:28:54 +0000 Subject: [PATCH 17/17] [Gradle Release Plugin] - new version commit: 'v2201.8.8-SNAPSHOT'. --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 9e469725036f..a601e0438708 100644 --- a/gradle.properties +++ b/gradle.properties @@ -6,7 +6,7 @@ org.gradle.vfs.watch=false systemProp.scan.capture-build-logging=false systemProp.scan.capture-test-logging=false -version=2201.8.7 +version=2201.8.8-SNAPSHOT group=org.ballerinalang bootstrappedOn=1.1.0-alpha specVersion=2023R1