Skip to content

Commit

Permalink
changed installation logic for NPM when node_modules doesnt exist + a…
Browse files Browse the repository at this point in the history
…dded test case
  • Loading branch information
eranturgeman committed Sep 25, 2024
1 parent 4bb05e5 commit f25e282
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 10 deletions.
40 changes: 31 additions & 9 deletions build/utils/npm.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func CalculateNpmDependenciesList(executablePath, srcPath, moduleId string, npmP
log = &utils.NullLog{}
}
// Calculate npm dependency tree using 'npm ls...'.
dependenciesMap, err := CalculateDependenciesMap(executablePath, srcPath, moduleId, npmParams, log)
dependenciesMap, err := CalculateDependenciesMap(executablePath, srcPath, moduleId, npmParams, log, false)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -92,7 +92,7 @@ type dependencyInfo struct {

// Run 'npm list ...' command and parse the returned result to create a dependencies map of.
// The dependencies map looks like name:version -> entities.Dependency.
func CalculateDependenciesMap(executablePath, srcPath, moduleId string, npmListParams NpmTreeDepListParam, log utils.Log) (map[string]*dependencyInfo, error) {
func CalculateDependenciesMap(executablePath, srcPath, moduleId string, npmListParams NpmTreeDepListParam, log utils.Log, skipInstall bool) (map[string]*dependencyInfo, error) {
dependenciesMap := make(map[string]*dependencyInfo)
// These arguments must be added at the end of the command, to override their other values (if existed in nm.npmArgs).
npmVersion, err := GetNpmVersion(executablePath, log)
Expand All @@ -108,7 +108,7 @@ func CalculateDependenciesMap(executablePath, srcPath, moduleId string, npmListP
if nodeModulesExist && !npmListParams.IgnoreNodeModules {
data = runNpmLsWithNodeModules(executablePath, srcPath, npmListParams.Args, log)
} else {
data, err = runNpmLsWithoutNodeModules(executablePath, srcPath, npmListParams, log, npmVersion)
data, err = runNpmLsWithoutNodeModules(executablePath, srcPath, npmListParams, log, npmVersion, skipInstall)
if err != nil {
return nil, err
}
Expand All @@ -135,13 +135,14 @@ func runNpmLsWithNodeModules(executablePath, srcPath string, npmArgs []string, l
return
}

func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams NpmTreeDepListParam, log utils.Log, npmVersion *version.Version) ([]byte, error) {
isPackageLockExist, isDirExistsErr := utils.IsFileExists(filepath.Join(srcPath, "package-lock.json"), false)
if isDirExistsErr != nil {
return nil, isDirExistsErr
func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams NpmTreeDepListParam, log utils.Log, npmVersion *version.Version, skipInstall bool) ([]byte, error) {
installRequired, err := isInstallRequired(srcPath, npmListParams, log, skipInstall)
if err != nil {
return nil, err
}
if !isPackageLockExist || (npmListParams.OverwritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log)) {
err := installPackageLock(executablePath, srcPath, npmListParams.InstallCommandArgs, npmListParams.Args, log, npmVersion)

if installRequired {
err = installPackageLock(executablePath, srcPath, npmListParams.InstallCommandArgs, npmListParams.Args, log, npmVersion)
if err != nil {
return nil, err
}
Expand All @@ -156,6 +157,27 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np
return data, nil
}

// We verify project installation status by examining 3 things:
// 1) The presence of the package-lock.json file
// 2) The presence of installation command that has been provided by the user
// 3) A check if an override was specifically requested and the lock file should be updated
// If install command was provided by the user - we install
// If package-lock.json file is missing and OR if an override was requested and the lock file should be updated - we install, UNLESS the user requested to skip the installation
// In this case we return an error
func isInstallRequired(srcPath string, npmListParams NpmTreeDepListParam, log utils.Log, skipInstall bool) (bool, error) {
isPackageLockExist, err := utils.IsFileExists(filepath.Join(srcPath, "package-lock.json"), false)
if err != nil {
return false, err
}

if len(npmListParams.InstallCommandArgs) > 0 {
return true, nil
} else if (!isPackageLockExist || (npmListParams.OverwritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log))) && skipInstall {
return false, &utils.ErrInstallForbidden{UninstalledDir: srcPath}
}
return !isPackageLockExist || (npmListParams.OverwritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log)), nil
}

func installPackageLock(executablePath, srcPath string, npmInstallCommandArgs, npmArgs []string, log utils.Log, npmVersion *version.Version) error {
if npmVersion.AtLeast("6.0.0") {
npmArgs = append(npmArgs, "--package-lock-only")
Expand Down
66 changes: 65 additions & 1 deletion build/utils/npm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package utils

import (
"encoding/json"
"errors"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -233,6 +234,7 @@ func TestDependencyWithNoIntegrity(t *testing.T) {
assert.Greaterf(t, len(dependencies), 0, "Error: dependencies are not found!")
}

/*
// This test case verifies that CalculateNpmDependenciesList correctly handles the exclusion of 'node_modules'
// and updates 'package-lock.json' as required, based on the 'IgnoreNodeModules' and 'OverwritePackageLock' parameters.
func TestDependencyPackageLockOnly(t *testing.T) {
Expand All @@ -249,12 +251,74 @@ func TestDependencyPackageLockOnly(t *testing.T) {
// Calculate dependencies.
dependencies, err := CalculateDependenciesMap("npm", path, "jfrogtest",
NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: true, OverwritePackageLock: true}, logger)
NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: true, OverwritePackageLock: true}, logger, false)
assert.NoError(t, err)
var expectedRes = getExpectedRespForTestDependencyPackageLockOnly()
assert.Equal(t, expectedRes, dependencies)
}
*/

func TestCalculateDependenciesMap(t *testing.T) {
testCases := []struct {
name string
path string
ignoreNodeModules bool
overwritePackageLock bool
recalculateLockFile bool
skipInstall bool
}{
{
// This test case verifies that we correctly handles the exclusion of 'node_modules'
// and updates 'package-lock.json' as required, based on the 'IgnoreNodeModules' and 'OverwritePackageLock' parameters.
name: "exclude node_modules and update package-lock.json",
path: "testdata/npm/project6",
ignoreNodeModules: true,
overwritePackageLock: true,
recalculateLockFile: true,
skipInstall: false,
},
{
name: "install required but forbidden",
path: "testdata/npm/noBuildProject",
ignoreNodeModules: false,
overwritePackageLock: false,
recalculateLockFile: false,
skipInstall: true,
},
}

for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
npmVersion, _, err := GetNpmVersionAndExecPath(logger)
require.NoError(t, err)
if !npmVersion.AtLeast("7.0.0") {
t.Skip("Running on npm v7 and above only, skipping...")
}
path, cleanup := tests.CreateTestProject(t, filepath.Join("..", test.path))
defer cleanup()
if test.recalculateLockFile {
assert.NoError(t, utils.MoveFile(filepath.Join(path, "package-lock_test.json"), filepath.Join(path, "package-lock.json")))
// sleep so the package.json modified time will be bigger than the package-lock.json, this make sure it will recalculate lock file.
require.NoError(t, os.Chtimes(filepath.Join(path, "package.json"), time.Now(), time.Now().Add(time.Millisecond*20)))
}

dependencies, err := CalculateDependenciesMap("npm", path, "jfrogtest",
NpmTreeDepListParam{Args: []string{}, IgnoreNodeModules: test.ignoreNodeModules, OverwritePackageLock: test.overwritePackageLock}, logger, test.skipInstall)

if test.recalculateLockFile {
assert.NoError(t, err)
var expectedRes = getExpectedRespForTestDependencyPackageLockOnly()
assert.Equal(t, expectedRes, dependencies)
} else {
assert.Error(t, err)
var installForbiddenErr *utils.ErrInstallForbidden
assert.True(t, errors.As(err, &installForbiddenErr))
}
})
}
}

func getExpectedRespForTestDependencyPackageLockOnly() map[string]*dependencyInfo {
return map[string]*dependencyInfo{
"underscore:1.13.6": {
Expand Down

0 comments on commit f25e282

Please sign in to comment.