From 3396e6e01a4b5e694e533950e0d7f9ac9151a441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergio=20Casta=C3=B1o=20Arteaga?= Date: Tue, 7 Dec 2021 11:28:42 +0100 Subject: [PATCH] Track packages views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergio Castaño Arteaga Signed-off-by: Cintia Sanchez Garcia Co-authored-by: Sergio Castaño Arteaga Co-authored-by: Cintia Sanchez Garcia --- .github/workflows/ci.yml | 2 +- charts/artifact-hub/Chart.yaml | 2 +- charts/artifact-hub/values.yaml | 4 +- cmd/hub/main.go | 8 +- .../functions/001_load_functions.sql | 3 +- .../packages/update_packages_views.sql | 17 ++ .../migrations/schema/032_packages_views.sql | 17 ++ ...ile-postgres-pgtap => Dockerfile-postgres} | 7 +- .../packages/update_packages_views.sql | 81 ++++++++++ database/tests/schema/schema.sql | 71 +++++---- docs/dev.md | 2 +- internal/handlers/handlers.go | 11 +- internal/handlers/pkg/handlers.go | 14 ++ internal/handlers/pkg/handlers_test.go | 48 +++++- internal/hub/pkg.go | 6 + internal/pkg/mock.go | 11 ++ internal/pkg/views.go | 146 ++++++++++++++++++ internal/pkg/views_test.go | 121 +++++++++++++++ internal/util/db.go | 6 + web/src/api/index.test.tsx | 20 ++- web/src/api/index.ts | 9 ++ .../layout/package/__fixtures__/index/19.json | 22 +++ web/src/layout/package/index.test.tsx | 18 +++ web/src/layout/package/index.tsx | 10 ++ 24 files changed, 613 insertions(+), 43 deletions(-) create mode 100644 database/migrations/functions/packages/update_packages_views.sql create mode 100644 database/migrations/schema/032_packages_views.sql rename database/tests/{Dockerfile-postgres-pgtap => Dockerfile-postgres} (51%) create mode 100644 database/tests/functions/packages/update_packages_views.sql create mode 100644 internal/pkg/views.go create mode 100644 internal/pkg/views_test.go create mode 100644 web/src/layout/package/__fixtures__/index/19.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eed378f17..2d403ec74 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,7 +43,7 @@ jobs: image: artifacthub/db-tests services: postgres: - image: artifacthub/postgres-pgtap + image: artifacthub/postgres env: POSTGRES_USER: tests POSTGRES_PASSWORD: tests diff --git a/charts/artifact-hub/Chart.yaml b/charts/artifact-hub/Chart.yaml index 9170c4196..802e62ca7 100644 --- a/charts/artifact-hub/Chart.yaml +++ b/charts/artifact-hub/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: artifact-hub description: Artifact Hub is a web-based application that enables finding, installing, and publishing Kubernetes packages. type: application -version: 1.4.1-3 +version: 1.4.1-4 appVersion: 1.4.0 kubeVersion: ">= 1.19.0-0" home: https://artifacthub.io diff --git a/charts/artifact-hub/values.yaml b/charts/artifact-hub/values.yaml index 1efa04665..731bbb786 100644 --- a/charts/artifact-hub/values.yaml +++ b/charts/artifact-hub/values.yaml @@ -273,8 +273,8 @@ trivy: postgresql: enabled: true image: - repository: postgres - tag: 12 + repository: artifacthub/postgres + tag: latest persistence: mountPath: /data postgresqlUsername: postgres diff --git a/cmd/hub/main.go b/cmd/hub/main.go index 7b9e5e72f..8205bfc1a 100644 --- a/cmd/hub/main.go +++ b/cmd/hub/main.go @@ -56,6 +56,7 @@ func main() { log.Fatal().Err(err).Msg("authorizer setup failed") } hc := util.SetupHTTPClient(cfg.GetBool("restrictedHTTPClient")) + vt := pkg.NewViewsTracker(db) // Setup and launch http server ctx, stop := context.WithCancel(context.Background()) @@ -72,6 +73,7 @@ func main() { Authorizer: az, HTTPClient: hc, OCIPuller: &oci.Puller{}, + ViewsTracker: vt, } h, err := handlers.Setup(ctx, cfg, hSvc) if err != nil { @@ -101,8 +103,12 @@ func main() { } }() - // Setup and launch events dispatcher + // Launch views tracker flusher var wg sync.WaitGroup + wg.Add(1) + go vt.Flusher(ctx, &wg) + + // Setup and launch events dispatcher eSvc := &event.Services{ DB: db, EventManager: event.NewManager(), diff --git a/database/migrations/functions/001_load_functions.sql b/database/migrations/functions/001_load_functions.sql index 6a3f2923d..a8ec7394a 100644 --- a/database/migrations/functions/001_load_functions.sql +++ b/database/migrations/functions/001_load_functions.sql @@ -52,8 +52,9 @@ {{ template "packages/semver_gt.sql" }} {{ template "packages/semver_gte.sql" }} {{ template "packages/toggle_star.sql" }} -{{ template "packages/update_snapshot_security_report.sql" }} {{ template "packages/unregister_package.sql" }} +{{ template "packages/update_packages_views.sql" }} +{{ template "packages/update_snapshot_security_report.sql" }} {{ template "repositories/add_repository.sql" }} {{ template "repositories/delete_repository.sql" }} diff --git a/database/migrations/functions/packages/update_packages_views.sql b/database/migrations/functions/packages/update_packages_views.sql new file mode 100644 index 000000000..2299a2aff --- /dev/null +++ b/database/migrations/functions/packages/update_packages_views.sql @@ -0,0 +1,17 @@ +-- update_packages_views updates the views of the packages provided. +create or replace function update_packages_views(p_lock_key bigint, p_data jsonb) +returns void as $$ + -- Make sure only one batch of updates is processed at a time + select pg_advisory_xact_lock(p_lock_key); + + -- Insert or update the corresponding views counters as needed + insert into package_views (package_id, version, day, total) + select + (value->>0)::uuid as package_id, + (value->>1)::text as version, + (value->>2)::date as day, + (value->>3)::integer as total + from jsonb_array_elements(p_data) + on conflict (package_id, version, day) do + update set total = package_views.total + excluded.total; +$$ language sql; diff --git a/database/migrations/schema/032_packages_views.sql b/database/migrations/schema/032_packages_views.sql new file mode 100644 index 000000000..ac18cddfe --- /dev/null +++ b/database/migrations/schema/032_packages_views.sql @@ -0,0 +1,17 @@ +create extension pg_partman; + +create table if not exists package_views ( + package_id uuid not null references package on delete cascade, + version text not null, + day date not null, + total integer not null, + unique (package_id, version, day) +) partition by range (day); + +select create_parent('public.package_views', 'day', 'native', 'monthly', p_start_partition := current_date::text); + +---- create above / drop below ---- + +drop table if exists package_views; +drop table template_public_package_views; +delete from part_config where parent_table = 'public.package_views'; diff --git a/database/tests/Dockerfile-postgres-pgtap b/database/tests/Dockerfile-postgres similarity index 51% rename from database/tests/Dockerfile-postgres-pgtap rename to database/tests/Dockerfile-postgres index d31d79930..9ded8d719 100644 --- a/database/tests/Dockerfile-postgres-pgtap +++ b/database/tests/Dockerfile-postgres @@ -1,10 +1,13 @@ -# Build pgtap extension +# Build extensions FROM postgres:13 AS builder RUN apt-get update RUN apt-get install -y git make postgresql-server-dev-13 RUN git clone https://github.com/theory/pgtap -RUN cd pgtap && make && make install +RUN cd pgtap && make && make install && cd .. +RUN git clone https://github.com/pgpartman/pg_partman +RUN cd pg_partman && make NO_BGW=1 install # Build final image FROM postgres:13 COPY --from=builder /usr/share/postgresql/13/extension/pgtap* /usr/share/postgresql/13/extension/ +COPY --from=builder /usr/share/postgresql/13/extension/pg_partman* /usr/share/postgresql/13/extension/ diff --git a/database/tests/functions/packages/update_packages_views.sql b/database/tests/functions/packages/update_packages_views.sql new file mode 100644 index 000000000..a89a4203e --- /dev/null +++ b/database/tests/functions/packages/update_packages_views.sql @@ -0,0 +1,81 @@ +-- Start transaction and plan tests +begin; +select plan(3); + +-- Declare some variables +\set lockKey 1 +\set org1ID '00000000-0000-0000-0000-000000000001' +\set repo1ID '00000000-0000-0000-0000-000000000001' +\set package1ID '00000000-0000-0000-0000-000000000001' +\set package2ID '00000000-0000-0000-0000-000000000002' + +-- Seed some data +insert into organization (organization_id, name, display_name, description, home_url) +values (:'org1ID', 'org1', 'Organization 1', 'Description 1', 'https://org1.com'); +insert into repository (repository_id, name, display_name, url, repository_kind_id, organization_id) +values (:'repo1ID', 'repo1', 'Repo 1', 'https://repo1.com', 0, :'org1ID'); +insert into package ( + package_id, + name, + latest_version, + repository_id +) values ( + :'package1ID', + 'pkg1', + '1.0.0', + :'repo1ID' +); +insert into package ( + package_id, + name, + latest_version, + repository_id +) values ( + :'package2ID', + 'pkg2', + '1.0.0', + :'repo1ID' +); + +-- Run some tests +select update_packages_views(:lockKey, '[ + ["00000000-0000-0000-0000-000000000001", "1.0.0", "2021-12-3", 10] +]'); +select results_eq( + 'select * from package_views', + $$ values + ('00000000-0000-0000-0000-000000000001'::uuid, '1.0.0', '2021-12-3'::date, 10) + $$, + 'First run: one insert' +); +select update_packages_views(:lockKey, '[ + ["00000000-0000-0000-0000-000000000001", "1.0.0", "2021-12-3", 10] +]'); +select results_eq( + 'select * from package_views', + $$ values + ('00000000-0000-0000-0000-000000000001'::uuid, '1.0.0', '2021-12-3'::date, 20) + $$, + 'Second run: one update' +); +select update_packages_views(:lockKey, '[ + ["00000000-0000-0000-0000-000000000001", "1.0.0", "2021-12-5", 10], + ["00000000-0000-0000-0000-000000000001", "1.0.1", "2021-12-5", 10], + ["00000000-0000-0000-0000-000000000002", "1.0.0", "2021-12-5", 10], + ["00000000-0000-0000-0000-000000000002", "1.0.0", "2021-12-6", 5] +]'); +select results_eq( + 'select * from package_views', + $$ values + ('00000000-0000-0000-0000-000000000001'::uuid, '1.0.0', '2021-12-3'::date, 20), + ('00000000-0000-0000-0000-000000000001'::uuid, '1.0.0', '2021-12-5'::date, 10), + ('00000000-0000-0000-0000-000000000001'::uuid, '1.0.1', '2021-12-5'::date, 10), + ('00000000-0000-0000-0000-000000000002'::uuid, '1.0.0', '2021-12-5'::date, 10), + ('00000000-0000-0000-0000-000000000002'::uuid, '1.0.0', '2021-12-6'::date, 5) + $$, + 'Third run: one update and four inserts' +); + +-- Finish tests and rollback transaction +select * from finish(); +rollback; diff --git a/database/tests/schema/schema.sql b/database/tests/schema/schema.sql index eb387380f..f7ff04d99 100644 --- a/database/tests/schema/schema.sql +++ b/database/tests/schema/schema.sql @@ -1,6 +1,6 @@ -- Start transaction and plan tests begin; -select plan(153); +select plan(184); -- Check default_text_search_config is correct select results_eq( @@ -13,38 +13,38 @@ select results_eq( select has_extension('pgcrypto'); select has_extension('pg_trgm'); select has_extension('tsm_system_rows'); +select has_extension('pg_partman'); -- Check expected tables exist -select tables_are(array[ - 'api_key', - 'delete_user_code', - 'email_verification_code', - 'event', - 'event_kind', - 'image', - 'image_version', - 'maintainer', - 'notification', - 'opt_out', - 'organization', - 'package', - 'package__maintainer', - 'password_reset_code', - 'production_usage', - 'repository', - 'repository_kind', - 'session', - 'snapshot', - 'subscription', - 'user', - 'user_starred_package', - 'user__organization', - 'version_functions', - 'version_schema', - 'webhook', - 'webhook__event_kind', - 'webhook__package' -]); +select has_table('api_key'); +select has_table('delete_user_code'); +select has_table('email_verification_code'); +select has_table('event'); +select has_table('event_kind'); +select has_table('image'); +select has_table('image_version'); +select has_table('maintainer'); +select has_table('notification'); +select has_table('opt_out'); +select has_table('organization'); +select has_table('package'); +select has_table('package_views'); +select has_table('package__maintainer'); +select has_table('password_reset_code'); +select has_table('production_usage'); +select has_table('repository'); +select has_table('repository_kind'); +select has_table('session'); +select has_table('snapshot'); +select has_table('subscription'); +select has_table('user'); +select has_table('user_starred_package'); +select has_table('user__organization'); +select has_table('version_functions'); +select has_table('version_schema'); +select has_table('webhook'); +select has_table('webhook__event_kind'); +select has_table('webhook__package'); -- Check tables have expected columns select columns_are('api_key', array[ @@ -140,6 +140,12 @@ select columns_are('package', array[ 'created_at', 'repository_id' ]); +select columns_are('package_views', array[ + 'package_id', + 'version', + 'day', + 'total' +]); select columns_are('package__maintainer', array[ 'package_id', 'maintainer_id' @@ -330,6 +336,9 @@ select indexes_are('package', array[ 'package_repository_id_idx', 'package_repository_id_name_key' ]); +select indexes_are('package_views', array[ + 'package_views_package_id_version_day_key' +]); select indexes_are('package__maintainer', array[ 'package__maintainer_pkey' ]); diff --git a/docs/dev.md b/docs/dev.md index 47a457867..83de1e09d 100644 --- a/docs/dev.md +++ b/docs/dev.md @@ -8,7 +8,7 @@ To start, please clone the [Artifact Hub repository](https://github.com/artifact ## Database -The datastore used by Artifact Hub is PostgreSQL. You can install it locally using your favorite OS package manager. +The datastore used by Artifact Hub is PostgreSQL. You can install it locally using your favorite OS package manager. The [pg_partman](https://github.com/pgpartman/pg_partman) extension must be installed as well. Once PostgreSQL is installed and its binaries are available in your `PATH`, we can initialize the database cluster and launch the database server: diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 07feb5370..9a4e7af7e 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -50,6 +50,7 @@ type Services struct { Authorizer hub.Authorizer HTTPClient hub.HTTPClient OCIPuller hub.OCIPuller + ViewsTracker hub.ViewsTracker } // Metrics groups some metrics collected from a Handlers instance. @@ -92,7 +93,14 @@ func Setup(ctx context.Context, cfg *viper.Viper, svc *Services) (*Handlers, err Organizations: org.NewHandlers(svc.OrganizationManager, svc.Authorizer, cfg), Users: userHandlers, Repositories: repo.NewHandlers(cfg, svc.RepositoryManager), - Packages: pkg.NewHandlers(svc.PackageManager, svc.RepositoryManager, cfg, svc.HTTPClient, svc.OCIPuller), + Packages: pkg.NewHandlers( + svc.PackageManager, + svc.RepositoryManager, + cfg, + svc.HTTPClient, + svc.OCIPuller, + svc.ViewsTracker, + ), Subscriptions: subscription.NewHandlers(svc.SubscriptionManager), Webhooks: webhook.NewHandlers(svc.WebhookManager, svc.HTTPClient), APIKeys: apikey.NewHandlers(svc.APIKeyManager), @@ -267,6 +275,7 @@ func (h *Handlers) setupRouter() { r.Get("/{packageID}/{version}/values", h.Packages.GetChartValues) r.Get("/{packageID}/{version}/values-schema", h.Packages.GetValuesSchema) r.Get("/{packageID}/{version}/templates", h.Packages.GetChartTemplates) + r.Post("/{packageID}/{version}/views", h.Packages.TrackView) r.Get("/{packageID}/changelog", h.Packages.GetChangelog) }) diff --git a/internal/handlers/pkg/handlers.go b/internal/handlers/pkg/handlers.go index a788c7148..a291ce6fb 100644 --- a/internal/handlers/pkg/handlers.go +++ b/internal/handlers/pkg/handlers.go @@ -37,6 +37,7 @@ type Handlers struct { logger zerolog.Logger hc hub.HTTPClient op hub.OCIPuller + vt hub.ViewsTracker tmplChangelogMD *template.Template } @@ -47,6 +48,7 @@ func NewHandlers( cfg *viper.Viper, hc hub.HTTPClient, op hub.OCIPuller, + vt hub.ViewsTracker, ) *Handlers { return &Handlers{ pkgManager: pkgManager, @@ -55,6 +57,7 @@ func NewHandlers( logger: log.With().Str("handlers", "pkg").Logger(), hc: hc, op: op, + vt: vt, tmplChangelogMD: setupChangelogMDTmpl(), } } @@ -532,6 +535,17 @@ func (h *Handlers) ToggleStar(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } +// TrackView is an http handler used to track a view of a given package version. +func (h *Handlers) TrackView(w http.ResponseWriter, r *http.Request) { + packageID := chi.URLParam(r, "packageID") + version := chi.URLParam(r, "version") + if err := h.vt.TrackView(packageID, version); err != nil { + helpers.RenderErrorJSON(w, err) + return + } + w.WriteHeader(http.StatusNoContent) +} + // getChartArchive is a helper function used to download a chart's archive from // the original source. func (h *Handlers) getChartArchive(ctx context.Context, packageID, version string) (*chart.Chart, error) { diff --git a/internal/handlers/pkg/handlers_test.go b/internal/handlers/pkg/handlers_test.go index afdde850f..f6c49757c 100644 --- a/internal/handlers/pkg/handlers_test.go +++ b/internal/handlers/pkg/handlers_test.go @@ -1538,6 +1538,49 @@ func TestToggleStar(t *testing.T) { }) } +func TestTrackView(t *testing.T) { + packageID := "00000000-0000-0000-0000-000000000001" + version := "1.0.0" + rctx := &chi.Context{ + URLParams: chi.RouteParams{ + Keys: []string{"packageID", "version"}, + Values: []string{packageID, version}, + }, + } + + t.Run("track view succeeded", func(t *testing.T) { + t.Parallel() + w := httptest.NewRecorder() + r, _ := http.NewRequest("POST", "/", nil) + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) + + hw := newHandlersWrapper() + hw.vt.On("TrackView", packageID, version).Return(nil) + hw.h.TrackView(w, r) + resp := w.Result() + defer resp.Body.Close() + + assert.Equal(t, http.StatusNoContent, resp.StatusCode) + hw.assertExpectations(t) + }) + + t.Run("error tracking view", func(t *testing.T) { + t.Parallel() + w := httptest.NewRecorder() + r, _ := http.NewRequest("POST", "/", nil) + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) + + hw := newHandlersWrapper() + hw.vt.On("TrackView", packageID, version).Return(hub.ErrInvalidInput) + hw.h.TrackView(w, r) + resp := w.Result() + defer resp.Body.Close() + + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + hw.assertExpectations(t) + }) +} + func TestGetChartArchive(t *testing.T) { ctx := context.Background() packageID := "packageID" @@ -1918,6 +1961,7 @@ type handlersWrapper struct { rm *repo.ManagerMock hc *tests.HTTPClientMock op *oci.PullerMock + vt *pkg.ViewsTrackerMock h *Handlers } @@ -1928,13 +1972,15 @@ func newHandlersWrapper() *handlersWrapper { rm := &repo.ManagerMock{} hc := &tests.HTTPClientMock{} op := &oci.PullerMock{} + vt := &pkg.ViewsTrackerMock{} return &handlersWrapper{ pm: pm, rm: rm, hc: hc, op: op, - h: NewHandlers(pm, rm, cfg, hc, op), + vt: vt, + h: NewHandlers(pm, rm, cfg, hc, op, vt), } } diff --git a/internal/hub/pkg.go b/internal/hub/pkg.go index 2e460b1ad..91e124ffb 100644 --- a/internal/hub/pkg.go +++ b/internal/hub/pkg.go @@ -262,3 +262,9 @@ type VersionChanges struct { ContainsSecurityUpdates bool `json:"contains_security_updates"` Prerelease bool `json:"prerelease"` } + +// ViewsTracker describes the methods a ViewsTracker implementation must +// provide. +type ViewsTracker interface { + TrackView(packageID, version string) error +} diff --git a/internal/pkg/mock.go b/internal/pkg/mock.go index f0bc2a0fd..f5e0b67e3 100644 --- a/internal/pkg/mock.go +++ b/internal/pkg/mock.go @@ -159,3 +159,14 @@ func (m *ManagerMock) Unregister(ctx context.Context, pkg *hub.Package) error { args := m.Called(ctx, pkg) return args.Error(0) } + +// ViewsTrackerMock is a mock implementation of the ViewsTracker interface. +type ViewsTrackerMock struct { + mock.Mock +} + +// TrackView implements the ViewsTracker interface. +func (m *ViewsTrackerMock) TrackView(packageID, version string) error { + args := m.Called(packageID, version) + return args.Error(0) +} diff --git a/internal/pkg/views.go b/internal/pkg/views.go new file mode 100644 index 000000000..a108c6241 --- /dev/null +++ b/internal/pkg/views.go @@ -0,0 +1,146 @@ +package pkg + +import ( + "context" + "encoding/json" + "fmt" + "sort" + "strings" + "sync" + "time" + + "github.com/Masterminds/semver/v3" + "github.com/artifacthub/hub/internal/hub" + "github.com/artifacthub/hub/internal/util" + "github.com/rs/zerolog/log" + "github.com/satori/uuid" +) + +const ( + // Database queries + updatePackagesViewsDBQ = `select update_packages_views($1::bigint, $2::jsonb)` + + // defaultFlushFrequency represents how often packages views will be + // written to the database. + defaultFlushFrequency = 5 * time.Minute + + // sep is the separator used in the total map keys. + sep = "##" +) + +// ViewsTracker aggregates packages views that are periodically flushed to the +// database. +type ViewsTracker struct { + db hub.DB + flushFrequency time.Duration + + mu sync.Mutex + total map[string]int +} + +// New creates a new ViewsTracker instance. +func NewViewsTracker(db hub.DB, opts ...func(t *ViewsTracker)) *ViewsTracker { + t := &ViewsTracker{ + db: db, + flushFrequency: defaultFlushFrequency, + total: make(map[string]int), + } + for _, o := range opts { + o(t) + } + return t +} + +// WithFlushFrequency allows configuring the views tracker flush frequency. +func WithFlushFrequency(d time.Duration) func(t *ViewsTracker) { + return func(t *ViewsTracker) { + t.flushFrequency = d + } +} + +// Flusher handles the periodic flushes of packages views. It'll keep running +// until the context provided is done. +func (t *ViewsTracker) Flusher(ctx context.Context, wg *sync.WaitGroup) { + defer wg.Done() + + doFlush := func() { + t.mu.Lock() + if len(t.total) == 0 { + t.mu.Unlock() + return + } + t.mu.Unlock() + if err := t.flush(); err != nil { + log.Error().Err(err).Msg("error flushing packages views") + } + } + for { + select { + case <-time.After(t.flushFrequency): + doFlush() + case <-ctx.Done(): + doFlush() + return + } + } +} + +// TrackView tracks a single view for a given package version. +func (t *ViewsTracker) TrackView(packageID, version string) error { + // Validate input + if _, err := uuid.FromString(packageID); err != nil { + return fmt.Errorf("%w: invalid package id", hub.ErrInvalidInput) + } + if _, err := semver.StrictNewVersion(version); err != nil { + return fmt.Errorf("%w: invalid version", hub.ErrInvalidInput) + } + + // Track view + key := buildTrackerKey(packageID, version) + t.mu.Lock() + t.total[key]++ + t.mu.Unlock() + + return nil +} + +// flush writes the aggregated packages views to the database. +func (t *ViewsTracker) flush() error { + // Prepare data for database update + t.mu.Lock() + keys := make([]string, 0, len(t.total)) + for k := range t.total { + keys = append(keys, k) + } + sort.Strings(keys) + data := make([][]interface{}, 0, len(t.total)) + for _, key := range keys { + packageID, version, date := parseTrackerKey(key) + data = append(data, []interface{}{packageID, version, date, t.total[key]}) + } + t.total = make(map[string]int) // TODO(tegioz): if the db write fails, data would be lost + t.mu.Unlock() + dataJSON, _ := json.Marshal(data) + + // Write data to database + _, err := t.db.Exec( + context.Background(), + updatePackagesViewsDBQ, + util.DBLockKeyUpdatePackagesViews, + dataJSON, + ) + return err +} + +// buildTrackerKey creates a key used to track the views for a given package +// version. +func buildTrackerKey(packageID, version string) string { + return fmt.Sprintf("%s%s%s%s%s", packageID, sep, version, sep, time.Now().Format("2006-01-02")) +} + +// parseTrackerKey parses a key used to track the views for a given package +// version, returning the package id, version and date. +func parseTrackerKey(key string) (string, string, string) { + parts := strings.Split(key, sep) + return parts[0], parts[1], parts[2] +} diff --git a/internal/pkg/views_test.go b/internal/pkg/views_test.go new file mode 100644 index 000000000..64e70d6b3 --- /dev/null +++ b/internal/pkg/views_test.go @@ -0,0 +1,121 @@ +package pkg + +import ( + "context" + "fmt" + "os" + "sync" + "testing" + "time" + + "github.com/artifacthub/hub/internal/tests" + "github.com/artifacthub/hub/internal/util" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" +) + +func TestMain(m *testing.M) { + zerolog.SetGlobalLevel(zerolog.Disabled) + os.Exit(m.Run()) +} + +func TestViewsTracker(t *testing.T) { + package1ID := "00000000-0000-0000-0000-000000000001" + package2ID := "00000000-0000-0000-0000-000000000002" + version := "1.0.0" + + t.Run("invalid input for TrackView", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + + vt := NewViewsTracker(db) + assert.Error(t, vt.TrackView("invalid", version)) + assert.Error(t, vt.TrackView(package1ID, "invalid")) + }) + + t.Run("custom flushing frequency", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + + vt := NewViewsTracker(db, WithFlushFrequency(2*time.Second)) + assert.NotNil(t, vt) + assert.Equal(t, 2*time.Second, vt.flushFrequency) + }) + + t.Run("nothing to flush", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + ctx, cancel := context.WithCancel(context.Background()) + var wg sync.WaitGroup + + vt := NewViewsTracker(db) + wg.Add(1) + go vt.Flusher(ctx, &wg) + cancel() + wg.Wait() + db.AssertExpectations(t) + }) + + t.Run("ctx cancelled, flush and stop", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + day := time.Now().Format("2006-01-02") + db.On("Exec", context.Background(), updatePackagesViewsDBQ, + util.DBLockKeyUpdatePackagesViews, + []byte(fmt.Sprintf(`[["00000000-0000-0000-0000-000000000001","1.0.0","%s",1]]`, day)), + ).Return(nil) + ctx, cancel := context.WithCancel(context.Background()) + var wg sync.WaitGroup + + vt := NewViewsTracker(db) + wg.Add(1) + go vt.Flusher(ctx, &wg) + assert.Nil(t, vt.TrackView(package1ID, version)) + cancel() + wg.Wait() + db.AssertExpectations(t) + }) + + t.Run("db error flushing", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + day := time.Now().Format("2006-01-02") + db.On("Exec", context.Background(), updatePackagesViewsDBQ, + util.DBLockKeyUpdatePackagesViews, + []byte(fmt.Sprintf(`[["00000000-0000-0000-0000-000000000001","1.0.0","%s",1]]`, day)), + ).Return(tests.ErrFakeDB) + ctx, cancel := context.WithCancel(context.Background()) + var wg sync.WaitGroup + + vt := NewViewsTracker(db) + wg.Add(1) + go vt.Flusher(ctx, &wg) + assert.Nil(t, vt.TrackView(package1ID, version)) + cancel() + wg.Wait() + db.AssertExpectations(t) + }) + + t.Run("some package views flushed by timer successfully", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + day := time.Now().Format("2006-01-02") + db.On("Exec", context.Background(), updatePackagesViewsDBQ, + util.DBLockKeyUpdatePackagesViews, + []byte(fmt.Sprintf(`[["00000000-0000-0000-0000-000000000001","1.0.0","%s",2],["00000000-0000-0000-0000-000000000002","1.0.0","%s",1]]`, day, day)), + ).Return(nil) + ctx, cancel := context.WithCancel(context.Background()) + var wg sync.WaitGroup + + vt := NewViewsTracker(db, WithFlushFrequency(1*time.Second)) + wg.Add(1) + go vt.Flusher(ctx, &wg) + assert.Nil(t, vt.TrackView(package1ID, version)) + assert.Nil(t, vt.TrackView(package1ID, version)) + assert.Nil(t, vt.TrackView(package2ID, version)) + time.Sleep(1500 * time.Millisecond) + db.AssertExpectations(t) + cancel() + wg.Wait() + }) +} diff --git a/internal/util/db.go b/internal/util/db.go index 2db510bd8..4ef31fcf9 100644 --- a/internal/util/db.go +++ b/internal/util/db.go @@ -15,6 +15,12 @@ import ( "github.com/spf13/viper" ) +const ( + // DBLockKeyUpdatePackagesViews represents the lock key used when updating + // the packages views counters in the database. + DBLockKeyUpdatePackagesViews = 1 +) + var ( // ErrDBInsufficientPrivilege indicates that the user does not have the // required privilege to perform the operation. diff --git a/web/src/api/index.test.tsx b/web/src/api/index.test.tsx index f6021d6d0..2ebfe27f2 100644 --- a/web/src/api/index.test.tsx +++ b/web/src/api/index.test.tsx @@ -1854,9 +1854,27 @@ describe('API', () => { }); }); + describe('trackView', () => { + it('success', async () => { + fetchMock.mockResponse('', { + headers: { + 'content-type': 'text/plain; charset=utf-8', + }, + status: 204, + }); + + const response = await API.trackView('pkgID', '1.0.0'); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock.mock.calls[0][0]).toEqual('/api/v1/packages/pkgID/1.0.0/views'); + expect(fetchMock.mock.calls[0][1]!.method).toBe('POST'); + expect(response).toBe(''); + }); + }); + describe('getChangelog', () => { it('success', async () => { - const changelog: ChangeLog[] = getData('43') as Changelog[]; + const changelog: ChangeLog[] = getData('43') as ChangeLog[]; fetchMock.mockResponse(JSON.stringify(changelog), { headers: { 'content-type': 'application/json', diff --git a/web/src/api/index.ts b/web/src/api/index.ts index 4d9469d95..98cb284f5 100644 --- a/web/src/api/index.ts +++ b/web/src/api/index.ts @@ -915,6 +915,15 @@ class API_CLASS { }); } + public trackView(packageId: string, version: string): Promise { + return this.apiFetch({ + url: `${this.API_BASE_URL}/packages/${packageId}/${version}/views`, + opts: { + method: 'POST', + }, + }); + } + public getChangelog(packageId: string): Promise { return this.apiFetch({ url: `${this.API_BASE_URL}/packages/${packageId}/changelog` }); } diff --git a/web/src/layout/package/__fixtures__/index/19.json b/web/src/layout/package/__fixtures__/index/19.json new file mode 100644 index 000000000..e713a17e6 --- /dev/null +++ b/web/src/layout/package/__fixtures__/index/19.json @@ -0,0 +1,22 @@ +{ + "packageId": "id", + "name": "test", + "version": "1.0.0", + "displayName": "Pretty name", + "description": "desc", + "logoImageId": "imageId", + "appVersion": "1.0.0", + "normalizedName": "pr", + "deprecated": false, + "isOperator": false, + "keywords": ["key1", "key2"], + "repository": { + "repositoryid": "id", + "kind": 0, + "name": "incubator", + "organizationName": "helm", + "organizationDisplayName": "Helm", + "url": "https://repo.url", + "private": false + } +} diff --git a/web/src/layout/package/index.test.tsx b/web/src/layout/package/index.test.tsx index 5f66bb8fa..7df0c25cb 100644 --- a/web/src/layout/package/index.test.tsx +++ b/web/src/layout/package/index.test.tsx @@ -72,6 +72,7 @@ describe('Package index', () => { it('renders component', async () => { const mockPackage = getMockPackage('2'); mocked(API).getPackage.mockResolvedValue(mockPackage); + mocked(API).trackView.mockResolvedValue(null); render( @@ -84,6 +85,23 @@ describe('Package index', () => { }); }); + it('calls track view', async () => { + const mockPackage = getMockPackage('19'); + mocked(API).getPackage.mockResolvedValue(mockPackage); + + render( + + + + ); + + await waitFor(() => { + expect(API.getPackage).toHaveBeenCalledTimes(1); + expect(API.trackView).toHaveBeenCalledTimes(1); + expect(API.trackView).toHaveBeenCalledWith('id', '1.0.0'); + }); + }); + it('displays loading spinner', async () => { const mockPackage = getMockPackage('3'); mocked(API).getPackage.mockResolvedValue(mockPackage); diff --git a/web/src/layout/package/index.tsx b/web/src/layout/package/index.tsx index 0d45740f6..1f4ffcc13 100644 --- a/web/src/layout/package/index.tsx +++ b/web/src/layout/package/index.tsx @@ -174,6 +174,14 @@ const PackageView = (props: Props) => { } }, [currentPkgId]); /* eslint-disable-line react-hooks/exhaustive-deps */ + async function trackView(pkgID: string, version: string) { + try { + API.trackView(pkgID, version); + } catch { + // Do not do anything + } + } + async function fetchPackageDetail() { try { setRelatedPosition(null); @@ -188,6 +196,8 @@ const PackageView = (props: Props) => { }/${detailPkg.repository.name}`; updateMetaIndex(metaTitle, detailPkg.description); setDetail(detailPkg); + // Track view + trackView(detailPkg.packageId, detailPkg.version!); if (currentHash) { setCurrentHash(undefined); }