Transfer - Fix and improve long props check

artifactory/commands/transferfiles/filesdiff.go

@@ -291,8 +291,6 @@ func generateDockerManifestAqlQuery(repoKey, fromTimestamp, toTimestamp string,
 
 func generateAqlSortingPart(paginationOffset int, disabledDistinctiveAql bool) string {
 	sortingPart := fmt.Sprintf(`.sort({"$asc":["name","path"]}).offset(%d).limit(%d)`, paginationOffset*AqlPaginationLimit, AqlPaginationLimit)
-	if disabledDistinctiveAql {
-		sortingPart += `.distinct(false)`
-	}
+	sortingPart += appendDistinctIfNeeded(disabledDistinctiveAql)
 	return sortingPart
 }

artifactory/commands/transferfiles/fulltransfer.go

@@ -288,9 +288,7 @@ func generateFolderContentAqlQuery(repoKey, relativePath string, paginationOffse
 	query := fmt.Sprintf(`items.find({"type":"any","$or":[{"$and":[{"repo":"%s","path":{"$match":"%s"},"name":{"$match":"*"}}]}]})`, repoKey, relativePath)
 	query += `.include("repo","path","name","type","size")`
 	query += fmt.Sprintf(`.sort({"$asc":["name"]}).offset(%d).limit(%d)`, paginationOffset*AqlPaginationLimit, AqlPaginationLimit)
-	if disabledDistinctiveAql {
-		query += `.distinct(false)`
-	}
+	query += appendDistinctIfNeeded(disabledDistinctiveAql)
 	return query
 }
 

artifactory/commands/transferfiles/longpropertycheck.go

@@ -10,6 +10,7 @@ import (
 	"github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/transferfiles/api"
 	cmdutils "github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/utils"
 	"github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/utils/precheckrunner"
+	"github.com/jfrog/jfrog-cli-core/v2/artifactory/utils"
 	"github.com/jfrog/jfrog-cli-core/v2/utils/coreutils"
 	"github.com/jfrog/jfrog-cli-core/v2/utils/progressbar"
 	"github.com/jfrog/jfrog-client-go/artifactory"
@@ -30,6 +31,7 @@ const (
 	threadCount                = 10
 	maxAllowedValLength        = 2400
 	longPropertyCheckName      = "Properties with value longer than 2.4K characters"
+	propertiesRequestTimeout   = time.Minute * 30
 )
 
 // Property - Represents a property of an item
@@ -54,14 +56,15 @@ type FileWithLongProperty struct {
 }
 
 type LongPropertyCheck struct {
-	producerConsumer parallel.Runner
-	filesChan        chan FileWithLongProperty
-	errorsQueue      *clientutils.ErrorsQueue
-	repos            []string
+	producerConsumer       parallel.Runner
+	filesChan              chan FileWithLongProperty
+	errorsQueue            *clientutils.ErrorsQueue
+	repos                  []string
+	disabledDistinctiveAql bool
 }
 
-func NewLongPropertyCheck(repos []string) *LongPropertyCheck {
-	return &LongPropertyCheck{repos: repos}
+func NewLongPropertyCheck(repos []string, disabledDistinctiveAql bool) *LongPropertyCheck {
+	return &LongPropertyCheck{repos: repos, disabledDistinctiveAql: disabledDistinctiveAql}
 }
 
 func (lpc *LongPropertyCheck) Name() string {
@@ -114,7 +117,7 @@ func (lpc *LongPropertyCheck) ExecuteCheck(args precheckrunner.RunArguments) (pa
 // Returns the number of long properties found
 func (lpc *LongPropertyCheck) longPropertiesTaskProducer(progress *progressbar.TasksProgressBar, args precheckrunner.RunArguments) int {
 	// Init
-	serviceManager, err := createTransferServiceManager(args.Context, args.ServerDetails)
+	serviceManager, err := utils.CreateServiceManagerWithContext(args.Context, args.ServerDetails, false, 0, retries, retriesWaitMilliSecs, propertiesRequestTimeout)
 	if err != nil {
 		return 0
 	}
@@ -131,7 +134,7 @@ func (lpc *LongPropertyCheck) longPropertiesTaskProducer(progress *progressbar.T
 			if long := isLongProperty(property); long {
 				log.Debug(fmt.Sprintf(`Found long property ('@%s':'%s')`, property.Key, property.Value))
 				if lpc.producerConsumer != nil {
-					_, _ = lpc.producerConsumer.AddTaskWithError(createSearchPropertyTask(property, lpc.repos, args, lpc.filesChan, progress), lpc.errorsQueue.AddError)
+					_, _ = lpc.producerConsumer.AddTaskWithError(lpc.createSearchPropertyTask(property, args, progress), lpc.errorsQueue.AddError)
 				}
 				if progress != nil {
 					progress.IncGeneralProgressTotalBy(1)
@@ -174,25 +177,25 @@ func getSearchAllPropertiesQuery(pageNumber int) string {
 
 // Create a task that fetch from the server the files with the given property.
 // We keep only the files that are at the requested repos and pass them at the files channel
-func createSearchPropertyTask(property Property, repos []string, args precheckrunner.RunArguments, filesChan chan FileWithLongProperty, progress *progressbar.TasksProgressBar) parallel.TaskFunc {
+func (lpc *LongPropertyCheck) createSearchPropertyTask(property Property, args precheckrunner.RunArguments, progress *progressbar.TasksProgressBar) parallel.TaskFunc {
 	return func(threadId int) (err error) {
-		serviceManager, err := createTransferServiceManager(args.Context, args.ServerDetails)
+		serviceManager, err := utils.CreateServiceManagerWithContext(args.Context, args.ServerDetails, false, 0, retries, retriesWaitMilliSecs, propertiesRequestTimeout)
 		if err != nil {
 			return
 		}
 		// Search
 		var query *servicesUtils.AqlSearchResult
-		if query, err = runSearchPropertyInFilesAql(serviceManager, property); err != nil {
+		if query, err = lpc.runSearchPropertyInFilesAql(serviceManager, property); err != nil {
 			return
 		}
 		log.Debug(fmt.Sprintf("[Thread=%d] Got %d files from the query", threadId, len(query.Results)))
 		for _, item := range query.Results {
 			file := api.FileRepresentation{Repo: item.Repo, Path: item.Path, Name: item.Name}
 			// Keep only if in the requested repos
-			if slices.Contains(repos, file.Repo) {
+			if slices.Contains(lpc.repos, file.Repo) {
 				fileWithLongProperty := FileWithLongProperty{file, property.valueLength(), property}
 				log.Debug(fmt.Sprintf("[Thread=%d] Found File{Repo=%s, Path=%s, Name=%s} with matching entry of long property.", threadId, file.Repo, file.Path, file.Name))
-				filesChan <- fileWithLongProperty
+				lpc.filesChan <- fileWithLongProperty
 			}
 		}
 		// Notify end of search for the current property
@@ -204,15 +207,17 @@ func createSearchPropertyTask(property Property, repos []string, args precheckru
 }
 
 // Get all the files that contains the given property using AQL
-func runSearchPropertyInFilesAql(serviceManager artifactory.ArtifactoryServicesManager, property Property) (result *servicesUtils.AqlSearchResult, err error) {
+func (lpc *LongPropertyCheck) runSearchPropertyInFilesAql(serviceManager artifactory.ArtifactoryServicesManager, property Property) (result *servicesUtils.AqlSearchResult, err error) {
 	result = &servicesUtils.AqlSearchResult{}
-	err = runAqlService(serviceManager, getSearchPropertyInFilesQuery(property), result)
+	err = runAqlService(serviceManager, lpc.getSearchPropertyInFilesQuery(property), result)
 	return
 }
 
 // Get the query that search files with specific property
-func getSearchPropertyInFilesQuery(property Property) string {
-	return fmt.Sprintf(`items.find({"type": {"$eq":"any"},"@%s":"%s"}).include("repo","path","name")`, property.Key, property.Value)
+func (lpc *LongPropertyCheck) getSearchPropertyInFilesQuery(property Property) string {
+	query := fmt.Sprintf(`items.find({"type": {"$eq":"any"},"@%s":"%s"}).include("repo","path","name")`, property.Key, property.Value)
+	query += appendDistinctIfNeeded(lpc.disabledDistinctiveAql)
+	return query
 }
 
 // Run AQL service that return a result in the given format structure 'v'

artifactory/commands/transferfiles/longpropertycheck_test.go

@@ -77,10 +77,10 @@ func TestGetLongProperties(t *testing.T) {
 }
 
 func testGetLongProperties(t *testing.T, serverProperties, expectedLongProperties []Property) {
-	testServer, serverDetails, _ := getLongPropertyCheckStubServer(t, serverProperties, propertyToFiles)
+	testServer, serverDetails, _ := getLongPropertyCheckStubServer(t, serverProperties, propertyToFiles, false)
 	defer testServer.Close()
 
-	longPropertyCheck := NewLongPropertyCheck([]string{})
+	longPropertyCheck := NewLongPropertyCheck([]string{}, true)
 	longPropertyCheck.filesChan = make(chan FileWithLongProperty, threadCount)
 
 	count := longPropertyCheck.longPropertiesTaskProducer(nil, precheckrunner.RunArguments{Context: nil, ServerDetails: serverDetails})
@@ -105,7 +105,7 @@ func TestSearchPropertyInFilesTask(t *testing.T) {
 }
 
 func testSearchPropertyInFilesTask(t *testing.T, prop Property, specificRepos []string, propertiesFiles map[Property][]api.FileRepresentation, expected []FileWithLongProperty) {
-	testServer, serverDetails, _ := getLongPropertyCheckStubServer(t, nil, propertiesFiles)
+	testServer, serverDetails, _ := getLongPropertyCheckStubServer(t, nil, propertiesFiles, false)
 	defer testServer.Close()
 
 	var result []FileWithLongProperty
@@ -120,7 +120,11 @@ func testSearchPropertyInFilesTask(t *testing.T, prop Property, specificRepos []
 		wait.Done()
 	}()
 
-	task := createSearchPropertyTask(prop, specificRepos, precheckrunner.RunArguments{Context: nil, ServerDetails: serverDetails}, filesChan, nil)
+	longPropertyCheck := LongPropertyCheck{
+		filesChan: filesChan,
+		repos:     specificRepos,
+	}
+	task := longPropertyCheck.createSearchPropertyTask(prop, precheckrunner.RunArguments{Context: nil, ServerDetails: serverDetails}, nil)
 	assert.NoError(t, task(0))
 	close(filesChan)
 	wait.Wait()
@@ -152,10 +156,10 @@ func TestSearchLongPropertiesInFiles(t *testing.T) {
 }
 
 func testSearchPropertiesInFiles(t *testing.T, properties []Property, specificRepos []string, propertiesFiles map[Property][]api.FileRepresentation, expected []FileWithLongProperty) {
-	testServer, serverDetails, _ := getLongPropertyCheckStubServer(t, properties, propertiesFiles)
+	testServer, serverDetails, _ := getLongPropertyCheckStubServer(t, properties, propertiesFiles, false)
 	defer testServer.Close()
 
-	longPropertyCheck := NewLongPropertyCheck(specificRepos)
+	longPropertyCheck := NewLongPropertyCheck(specificRepos, false)
 	longPropertyCheck.producerConsumer = parallel.NewRunner(threadCount, maxThreadCapacity, false)
 	longPropertyCheck.filesChan = make(chan FileWithLongProperty, threadCount)
 	longPropertyCheck.errorsQueue = clientutils.NewErrorsQueue(1)
@@ -181,33 +185,34 @@ func testSearchPropertiesInFiles(t *testing.T, properties []Property, specificRe
 
 func TestLongPropertyExecuteCheck(t *testing.T) {
 	cases := []struct {
-		serverProperties []Property
-		specificRepos    []string
-		shouldPass       bool
+		serverProperties       []Property
+		specificRepos          []string
+		disabledDistinctiveAql bool
+		shouldPass             bool
 	}{
-		{[]Property{}, []string{"Repo", "OtherRepo"}, true},
-		{[]Property{property, shorterProperty, borderProperty}, []string{"Repo", "OtherRepo"}, true},
-		{[]Property{property, shorterProperty, borderProperty, longProperty, longProperty2}, []string{"Repo", "OtherRepo"}, false},
-		{[]Property{property, shorterProperty, borderProperty, longProperty2}, []string{"Repo", "OtherRepo"}, false},
-		{[]Property{property, shorterProperty, borderProperty, longProperty2}, []string{"OtherRepo"}, true},
+		{[]Property{}, []string{"Repo", "OtherRepo"}, true, true},
+		{[]Property{property, shorterProperty, borderProperty}, []string{"Repo", "OtherRepo"}, false, true},
+		{[]Property{property, shorterProperty, borderProperty, longProperty, longProperty2}, []string{"Repo", "OtherRepo"}, true, false},
+		{[]Property{property, shorterProperty, borderProperty, longProperty2}, []string{"Repo", "OtherRepo"}, false, false},
+		{[]Property{property, shorterProperty, borderProperty, longProperty2}, []string{"OtherRepo"}, true, true},
 	}
 
 	for _, testCase := range cases {
-		testLongPropertyCheckWithStubServer(t, testCase.serverProperties, testCase.specificRepos, propertyToFiles, testCase.shouldPass)
+		testLongPropertyCheckWithStubServer(t, testCase.serverProperties, testCase.specificRepos, propertyToFiles, testCase.disabledDistinctiveAql, testCase.shouldPass)
 	}
 }
 
-func testLongPropertyCheckWithStubServer(t *testing.T, serverProperties []Property, specificRepos []string, propertiesFiles map[Property][]api.FileRepresentation, shouldPass bool) {
-	testServer, serverDetails, _ := getLongPropertyCheckStubServer(t, serverProperties, propertiesFiles)
+func testLongPropertyCheckWithStubServer(t *testing.T, serverProperties []Property, specificRepos []string, propertiesFiles map[Property][]api.FileRepresentation, disabledDistinctiveAql, shouldPass bool) {
+	testServer, serverDetails, _ := getLongPropertyCheckStubServer(t, serverProperties, propertiesFiles, disabledDistinctiveAql)
 	defer testServer.Close()
 
-	longPropertyCheck := NewLongPropertyCheck(specificRepos)
+	longPropertyCheck := NewLongPropertyCheck(specificRepos, disabledDistinctiveAql)
 	passed, err := longPropertyCheck.ExecuteCheck(precheckrunner.RunArguments{Context: nil, ServerDetails: serverDetails})
 	assert.NoError(t, err)
 	assert.Equal(t, shouldPass, passed)
 }
 
-func getLongPropertyCheckStubServer(t *testing.T, serverProperties []Property, propertiesFiles map[Property][]api.FileRepresentation) (*httptest.Server, *config.ServerDetails, artifactory.ArtifactoryServicesManager) {
+func getLongPropertyCheckStubServer(t *testing.T, serverProperties []Property, propertiesFiles map[Property][]api.FileRepresentation, disabledDistinctiveAql bool) (*httptest.Server, *config.ServerDetails, artifactory.ArtifactoryServicesManager) {
 	return commonTests.CreateRtRestsMockServer(t, func(w http.ResponseWriter, r *http.Request) {
 		if r.RequestURI == "/"+"api/search/aql" {
 			content, err := io.ReadAll(r.Body)
@@ -238,6 +243,11 @@ func getLongPropertyCheckStubServer(t *testing.T, serverProperties []Property, p
 				assert.NoError(t, err)
 				_, err = w.Write(content)
 				assert.NoError(t, err)
+				if disabledDistinctiveAql {
+					assert.Contains(t, strContent, ".distinct(false)")
+				} else {
+					assert.NotContains(t, strContent, ".distinct(false)")
+				}
 			}
 		}
 	})

artifactory/commands/transferfiles/transfer.go

@@ -144,6 +144,10 @@ func (tdc *TransferFilesCommand) Run() (err error) {
 		return err
 	}
 
+	if err = tdc.initDistinctAql(); err != nil {
+		return err
+	}
+
 	if tdc.preChecks {
 		if runner, err := tdc.NewTransferDataPreChecksRunner(); err != nil {
 			return err
@@ -186,10 +190,6 @@ func (tdc *TransferFilesCommand) Run() (err error) {
 		return err
 	}
 
-	if err = tdc.initDistinctAql(); err != nil {
-		return err
-	}
-
 	if err = tdc.initStateManager(allSourceLocalRepos, sourceBuildInfoRepos); err != nil {
 		return err
 	}
@@ -341,7 +341,7 @@ func (tdc *TransferFilesCommand) NewTransferDataPreChecksRunner() (runner *prech
 	runner = precheckrunner.NewPreChecksRunner()
 
 	// Add pre checks here
-	runner.AddCheck(NewLongPropertyCheck(append(localRepos, federatedRepos...)))
+	runner.AddCheck(NewLongPropertyCheck(append(localRepos, federatedRepos...), tdc.disabledDistinctiveAql))
 
 	return
 }

artifactory/commands/transferfiles/utils.go

@@ -143,6 +143,13 @@ func createSrcRtUserPluginServiceManager(ctx context.Context, sourceRtDetails *c
 	return NewSrcUserPluginService(serviceManager.GetConfig().GetServiceDetails(), serviceManager.Client()), nil
 }
 
+func appendDistinctIfNeeded(disabledDistinctiveAql bool) string {
+	if disabledDistinctiveAql {
+		return `.distinct(false)`
+	}
+	return ""
+}
+
 func runAql(ctx context.Context, sourceRtDetails *config.ServerDetails, query string) (result *serviceUtils.AqlSearchResult, err error) {
 	serviceManager, err := createTransferServiceManager(ctx, sourceRtDetails)
 	if err != nil {

artifactory/commands/utils/precheckrunner/remoteurlchecker.go

@@ -20,7 +20,7 @@ import (
 type RemoteUrlCheckStatus string
 
 const (
-	longPropertyCheckName           = "Remote repositories URL connectivity"
+	remoteUrlCheckName              = "Remote repositories URL connectivity"
 	remoteUrlCheckPollingTimeout    = 30 * time.Minute
 	remoteUrlCheckPollingInterval   = 5 * time.Second
 	remoteUrlCheckRetries           = 3
@@ -61,7 +61,7 @@ func NewRemoteRepositoryCheck(targetServicesManager *artifactory.ArtifactoryServ
 }
 
 func (rrc *RemoteRepositoryCheck) Name() string {
-	return longPropertyCheckName
+	return remoteUrlCheckName
 }
 
 func (rrc *RemoteRepositoryCheck) ExecuteCheck(args RunArguments) (passed bool, err error) {