Skip to content

Commit 70b1a9a

Browse files
JAORMXJAORMX
authored
Don't return errors when getting properties (#5515)
* Don't return errors when getting properties This simplifies the calling pattern. Signed-off-by: Juan Antonio Osorio <[email protected]> * Address PR feedback Signed-off-by: Juan Antonio Osorio <[email protected]> * More feedback addressing Signed-off-by: Juan Antonio Osorio <[email protected]> --------- Signed-off-by: Juan Antonio Osorio <[email protected]>
1 parent 3e99ca4 commit 70b1a9a

35 files changed

+144
-311
lines changed

‎cmd/dev/app/rule_type/rttst.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -371,10 +371,7 @@ func readEntityWithPropertiesFromFile(
371371
return nil, fmt.Errorf("error decoding json: %w", err)
372372
}
373373

374-
props, err := entProps.NewProperties(propertiesMap)
375-
if err != nil {
376-
return nil, fmt.Errorf("error creating properties: %w", err)
377-
}
374+
props := entProps.NewProperties(propertiesMap)
378375

379376
return &entModels.EntityWithProperties{
380377
Entity: entModels.EntityInstance{

‎internal/controlplane/handlers_entities.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,19 +112,15 @@ func createEntityMessage(
112112
msg := message.NewMessage(uuid.New().String(), nil)
113113
msg.SetContext(ctx)
114114

115-
repoProps, err := properties.NewProperties(props.AsMap())
116-
if err != nil {
117-
return nil, err
118-
}
115+
repoProps := properties.NewProperties(props.AsMap())
119116

120117
event := messages.NewMinderEvent().
121118
WithProjectID(projectID).
122119
WithProviderID(providerID).
123120
WithEntityType(pb.Entity_ENTITY_REPOSITORIES).
124121
WithProperties(repoProps)
125122

126-
err = event.ToMessage(msg)
127-
if err != nil {
123+
if err := event.ToMessage(msg); err != nil {
128124
l.Error().Err(err).Msg("error marshalling register entities message")
129125
return nil, err
130126
}

‎internal/controlplane/handlers_repositories.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,10 @@ func (s *Server) repoCreateInfoFromUpstreamRepositoryRef(
100100
return nil, nil, util.UserVisibleError(codes.InvalidArgument, "missing repository name")
101101
}
102102

103-
fetchByProps, err := properties.NewProperties(map[string]any{
103+
fetchByProps := properties.NewProperties(map[string]any{
104104
properties.PropertyUpstreamID: fmt.Sprintf("%d", rep.GetRepoId()),
105105
properties.PropertyName: fmt.Sprintf("%s/%s", rep.GetOwner(), rep.GetName()),
106106
})
107-
if err != nil {
108-
return nil, nil, fmt.Errorf("error creating properties: %w", err)
109-
}
110107

111108
provider, err := s.inferProviderByOwner(ctx, rep.GetOwner(), projectID, providerName)
112109
if err != nil {
@@ -127,10 +124,7 @@ func (s *Server) repoCreateInfoFromUpstreamEntityRef(
127124
entity *pb.UpstreamEntityRef,
128125
) (*properties.Properties, *db.Provider, error) {
129126
inPropsMap := entity.GetProperties().AsMap()
130-
fetchByProps, err := properties.NewProperties(inPropsMap)
131-
if err != nil {
132-
return nil, nil, fmt.Errorf("error creating properties: %w", err)
133-
}
127+
fetchByProps := properties.NewProperties(inPropsMap)
134128

135129
provider, err := s.providerStore.GetByName(ctx, projectID, providerName)
136130
if err != nil {

‎internal/controlplane/handlers_repositories_test.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ func mustNewPBStruct(m map[string]any) *structpb.Struct {
443443

444444
func simpleDbRepository(name string, id int64) *models.EntityWithProperties {
445445
//nolint:errcheck // this shouldn't fail
446-
props, _ := properties.NewProperties(map[string]any{
446+
props := properties.NewProperties(map[string]any{
447447
"repo_id": id,
448448
properties.PropertyUpstreamID: fmt.Sprintf("%d", id),
449449
})
@@ -454,15 +454,12 @@ func simpleDbRepository(name string, id int64) *models.EntityWithProperties {
454454
}
455455

456456
func simpleUpstreamRepositoryRef(name string, id int64, registered bool) *UpstreamRepoAndEntityRef {
457-
props, err := properties.NewProperties(map[string]any{
457+
props := properties.NewProperties(map[string]any{
458458
properties.PropertyUpstreamID: fmt.Sprintf("%d", id),
459459
ghprops.RepoPropertyId: id,
460460
ghprops.RepoPropertyName: name,
461461
ghprops.RepoPropertyOwner: repoOwner,
462462
})
463-
if err != nil {
464-
panic(err)
465-
}
466463

467464
return &UpstreamRepoAndEntityRef{
468465
Repo: &pb.UpstreamRepositoryRef{

‎internal/entities/handlers/handler.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,7 @@ func (*handleEntityAndDoBase) matchPropertiesCheck(
159159
return nil
160160
}
161161

162-
matchProps, err := properties.NewProperties(entMsg.MatchProps)
163-
if err != nil {
164-
return err
165-
}
162+
matchProps := properties.NewProperties(entMsg.MatchProps)
166163

167164
for propName, prop := range matchProps.Iterate() {
168165
entProp := ewp.Properties.GetProperty(propName)

‎internal/entities/handlers/handler_test.go‎

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,7 @@ type (
9797
)
9898

9999
func getPullRequestProperties() *properties.Properties {
100-
props, err := properties.NewProperties(pullRequestPropMap)
101-
if err != nil {
102-
panic(err)
103-
}
104-
105-
return props
100+
return properties.NewProperties(pullRequestPropMap)
106101
}
107102

108103
func newProviderMock(opts ...func(providerMock)) providerMockBuilder {
@@ -142,9 +137,7 @@ func WithSuccessfulPropertiesToProtoMessage(proto protoreflect.ProtoMessage) fun
142137
func buildEwp(t *testing.T, ewp models.EntityWithProperties, propMap map[string]any) *models.EntityWithProperties {
143138
t.Helper()
144139

145-
entProps, err := properties.NewProperties(propMap)
146-
require.NoError(t, err)
147-
ewp.Properties = entProps
140+
ewp.Properties = properties.NewProperties(propMap)
148141

149142
return &ewp
150143
}
@@ -298,7 +291,7 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
298291
name: "NewRefreshEntityAndEvaluateHandler: successful refresh and publish of a repo",
299292
handlerBuilderFn: refreshEntityHandlerBuilder,
300293
messageBuilder: func() *message.HandleEntityAndDoMessage {
301-
getByProps, _ := properties.NewProperties(map[string]any{
294+
getByProps := properties.NewProperties(map[string]any{
302295
properties.PropertyUpstreamID: "123",
303296
})
304297

@@ -328,11 +321,11 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
328321
name: "NewRefreshEntityAndEvaluateHandler: if match_props match, publish",
329322
handlerBuilderFn: refreshEntityHandlerBuilder,
330323
messageBuilder: func() *message.HandleEntityAndDoMessage {
331-
getByProps, _ := properties.NewProperties(map[string]any{
324+
getByProps := properties.NewProperties(map[string]any{
332325
properties.PropertyUpstreamID: "123",
333326
})
334327

335-
matchProps, _ := properties.NewProperties(map[string]any{
328+
matchProps := properties.NewProperties(map[string]any{
336329
ghprops.RepoPropertyHookId: repoPropHookID,
337330
})
338331

@@ -363,11 +356,11 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
363356
name: "NewRefreshEntityAndEvaluateHandler: if match_props don't match, don't publish",
364357
handlerBuilderFn: refreshEntityHandlerBuilder,
365358
messageBuilder: func() *message.HandleEntityAndDoMessage {
366-
getByProps, _ := properties.NewProperties(map[string]any{
359+
getByProps := properties.NewProperties(map[string]any{
367360
properties.PropertyUpstreamID: "123",
368361
})
369362

370-
matchProps, _ := properties.NewProperties(map[string]any{
363+
matchProps := properties.NewProperties(map[string]any{
371364
ghprops.RepoPropertyHookId: repoPropHookID + 1,
372365
})
373366

@@ -393,7 +386,7 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
393386
name: "NewRefreshEntityAndEvaluateHandler: private repo publishes if feature enabled",
394387
handlerBuilderFn: refreshEntityHandlerBuilder,
395388
messageBuilder: func() *message.HandleEntityAndDoMessage {
396-
getByProps, _ := properties.NewProperties(map[string]any{
389+
getByProps := properties.NewProperties(map[string]any{
397390
properties.PropertyUpstreamID: "123",
398391
})
399392

@@ -427,7 +420,7 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
427420
name: "NewRefreshEntityAndEvaluateHandler: private repo does not publish if feature disabled",
428421
handlerBuilderFn: refreshEntityHandlerBuilder,
429422
messageBuilder: func() *message.HandleEntityAndDoMessage {
430-
getByProps, _ := properties.NewProperties(map[string]any{
423+
getByProps := properties.NewProperties(map[string]any{
431424
properties.PropertyUpstreamID: "123",
432425
})
433426

@@ -456,7 +449,7 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
456449
name: "NewRefreshEntityAndEvaluateHandler: archived repo does not publish",
457450
handlerBuilderFn: refreshEntityHandlerBuilder,
458451
messageBuilder: func() *message.HandleEntityAndDoMessage {
459-
getByProps, _ := properties.NewProperties(map[string]any{
452+
getByProps := properties.NewProperties(map[string]any{
460453
properties.PropertyUpstreamID: "123",
461454
})
462455

@@ -484,7 +477,7 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
484477
name: "NewRefreshEntityAndEvaluateHandler: Failure to get an entity doesn't publish",
485478
handlerBuilderFn: refreshEntityHandlerBuilder,
486479
messageBuilder: func() *message.HandleEntityAndDoMessage {
487-
getByProps, _ := properties.NewProperties(map[string]any{
480+
getByProps := properties.NewProperties(map[string]any{
488481
properties.PropertyUpstreamID: "123",
489482
})
490483

@@ -506,7 +499,7 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
506499
name: "NewRefreshEntityAndEvaluateHandler: Failure to retrieve all properties doesn't publish",
507500
handlerBuilderFn: refreshEntityHandlerBuilder,
508501
messageBuilder: func() *message.HandleEntityAndDoMessage {
509-
getByProps, _ := properties.NewProperties(map[string]any{
502+
getByProps := properties.NewProperties(map[string]any{
510503
properties.PropertyUpstreamID: "123",
511504
})
512505

@@ -529,7 +522,7 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
529522
name: "NewRefreshEntityAndEvaluateHandler: Failure to convert entity to proto doesn't publish",
530523
handlerBuilderFn: refreshEntityHandlerBuilder,
531524
messageBuilder: func() *message.HandleEntityAndDoMessage {
532-
getByProps, _ := properties.NewProperties(map[string]any{
525+
getByProps := properties.NewProperties(map[string]any{
533526
properties.PropertyUpstreamID: "123",
534527
})
535528

@@ -553,11 +546,11 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
553546
name: "NewAddOriginatingEntityHandler: Adding a pull request originating entity publishes",
554547
handlerBuilderFn: addOriginatingEntityHandlerBuilder,
555548
messageBuilder: func() *message.HandleEntityAndDoMessage {
556-
prProps, _ := properties.NewProperties(map[string]any{
549+
prProps := properties.NewProperties(map[string]any{
557550
properties.PropertyUpstreamID: "789",
558551
ghprops.PullPropertyNumber: int64(789),
559552
})
560-
originatorProps, _ := properties.NewProperties(map[string]any{
553+
originatorProps := properties.NewProperties(map[string]any{
561554
properties.PropertyUpstreamID: "123",
562555
})
563556

@@ -627,11 +620,11 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
627620
name: "NewRemoveOriginatingEntityHandler: Happy path does not publish",
628621
handlerBuilderFn: removeOriginatingEntityHandlerBuilder,
629622
messageBuilder: func() *message.HandleEntityAndDoMessage {
630-
prProps, _ := properties.NewProperties(map[string]any{
623+
prProps := properties.NewProperties(map[string]any{
631624
properties.PropertyUpstreamID: "789",
632625
ghprops.PullPropertyNumber: int64(789),
633626
})
634-
originatorProps, _ := properties.NewProperties(map[string]any{
627+
originatorProps := properties.NewProperties(map[string]any{
635628
properties.PropertyUpstreamID: "123",
636629
})
637630

@@ -663,7 +656,7 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
663656
name: "NewGetEntityAndDeleteHandler: happy path publishes",
664657
handlerBuilderFn: getAndDeleteEntityHandlerBuilder,
665658
messageBuilder: func() *message.HandleEntityAndDoMessage {
666-
getByProps, _ := properties.NewProperties(map[string]any{
659+
getByProps := properties.NewProperties(map[string]any{
667660
properties.PropertyUpstreamID: "123",
668661
})
669662

@@ -691,7 +684,7 @@ func TestRefreshEntityAndDoHandler_HandleRefreshEntityAndEval(t *testing.T) {
691684
name: "NewGetEntityAndDeleteHandler: failure to get entity does not publish",
692685
handlerBuilderFn: getAndDeleteEntityHandlerBuilder,
693686
messageBuilder: func() *message.HandleEntityAndDoMessage {
694-
getByProps, _ := properties.NewProperties(map[string]any{
687+
getByProps := properties.NewProperties(map[string]any{
695688
properties.PropertyUpstreamID: "123",
696689
})
697690

‎internal/entities/handlers/message/message_test.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,32 +84,29 @@ func TestEntityRefreshAndDoMessageRoundTrip(t *testing.T) {
8484
t.Run(sc.name, func(t *testing.T) {
8585
t.Parallel()
8686

87-
props, err := properties.NewProperties(sc.props)
88-
require.NoError(t, err)
87+
props := properties.NewProperties(sc.props)
8988

9089
original := NewEntityRefreshAndDoMessage().
9190
WithEntity(sc.entType, props).
9291
WithProviderImplementsHint(sc.providerHint).
9392
WithProviderClassHint(sc.providerClass)
9493

9594
if sc.ownerProps != nil {
96-
ownerProps, err := properties.NewProperties(sc.ownerProps)
97-
require.NoError(t, err)
95+
ownerProps := properties.NewProperties(sc.ownerProps)
9896
original.WithOriginator(sc.ownerType, ownerProps)
9997
}
10098

10199
if sc.matchProps != nil {
102-
matchProps, err := properties.NewProperties(sc.matchProps)
103-
require.NoError(t, err)
100+
matchProps := properties.NewProperties(sc.matchProps)
104101
original.WithMatchProps(matchProps)
105102
}
106103

107104
handlerMsg := message.NewMessage(uuid.New().String(), nil)
108-
err = original.ToMessage(handlerMsg)
105+
err := original.ToMessage(handlerMsg)
109106
require.NoError(t, err)
110107

111108
roundTrip, err := ToEntityRefreshAndDo(handlerMsg)
112-
assert.NoError(t, err)
109+
require.NoError(t, err)
113110
assert.Equal(t, original.Entity.GetByProps, roundTrip.Entity.GetByProps)
114111
assert.Equal(t, original.Entity.Type, roundTrip.Entity.Type)
115112
assert.Equal(t, original.Hint.ProviderImplementsHint, roundTrip.Hint.ProviderImplementsHint)

‎internal/entities/handlers/strategies/entity/add_originating_entity.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,7 @@ func NewAddOriginatingEntityStrategy(
4444
func (a *addOriginatingEntityStrategy) GetEntity(
4545
ctx context.Context, entMsg *message.HandleEntityAndDoMessage,
4646
) (*models.EntityWithProperties, error) {
47-
childProps, err := properties.NewProperties(entMsg.Entity.GetByProps)
48-
if err != nil {
49-
return nil, fmt.Errorf("error creating properties: %w", err)
50-
}
47+
childProps := properties.NewProperties(entMsg.Entity.GetByProps)
5148

5249
// store the originating entity
5350
childEwp, err := db.WithTransaction(a.store, func(t db.ExtendQuerier) (*models.EntityWithProperties, error) {

‎internal/entities/handlers/strategies/entity/common.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,7 @@ func getEntityInner(
3737
}
3838
}
3939

40-
lookupProperties, err := properties.NewProperties(entPropMap)
41-
if err != nil {
42-
return nil, fmt.Errorf("error creating properties: %w", err)
43-
}
40+
lookupProperties := properties.NewProperties(entPropMap)
4441

4542
ewp, err := propSvc.EntityWithPropertiesByUpstreamHint(
4643
ctx,

‎internal/entities/models/models.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func DbPropsToModel(dbProps []db.Property) (*properties.Properties, error) {
8484
propMap[prop.Key] = anyVal
8585
}
8686

87-
return properties.NewProperties(propMap)
87+
return properties.NewProperties(propMap), nil
8888
}
8989

9090
// DbPropToModel converts a single db.Property to a properties.Property instance.

0 commit comments

Comments
 (0)