From 61ca952ca5da818bf35e6692a19cabdab5719e57 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Fri, 4 Oct 2024 10:46:13 +0300 Subject: [PATCH 1/2] properties service: Don't fail if multiple entries are found the `matchEntityWithHint` attempts to look for an entity using identifying properties. For PRs, the upstream ID is not sufficient since it's an integer that will clash with other PRs. In this case, we should instead continue the search and search by name which is more unique. Signed-off-by: Juan Antonio Osorio --- internal/entities/properties/service/helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/entities/properties/service/helpers.go b/internal/entities/properties/service/helpers.go index 658f341e61..32ada80cc3 100644 --- a/internal/entities/properties/service/helpers.go +++ b/internal/entities/properties/service/helpers.go @@ -232,7 +232,7 @@ func matchEntityWithHint( continue } else if errors.Is(err, ErrMultipleEntities) { l.Error().Msg("multiple entities matched") - return nil, ErrMultipleEntities + continue } return nil, fmt.Errorf("failed to match entity by hint: %w", err) } From 2cd3c64345d0294744c1b94c5b4eec9118897dac Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Fri, 4 Oct 2024 11:07:55 +0200 Subject: [PATCH 2/2] Adjust unit tests --- .../properties/service/service_test.go | 80 ++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/internal/entities/properties/service/service_test.go b/internal/entities/properties/service/service_test.go index e095db189b..fbde620a7d 100644 --- a/internal/entities/properties/service/service_test.go +++ b/internal/entities/properties/service/service_test.go @@ -675,7 +675,85 @@ func TestPropertiesService_EntityWithPropertiesByUpstreamHint(t *testing.T) { insertPropertiesFromMap(ctx, t, store, ent.ID, propMap) } }, - expectedError: ErrMultipleEntities, + expectedError: ErrEntityNotFound, + }, + { + name: "Multiple pull requests with the same ID matched by name", + entType: minderv1.Entity_ENTITY_PULL_REQUESTS, + byPropMap: map[string]any{ + properties.PropertyUpstreamID: "1", + properties.PropertyName: "test-repo-2-with-pr/1", + }, + hint: ByUpstreamHint{ + ProviderImplements: db.NullProviderType{ + ProviderType: db.ProviderTypeGithub, + Valid: true, + }, + }, + dbSetup: func(t *testing.T, store db.Store) { + t.Helper() + + repo1, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(minderv1.Entity_ENTITY_REPOSITORIES), + Name: "test-repo-1-with-pr", + ProjectID: tctx.dbProj.ID, + ProviderID: tctx.ghAppProvider.ID, + }) + require.NoError(t, err) + + repo2, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(minderv1.Entity_ENTITY_REPOSITORIES), + Name: "test-repo-2-with-pr", + ProjectID: tctx.dbProj.ID, + ProviderID: tctx.ghAppProvider.ID, + }) + require.NoError(t, err) + + repo1Pr, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(minderv1.Entity_ENTITY_PULL_REQUESTS), + Name: "test-repo-1-with-pr/1", + ProjectID: tctx.dbProj.ID, + ProviderID: tctx.ghAppProvider.ID, + OriginatedFrom: uuid.NullUUID{ + UUID: repo1.ID, + }, + }) + require.NoError(t, err) + + insertPropertiesFromMap(ctx, t, store, + repo1Pr.ID, + map[string]any{ + properties.PropertyUpstreamID: "1", + properties.PropertyName: "test-repo-1-with-pr/1", + }, + ) + + repo2Pr, err := store.CreateEntity(ctx, db.CreateEntityParams{ + EntityType: entities.EntityTypeToDB(minderv1.Entity_ENTITY_PULL_REQUESTS), + Name: "test-repo-2-with-pr/1", + ProjectID: tctx.dbProj.ID, + ProviderID: tctx.ghAppProvider.ID, + OriginatedFrom: uuid.NullUUID{ + UUID: repo2.ID, + }, + }) + require.NoError(t, err) + + insertPropertiesFromMap(ctx, t, store, + repo2Pr.ID, + map[string]any{ + properties.PropertyUpstreamID: "1", + properties.PropertyName: "test-repo-2-with-pr/1", + }, + ) + }, + checkResult: func(t *testing.T, result *models.EntityWithProperties) { + t.Helper() + require.NotNil(t, result) + require.Equal(t, "test-repo-2-with-pr/1", result.Entity.Name) + require.Equal(t, "1", result.Properties.GetProperty(properties.PropertyUpstreamID).GetString()) + require.Equal(t, tctx.ghAppProvider.ID, result.Entity.ProviderID) + }, }, { name: "One of multiple entities matched by provider hint",