Skip to content

Commit eff5928

Browse files
committed
feat: cache changed files in Gitea provider
Implemented a caching mechanism for changed files in the Gitea provider to prevent redundant API requests during the reconciliation process. This mirror the implementation on PR #2317 Jira: https://issues.redhat.com/browse/SRVKP-10944 Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1 parent 43e030f commit eff5928

File tree

2 files changed

+60
-18
lines changed

2 files changed

+60
-18
lines changed

pkg/provider/gitea/gitea.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,13 @@ type Provider struct {
6161
Token *string
6262
giteaInstanceURL string
6363
// only exposed for e2e tests
64-
Password string
65-
repo *v1alpha1.Repository
66-
eventEmitter *events.EventEmitter
67-
run *params.Run
68-
triggerEvent string
69-
pacUserID int64 // user login used by PAC
64+
Password string
65+
repo *v1alpha1.Repository
66+
eventEmitter *events.EventEmitter
67+
run *params.Run
68+
triggerEvent string
69+
pacUserID int64 // user login used by PAC
70+
cachedChangedFiles *changedfiles.ChangedFiles
7071
}
7172

7273
func (v *Provider) Client() *forgejo.Client {
@@ -535,7 +536,19 @@ type PushPayload struct {
535536
Commits []forgejo.PayloadCommit `json:"commits,omitempty"`
536537
}
537538

538-
func (v *Provider) GetFiles(_ context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) {
539+
// GetFiles gets and caches the list of files changed by a given event.
540+
func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) {
541+
if v.cachedChangedFiles == nil {
542+
changes, err := v.fetchChangedFiles(ctx, runevent)
543+
if err != nil {
544+
return changedfiles.ChangedFiles{}, err
545+
}
546+
v.cachedChangedFiles = &changes
547+
}
548+
return *v.cachedChangedFiles, nil
549+
}
550+
551+
func (v *Provider) fetchChangedFiles(_ context.Context, runevent *info.Event) (changedfiles.ChangedFiles, error) {
539552
changedFiles := changedfiles.ChangedFiles{}
540553

541554
//nolint:exhaustive // we don't need to handle all cases

pkg/provider/gitea/gitea_test.go

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,13 @@ import (
2525
"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
2626
tgitea "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitea/test"
2727
"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
28+
metricsutils "github.com/openshift-pipelines/pipelines-as-code/pkg/test/metricstest"
2829
"go.uber.org/zap"
2930
zapobserver "go.uber.org/zap/zaptest/observer"
3031
"gotest.tools/v3/assert"
3132
"gotest.tools/v3/golden"
33+
"knative.dev/pkg/metrics/metricstest"
34+
_ "knative.dev/pkg/metrics/testing"
3235
rtesting "knative.dev/pkg/reconciler/testing"
3336
)
3437

@@ -277,16 +280,17 @@ func TestProvider_Validate(t *testing.T) {
277280
}
278281
}
279282

280-
func TestProvider_GetFiles(t *testing.T) {
283+
func TestProviderGetFiles(t *testing.T) {
281284
type args struct {
282285
runevent *info.Event
283286
}
284287
tests := []struct {
285-
name string
286-
args args
287-
changedFiles string
288-
want changedfiles.ChangedFiles
289-
wantErr bool
288+
name string
289+
args args
290+
changedFiles string
291+
want changedfiles.ChangedFiles
292+
wantErr bool
293+
wantAPIRequestCount int64
290294
}{
291295
{
292296
name: "pull_request",
@@ -312,7 +316,8 @@ func TestProvider_GetFiles(t *testing.T) {
312316
Modified: []string{"modified.txt"},
313317
Renamed: []string{"renamed.txt"},
314318
},
315-
changedFiles: `[{"filename":"added.txt","status":"added"},{"filename":"deleted.txt","status":"deleted"},{"filename":"modified.txt","status":"changed"},{"filename":"renamed.txt","status":"renamed"}]`,
319+
changedFiles: `[{"filename":"added.txt","status":"added"},{"filename":"deleted.txt","status":"deleted"},{"filename":"modified.txt","status":"changed"},{"filename":"renamed.txt","status":"renamed"}]`,
320+
wantAPIRequestCount: 1,
316321
},
317322
{
318323
name: "push",
@@ -342,12 +347,12 @@ func TestProvider_GetFiles(t *testing.T) {
342347
},
343348
Deleted: []string{"deleted.txt"},
344349
Modified: []string{"modified.txt"},
345-
// Renamed: []string{"renamed.txt"},
346350
},
347351
},
348352
}
349353
for _, tt := range tests {
350354
t.Run(tt.name, func(t *testing.T) {
355+
metricsutils.ResetMetrics()
351356
fakeclient, mux, teardown := tgitea.Setup(t)
352357
defer teardown()
353358

@@ -360,12 +365,18 @@ func TestProvider_GetFiles(t *testing.T) {
360365
repo := &v1alpha1.Repository{Spec: v1alpha1.RepositorySpec{
361366
Settings: &v1alpha1.Settings{},
362367
}}
368+
giteaInstanceURL := "https://gitea.example.com"
363369
gprovider := Provider{
364-
giteaClient: fakeclient,
365-
repo: repo,
366-
Logger: logger,
370+
giteaClient: fakeclient,
371+
repo: repo,
372+
Logger: logger,
373+
giteaInstanceURL: giteaInstanceURL,
374+
triggerEvent: string(tt.args.runevent.TriggerTarget),
367375
}
368376

377+
metricsTags := map[string]string{"provider": giteaInstanceURL, "event-type": string(tt.args.runevent.TriggerTarget)}
378+
metricstest.CheckStatsNotReported(t, "pipelines_as_code_git_provider_api_request_count")
379+
369380
got, err := gprovider.GetFiles(ctx, tt.args.runevent)
370381

371382
if (err != nil) != tt.wantErr {
@@ -401,6 +412,24 @@ func TestProvider_GetFiles(t *testing.T) {
401412
if !reflect.DeepEqual(got.Renamed, tt.want.Renamed) {
402413
t.Errorf("Provider.GetFiles() Renamed = %v, want %v", got.Renamed, tt.want.Renamed)
403414
}
415+
416+
// Verify metrics from first call
417+
if tt.wantAPIRequestCount > 0 {
418+
metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, tt.wantAPIRequestCount)
419+
} else {
420+
metricstest.CheckStatsNotReported(t, "pipelines_as_code_git_provider_api_request_count")
421+
}
422+
423+
// Verify caching: second call should return cached result without additional API calls
424+
got2, err2 := gprovider.GetFiles(ctx, tt.args.runevent)
425+
assert.NilError(t, err2)
426+
assert.DeepEqual(t, got, got2)
427+
428+
if tt.wantAPIRequestCount > 0 {
429+
metricstest.CheckCountData(t, "pipelines_as_code_git_provider_api_request_count", metricsTags, tt.wantAPIRequestCount)
430+
} else {
431+
metricstest.CheckStatsNotReported(t, "pipelines_as_code_git_provider_api_request_count")
432+
}
404433
})
405434
}
406435
}

0 commit comments

Comments
 (0)