diff --git a/.gitattributes b/.gitattributes index f81833b940..5f60152faf 100644 --- a/.gitattributes +++ b/.gitattributes @@ -31,6 +31,7 @@ catalog/pkg/openapi/model_catalog_source_list.go linguist-generated=true catalog/pkg/openapi/model_catalog_source_preview_response.go linguist-generated=true catalog/pkg/openapi/model_catalog_source_preview_response_all_of_summary.go linguist-generated=true catalog/pkg/openapi/model_error.go linguist-generated=true +catalog/pkg/openapi/model_field_filter.go linguist-generated=true catalog/pkg/openapi/model_filter_option.go linguist-generated=true catalog/pkg/openapi/model_filter_option_range.go linguist-generated=true catalog/pkg/openapi/model_filter_options_list.go linguist-generated=true diff --git a/.github/workflows/async-upload-test.yml b/.github/workflows/async-upload-test.yml index 53b16e8285..3a7fc0833e 100644 --- a/.github/workflows/async-upload-test.yml +++ b/.github/workflows/async-upload-test.yml @@ -39,7 +39,7 @@ jobs: run: working-directory: jobs/async-upload steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Set up Python uses: actions/setup-python@v6 with: @@ -66,7 +66,7 @@ jobs: run: working-directory: jobs/async-upload steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Set up Python uses: actions/setup-python@v6 with: @@ -89,7 +89,7 @@ jobs: working-directory: jobs/async-upload steps: - name: Check out the repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 with: fetch-depth: 0 - name: Set up Python diff --git a/.github/workflows/build-and-push-async-upload.yml b/.github/workflows/build-and-push-async-upload.yml index 9868b289f9..dcbb201a8a 100644 --- a/.github/workflows/build-and-push-async-upload.yml +++ b/.github/workflows/build-and-push-async-upload.yml @@ -31,7 +31,7 @@ jobs: packages: write steps: - name: Checkout repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 diff --git a/.github/workflows/build-and-push-csi-image.yml b/.github/workflows/build-and-push-csi-image.yml index 07f76db02c..93b37bd60a 100644 --- a/.github/workflows/build-and-push-csi-image.yml +++ b/.github/workflows/build-and-push-csi-image.yml @@ -38,7 +38,7 @@ jobs: if: github.head_ref == '' && github.ref == 'refs/heads/main' run: echo "BUILD_CONTEXT=main" >> $GITHUB_ENV # checkout branch - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 # set image version - name: Set main-branch environment if: env.BUILD_CONTEXT == 'main' diff --git a/.github/workflows/build-and-push-image.yml b/.github/workflows/build-and-push-image.yml index ea2fc5cc47..dc69d88220 100644 --- a/.github/workflows/build-and-push-image.yml +++ b/.github/workflows/build-and-push-image.yml @@ -42,7 +42,7 @@ jobs: if: github.head_ref == '' && github.ref == 'refs/heads/main' run: echo "BUILD_CONTEXT=main" >> $GITHUB_ENV # checkout branch - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 # Set up QEMU for multi-architecture builds - name: Set up QEMU uses: docker/setup-qemu-action@v3 diff --git a/.github/workflows/build-and-push-ui-images-standalone.yml b/.github/workflows/build-and-push-ui-images-standalone.yml index b7c2ee90da..0f12751470 100644 --- a/.github/workflows/build-and-push-ui-images-standalone.yml +++ b/.github/workflows/build-and-push-ui-images-standalone.yml @@ -29,7 +29,7 @@ jobs: packages: write steps: - name: Checkout repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 diff --git a/.github/workflows/build-and-push-ui-images.yml b/.github/workflows/build-and-push-ui-images.yml index 76ede95a35..ba1d53a547 100644 --- a/.github/workflows/build-and-push-ui-images.yml +++ b/.github/workflows/build-and-push-ui-images.yml @@ -29,7 +29,7 @@ jobs: packages: write steps: - name: Checkout repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 diff --git a/.github/workflows/build-image-pr.yml b/.github/workflows/build-image-pr.yml index fa5806d388..cd36286822 100644 --- a/.github/workflows/build-image-pr.yml +++ b/.github/workflows/build-image-pr.yml @@ -25,7 +25,7 @@ jobs: build-and-test-image: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Generate Tag shell: bash id: tags diff --git a/.github/workflows/build-image-ui-pr.yml b/.github/workflows/build-image-ui-pr.yml index a76e54ebce..7a091b7142 100644 --- a/.github/workflows/build-image-ui-pr.yml +++ b/.github/workflows/build-image-ui-pr.yml @@ -27,7 +27,7 @@ jobs: runs-on: ubuntu-latest steps: # checkout branch - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Build UI Image shell: bash run: ./scripts/build_deploy.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f2d23a045b..24024d1b61 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -25,7 +25,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Setup Go uses: actions/setup-go@v6 with: diff --git a/.github/workflows/check-db-schema-structs.yaml b/.github/workflows/check-db-schema-structs.yaml index 056755b7d2..c64cfe9cf6 100644 --- a/.github/workflows/check-db-schema-structs.yaml +++ b/.github/workflows/check-db-schema-structs.yaml @@ -14,7 +14,7 @@ jobs: check-mysql-schema-structs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Setup Go uses: actions/setup-go@v6 with: @@ -34,7 +34,7 @@ jobs: check-postgres-schema-structs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Setup Go uses: actions/setup-go@v6 with: diff --git a/.github/workflows/check-gitattributes.yaml b/.github/workflows/check-gitattributes.yaml index edbcafbb3d..0010146de7 100644 --- a/.github/workflows/check-gitattributes.yaml +++ b/.github/workflows/check-gitattributes.yaml @@ -8,6 +8,6 @@ jobs: validate: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Validate OpenAPI spec run: ./scripts/gen_gitattributes.sh --check diff --git a/.github/workflows/check-openapi-spec-pr.yaml b/.github/workflows/check-openapi-spec-pr.yaml index cd96a78057..8bc44ab1cf 100644 --- a/.github/workflows/check-openapi-spec-pr.yaml +++ b/.github/workflows/check-openapi-spec-pr.yaml @@ -12,7 +12,7 @@ jobs: validate: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Validate OpenAPI spec run: | make openapi/validate diff --git a/.github/workflows/controller-test.yml b/.github/workflows/controller-test.yml index f0ecb23237..f745ac236e 100644 --- a/.github/workflows/controller-test.yml +++ b/.github/workflows/controller-test.yml @@ -39,7 +39,7 @@ jobs: echo "tag=${tag}" >> $GITHUB_OUTPUT - name: Clone the code - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Setup Go uses: actions/setup-go@v6 diff --git a/.github/workflows/csi-test.yml b/.github/workflows/csi-test.yml index 652f39413b..64e1713af7 100644 --- a/.github/workflows/csi-test.yml +++ b/.github/workflows/csi-test.yml @@ -35,7 +35,7 @@ jobs: build-and-test-csi-image: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Generate tag shell: bash diff --git a/.github/workflows/first-time-contributor-pr.yml b/.github/workflows/first-time-contributor-pr.yml index 7c0729cfff..fe35abf385 100644 --- a/.github/workflows/first-time-contributor-pr.yml +++ b/.github/workflows/first-time-contributor-pr.yml @@ -15,14 +15,14 @@ permissions: # set contents: read at top-level, per OpenSSF ScoreCard rule Token jobs: welcome: - if: ${{ github.actor != 'dependabot[bot]' }} + # do NOT skip the entire workflow as that generates troubles with Prow/Tide, i.e. do not do `if` checks here runs-on: ubuntu-latest permissions: pull-requests: write issues: write steps: - name: Checkout repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Install PyYAML run: pip3 install pyyaml @@ -50,6 +50,7 @@ jobs: echo "Approvers: ${{ steps.set-approvers.outputs.approvers }}" - name: Welcome first-time contributors message + if: ${{ github.actor != 'dependabot[bot]' }} uses: actions/first-interaction@v3 continue-on-error: true with: diff --git a/.github/workflows/go-mod-tidy-diff-check.yml b/.github/workflows/go-mod-tidy-diff-check.yml index 50f8bc9b9b..f71fc5751f 100644 --- a/.github/workflows/go-mod-tidy-diff-check.yml +++ b/.github/workflows/go-mod-tidy-diff-check.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Set up Go uses: actions/setup-go@v6 diff --git a/.github/workflows/prepare.yml b/.github/workflows/prepare.yml index e862b841d6..9d1aff409e 100644 --- a/.github/workflows/prepare.yml +++ b/.github/workflows/prepare.yml @@ -8,7 +8,7 @@ jobs: prepare: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Setup Go uses: actions/setup-go@v6 with: diff --git a/.github/workflows/python-release.yml b/.github/workflows/python-release.yml index 13fc10106b..f003536a31 100644 --- a/.github/workflows/python-release.yml +++ b/.github/workflows/python-release.yml @@ -16,7 +16,7 @@ jobs: FORCE_COLOR: "1" steps: - name: Check out the repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 with: fetch-depth: 0 - name: Set up Python diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 8661f6f8e3..e450445dcb 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -31,7 +31,7 @@ jobs: FORCE_COLOR: "1" steps: - name: Check out the repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Set up Python ${{ matrix.python }} uses: actions/setup-python@v6 with: @@ -74,7 +74,7 @@ jobs: nodejs: ["20"] steps: - name: Check out the repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 with: fetch-depth: 0 - name: Set up Python @@ -148,7 +148,7 @@ jobs: DEPLOY_MANIFEST_DB: "${{ matrix.manifest-db }}" steps: - name: Check out the repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Set up Python ${{ matrix.python }} uses: actions/setup-python@v6 with: @@ -268,7 +268,7 @@ jobs: FORCE_COLOR: "1" steps: - name: Check out the repository - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Set up Python ${{ matrix.python }} uses: actions/setup-python@v6 with: diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index 3b3a5550c7..2c1e458703 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -31,7 +31,7 @@ jobs: steps: - name: "Checkout code" - uses: actions/checkout@v6.0.0 # unify for Dependabot bump + uses: actions/checkout@v6.0.1 # unify for Dependabot bump with: persist-credentials: false diff --git a/.github/workflows/test-fuzz.yml b/.github/workflows/test-fuzz.yml index 7d18ff4b34..503240a6d0 100644 --- a/.github/workflows/test-fuzz.yml +++ b/.github/workflows/test-fuzz.yml @@ -53,7 +53,7 @@ jobs: } - name: Checkout PR - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 with: ref: ${{ fromJson(steps.pr.outputs.result).sha }} diff --git a/.github/workflows/trivy-image-scanning.yaml b/.github/workflows/trivy-image-scanning.yaml index 373c453c03..96ea403a0f 100644 --- a/.github/workflows/trivy-image-scanning.yaml +++ b/.github/workflows/trivy-image-scanning.yaml @@ -24,7 +24,7 @@ jobs: ] steps: - name: Checkout code - uses: actions/checkout@v6.0.0 + uses: actions/checkout@v6.0.1 - name: Sanitize image name for SARIF filename run: | diff --git a/.github/workflows/ui-bff-build.yml b/.github/workflows/ui-bff-build.yml index 1d5bd4cda0..0c6ca62e61 100644 --- a/.github/workflows/ui-bff-build.yml +++ b/.github/workflows/ui-bff-build.yml @@ -26,7 +26,7 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Setup Go uses: actions/setup-go@v6 diff --git a/.github/workflows/ui-frontend-build.yml b/.github/workflows/ui-frontend-build.yml index 3875175cd4..f68b2bfce8 100644 --- a/.github/workflows/ui-frontend-build.yml +++ b/.github/workflows/ui-frontend-build.yml @@ -26,7 +26,7 @@ jobs: test-and-build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6.0.0 + - uses: actions/checkout@v6.0.1 - name: Set up Node.js uses: actions/setup-node@v6 diff --git a/api/openapi/catalog.yaml b/api/openapi/catalog.yaml index fac5211c6e..0b9db1e69c 100644 --- a/api/openapi/catalog.yaml +++ b/api/openapi/catalog.yaml @@ -831,6 +831,19 @@ components: message: description: Error message type: string + FieldFilter: + type: object + required: + - operator + - value + properties: + operator: + type: string + description: Filter operator (e.g., '<', '=', '>', 'IN') + example: '<' + value: + description: Filter value (can be number, string, or array) + example: 70 FilterOption: type: object required: @@ -867,6 +880,13 @@ components: description: A single filter option. additionalProperties: $ref: "#/components/schemas/FilterOption" + namedQueries: + type: object + description: Predefined named queries for common filtering scenarios + additionalProperties: + type: object + additionalProperties: + $ref: "#/components/schemas/FieldFilter" MetadataBoolValue: description: A bool property value. type: object diff --git a/api/openapi/src/catalog.yaml b/api/openapi/src/catalog.yaml index e538e9ffd7..e36764494b 100644 --- a/api/openapi/src/catalog.yaml +++ b/api/openapi/src/catalog.yaml @@ -746,6 +746,19 @@ components: max: type: number format: double + FieldFilter: + type: object + required: + - operator + - value + properties: + operator: + type: string + description: Filter operator (e.g., '<', '=', '>', 'IN') + example: '<' + value: + description: Filter value (can be number, string, or array) + example: 70 FilterOptionsList: description: List of FilterOptions type: object @@ -755,6 +768,13 @@ components: description: A single filter option. additionalProperties: $ref: "#/components/schemas/FilterOption" + namedQueries: + type: object + description: Predefined named queries for common filtering scenarios + additionalProperties: + type: object + additionalProperties: + $ref: "#/components/schemas/FieldFilter" OrderByField: description: |- Supported fields for ordering result entities. diff --git a/catalog/internal/catalog/catalog_test.go b/catalog/internal/catalog/catalog_test.go index 0b63fc1074..ac5cc17bc9 100644 --- a/catalog/internal/catalog/catalog_test.go +++ b/catalog/internal/catalog/catalog_test.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "sort" + "sync" "testing" "time" @@ -376,6 +377,9 @@ func TestLoadCatalogSourcesWithMockRepositories(t *testing.T) { // Wait a bit for the goroutine to process time.Sleep(100 * time.Millisecond) + mockModelRepo.mu.RLock() + defer mockModelRepo.mu.RUnlock() + // Verify that the model was saved if len(mockModelRepo.SavedModels) != 1 { t.Errorf("Expected 1 model to be saved, got %d", len(mockModelRepo.SavedModels)) @@ -390,11 +394,17 @@ func TestLoadCatalogSourcesWithMockRepositories(t *testing.T) { } } + mockModelArtifactRepo.mu.RLock() + defer mockModelArtifactRepo.mu.RUnlock() + // Verify that artifacts were saved if len(mockModelArtifactRepo.SavedArtifacts) != 1 { t.Errorf("Expected 1 model artifact to be saved, got %d", len(mockModelArtifactRepo.SavedArtifacts)) } + mockMetricsArtifactRepo.mu.RLock() + defer mockMetricsArtifactRepo.mu.RUnlock() + if len(mockMetricsArtifactRepo.SavedMetrics) != 1 { t.Errorf("Expected 1 metrics artifact to be saved, got %d", len(mockMetricsArtifactRepo.SavedMetrics)) } @@ -470,6 +480,9 @@ func TestLoadCatalogSourcesWithRepositoryErrors(t *testing.T) { // Wait for processing time.Sleep(100 * time.Millisecond) + mockModelRepo.mu.RLock() + defer mockModelRepo.mu.RUnlock() + // Verify that no models were saved due to the error if len(mockModelRepo.SavedModels) != 0 { t.Errorf("Expected 0 models to be saved due to error, got %d", len(mockModelRepo.SavedModels)) @@ -638,11 +651,14 @@ func (m *MockCatalogModelRepositoryWithErrors) Save(model dbmodels.CatalogModel) // MockCatalogModelRepository mocks the CatalogModelRepository interface. type MockCatalogModelRepository struct { + mu sync.RWMutex SavedModels []dbmodels.CatalogModel NextID int32 } func (m *MockCatalogModelRepository) GetByID(id int32) (dbmodels.CatalogModel, error) { + m.mu.RLock() + defer m.mu.RUnlock() for _, model := range m.SavedModels { if model.GetID() != nil && *model.GetID() == id { return model, nil @@ -652,6 +668,8 @@ func (m *MockCatalogModelRepository) GetByID(id int32) (dbmodels.CatalogModel, e } func (m *MockCatalogModelRepository) List(listOptions dbmodels.CatalogModelListOptions) (*mrmodels.ListWrapper[dbmodels.CatalogModel], error) { + m.mu.RLock() + defer m.mu.RUnlock() return &mrmodels.ListWrapper[dbmodels.CatalogModel]{ Items: m.SavedModels, PageSize: int32(len(m.SavedModels)), @@ -660,6 +678,8 @@ func (m *MockCatalogModelRepository) List(listOptions dbmodels.CatalogModelListO } func (m *MockCatalogModelRepository) GetByName(name string) (dbmodels.CatalogModel, error) { + m.mu.RLock() + defer m.mu.RUnlock() for _, model := range m.SavedModels { if model.GetAttributes() != nil && model.GetAttributes().Name != nil && *model.GetAttributes().Name == name { return model, nil @@ -669,6 +689,9 @@ func (m *MockCatalogModelRepository) GetByName(name string) (dbmodels.CatalogMod } func (m *MockCatalogModelRepository) Save(model dbmodels.CatalogModel) (dbmodels.CatalogModel, error) { + m.mu.Lock() + defer m.mu.Unlock() + m.NextID++ id := m.NextID @@ -685,13 +708,31 @@ func (m *MockCatalogModelRepository) Save(model dbmodels.CatalogModel) (dbmodels return savedModel, nil } +func (m *MockCatalogModelRepository) DeleteBySource(sourceID string) error { + // Mock implementation - no-op for testing + return nil +} + +func (m *MockCatalogModelRepository) DeleteByID(id int32) error { + // Mock implementation - no-op for testing + return nil +} + +func (m *MockCatalogModelRepository) GetDistinctSourceIDs() ([]string, error) { + // Mock implementation - return empty list by default + return []string{}, nil +} + // MockCatalogModelArtifactRepository mocks the CatalogModelArtifactRepository interface. type MockCatalogModelArtifactRepository struct { + mu sync.RWMutex SavedArtifacts []dbmodels.CatalogModelArtifact NextID int32 } func (m *MockCatalogModelArtifactRepository) GetByID(id int32) (dbmodels.CatalogModelArtifact, error) { + m.mu.RLock() + defer m.mu.RUnlock() for _, artifact := range m.SavedArtifacts { if artifact.GetID() != nil && *artifact.GetID() == id { return artifact, nil @@ -701,6 +742,8 @@ func (m *MockCatalogModelArtifactRepository) GetByID(id int32) (dbmodels.Catalog } func (m *MockCatalogModelArtifactRepository) List(listOptions dbmodels.CatalogModelArtifactListOptions) (*mrmodels.ListWrapper[dbmodels.CatalogModelArtifact], error) { + m.mu.RLock() + defer m.mu.RUnlock() return &mrmodels.ListWrapper[dbmodels.CatalogModelArtifact]{ Items: m.SavedArtifacts, PageSize: int32(len(m.SavedArtifacts)), @@ -709,6 +752,9 @@ func (m *MockCatalogModelArtifactRepository) List(listOptions dbmodels.CatalogMo } func (m *MockCatalogModelArtifactRepository) Save(modelArtifact dbmodels.CatalogModelArtifact, parentResourceID *int32) (dbmodels.CatalogModelArtifact, error) { + m.mu.Lock() + defer m.mu.Unlock() + m.NextID++ id := m.NextID @@ -727,11 +773,15 @@ func (m *MockCatalogModelArtifactRepository) Save(modelArtifact dbmodels.Catalog // MockCatalogMetricsArtifactRepository mocks the CatalogMetricsArtifactRepository interface. type MockCatalogMetricsArtifactRepository struct { + mu sync.RWMutex SavedMetrics []dbmodels.CatalogMetricsArtifact NextID int32 } func (m *MockCatalogMetricsArtifactRepository) GetByID(id int32) (dbmodels.CatalogMetricsArtifact, error) { + m.mu.RLock() + defer m.mu.RUnlock() + for _, metrics := range m.SavedMetrics { if metrics.GetID() != nil && *metrics.GetID() == id { return metrics, nil @@ -741,6 +791,9 @@ func (m *MockCatalogMetricsArtifactRepository) GetByID(id int32) (dbmodels.Catal } func (m *MockCatalogMetricsArtifactRepository) List(listOptions dbmodels.CatalogMetricsArtifactListOptions) (*mrmodels.ListWrapper[dbmodels.CatalogMetricsArtifact], error) { + m.mu.RLock() + defer m.mu.RUnlock() + return &mrmodels.ListWrapper[dbmodels.CatalogMetricsArtifact]{ Items: m.SavedMetrics, PageSize: int32(len(m.SavedMetrics)), @@ -749,6 +802,9 @@ func (m *MockCatalogMetricsArtifactRepository) List(listOptions dbmodels.Catalog } func (m *MockCatalogMetricsArtifactRepository) Save(metricsArtifact dbmodels.CatalogMetricsArtifact, parentResourceID *int32) (dbmodels.CatalogMetricsArtifact, error) { + m.mu.Lock() + defer m.mu.Unlock() + m.NextID++ id := m.NextID @@ -766,6 +822,9 @@ func (m *MockCatalogMetricsArtifactRepository) Save(metricsArtifact dbmodels.Cat } func (m *MockCatalogMetricsArtifactRepository) BatchSave(metricsArtifacts []dbmodels.CatalogMetricsArtifact, parentResourceID *int32) ([]dbmodels.CatalogMetricsArtifact, error) { + m.mu.Lock() + defer m.mu.Unlock() + savedArtifacts := make([]dbmodels.CatalogMetricsArtifact, len(metricsArtifacts)) for i, metricsArtifact := range metricsArtifacts { @@ -790,11 +849,14 @@ func (m *MockCatalogMetricsArtifactRepository) BatchSave(metricsArtifacts []dbmo // MockCatalogArtifactRepository mocks the CatalogArtifactRepository interface. type MockCatalogArtifactRepository struct { + mu sync.RWMutex SavedArtifacts []dbmodels.CatalogArtifact NextID int32 } func (m *MockCatalogArtifactRepository) GetByID(id int32) (dbmodels.CatalogArtifact, error) { + m.mu.RLock() + defer m.mu.RUnlock() for _, artifact := range m.SavedArtifacts { // Check both model and metrics artifacts for the ID if artifact.CatalogModelArtifact != nil && artifact.CatalogModelArtifact.GetID() != nil && *artifact.CatalogModelArtifact.GetID() == id { @@ -808,6 +870,8 @@ func (m *MockCatalogArtifactRepository) GetByID(id int32) (dbmodels.CatalogArtif } func (m *MockCatalogArtifactRepository) List(listOptions dbmodels.CatalogArtifactListOptions) (*mrmodels.ListWrapper[dbmodels.CatalogArtifact], error) { + m.mu.RLock() + defer m.mu.RUnlock() return &mrmodels.ListWrapper[dbmodels.CatalogArtifact]{ Items: m.SavedArtifacts, PageSize: int32(len(m.SavedArtifacts)), diff --git a/catalog/internal/catalog/db_catalog.go b/catalog/internal/catalog/db_catalog.go index 8e714a4209..d53e896b61 100644 --- a/catalog/internal/catalog/db_catalog.go +++ b/catalog/internal/catalog/db_catalog.go @@ -237,8 +237,32 @@ func (d *dbCatalogImpl) GetFilterOptions(ctx context.Context) (*apimodels.Filter } } + // Get named queries from sources configuration + var namedQueriesPtr *map[string]map[string]apimodels.FieldFilter + if d.sources != nil { + namedQueriesMap := d.sources.GetNamedQueries() + + // Convert internal FieldFilter to API FieldFilter + apiNamedQueries := make(map[string]map[string]apimodels.FieldFilter, len(namedQueriesMap)) + for queryName, fieldFilters := range namedQueriesMap { + apiFieldFilters := make(map[string]apimodels.FieldFilter, len(fieldFilters)) + for fieldName, filter := range fieldFilters { + apiFieldFilters[fieldName] = apimodels.FieldFilter{ + Operator: filter.Operator, + Value: filter.Value, + } + } + apiNamedQueries[queryName] = apiFieldFilters + } + + if len(apiNamedQueries) > 0 { + namedQueriesPtr = &apiNamedQueries + } + } + return &apimodels.FilterOptionsList{ - Filters: &options, + Filters: &options, + NamedQueries: namedQueriesPtr, }, nil } diff --git a/catalog/internal/catalog/db_catalog_test.go b/catalog/internal/catalog/db_catalog_test.go index d5885f2c85..818ed93e52 100644 --- a/catalog/internal/catalog/db_catalog_test.go +++ b/catalog/internal/catalog/db_catalog_test.go @@ -1506,6 +1506,54 @@ func TestDBCatalog_GetPerformanceArtifactsWithService(t *testing.T) { assert.Equal(t, "3", replicas.MetadataIntValue.IntValue) } +func TestGetFilterOptionsWithNamedQueries(t *testing.T) { + // Setup mock sources with named queries + sources := NewSourceCollection() + namedQueries := map[string]map[string]FieldFilter{ + "validation-default": { + "ttft_p90": {Operator: "<", Value: 70}, + "workload_type": {Operator: "=", Value: "Chat"}, + }, + "high-performance": { + "performance_score": {Operator: ">", Value: 0.95}, + }, + } + + err := sources.MergeWithNamedQueries("test", map[string]Source{}, namedQueries) + require.NoError(t, err) + + // Create catalog with mocked dependencies + mockServices := service.Services{ + PropertyOptionsRepository: &mockPropertyRepository{}, + } + catalog := NewDBCatalog(mockServices, sources) + + // Test GetFilterOptions includes named queries + result, err := catalog.GetFilterOptions(context.Background()) + require.NoError(t, err) + require.NotNil(t, result.NamedQueries) + + queries := *result.NamedQueries + assert.Len(t, queries, 2) + + validationQuery := queries["validation-default"] + assert.Equal(t, "<", validationQuery["ttft_p90"].Operator) + assert.Equal(t, 70, validationQuery["ttft_p90"].Value) + assert.Equal(t, "=", validationQuery["workload_type"].Operator) + assert.Equal(t, "Chat", validationQuery["workload_type"].Value) +} + +// Mock repository for testing +type mockPropertyRepository struct{} + +func (m *mockPropertyRepository) List(optionType models.PropertyOptionType, limit int32) ([]models.PropertyOption, error) { + return []models.PropertyOption{}, nil +} + +func (m *mockPropertyRepository) Refresh(optionType models.PropertyOptionType) error { + return nil +} + func getCatalogModelArtifactTypeIDForDBTest(t *testing.T, db *gorm.DB) int32 { var typeRecord schema.Type err := db.Where("name = ?", service.CatalogModelArtifactTypeName).First(&typeRecord).Error diff --git a/catalog/internal/catalog/hf_catalog.go b/catalog/internal/catalog/hf_catalog.go index 21aa0a4a0f..7ffcfd3b58 100644 --- a/catalog/internal/catalog/hf_catalog.go +++ b/catalog/internal/catalog/hf_catalog.go @@ -577,6 +577,12 @@ func (p *hfModelProvider) emit(ctx context.Context, models []ModelProviderRecord return } } + + // Send an empty record to indicate that we're done with the batch. + select { + case out <- ModelProviderRecord{}: + case <-done: + } } // validateCredentials checks if the HuggingFace API key credentials are valid diff --git a/catalog/internal/catalog/integration_test.go b/catalog/internal/catalog/integration_test.go new file mode 100644 index 0000000000..2569ff0280 --- /dev/null +++ b/catalog/internal/catalog/integration_test.go @@ -0,0 +1,106 @@ +package catalog + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/kubeflow/model-registry/catalog/internal/db/service" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNamedQueriesEndToEnd(t *testing.T) { + // Create temporary YAML file with named queries + tempDir, err := os.MkdirTemp("", "named-queries-test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + yamlContent := ` +catalogs: [] +labels: [] +namedQueries: + validation-default: + ttft_p90: + operator: '<' + value: 70 + workload_type: + operator: '=' + value: "Chat" + high-performance: + performance_score: + operator: '>' + value: 0.95 + tpot_mean: + operator: '<' + value: 30 +` + + yamlPath := filepath.Join(tempDir, "test-sources.yaml") + err = os.WriteFile(yamlPath, []byte(yamlContent), 0644) + require.NoError(t, err) + + // Test configuration loading + loader := NewLoader(service.Services{}, []string{yamlPath}) + err = loader.parseAndMerge(yamlPath) + require.NoError(t, err) + + // Verify named queries are loaded + namedQueries := loader.Sources.GetNamedQueries() + assert.Len(t, namedQueries, 2) + + // Test validation-default query + validationQuery := namedQueries["validation-default"] + assert.Equal(t, "<", validationQuery["ttft_p90"].Operator) + assert.Equal(t, float64(70), validationQuery["ttft_p90"].Value) + assert.Equal(t, "=", validationQuery["workload_type"].Operator) + assert.Equal(t, "Chat", validationQuery["workload_type"].Value) + + // Test high-performance query + perfQuery := namedQueries["high-performance"] + assert.Equal(t, ">", perfQuery["performance_score"].Operator) + assert.Equal(t, float64(0.95), perfQuery["performance_score"].Value) + assert.Equal(t, "<", perfQuery["tpot_mean"].Operator) + assert.Equal(t, float64(30), perfQuery["tpot_mean"].Value) + + // Test API response includes named queries + mockServices := service.Services{ + PropertyOptionsRepository: &mockPropertyRepository{}, + } + catalog := NewDBCatalog(mockServices, loader.Sources) + + filterOptions, err := catalog.GetFilterOptions(context.Background()) + require.NoError(t, err) + require.NotNil(t, filterOptions.NamedQueries) + + apiQueries := *filterOptions.NamedQueries + assert.Len(t, apiQueries, 2) + assert.Contains(t, apiQueries, "validation-default") + assert.Contains(t, apiQueries, "high-performance") +} + +func TestNamedQueriesValidationErrors(t *testing.T) { + tempDir, err := os.MkdirTemp("", "named-queries-validation-test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + // Test invalid operator + invalidYaml := ` +catalogs: [] +namedQueries: + bad-query: + field1: + operator: 'INVALID_OP' + value: 100 +` + + yamlPath := filepath.Join(tempDir, "invalid-sources.yaml") + err = os.WriteFile(yamlPath, []byte(invalidYaml), 0644) + require.NoError(t, err) + + loader := NewLoader(service.Services{}, []string{yamlPath}) + err = loader.parseAndMerge(yamlPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unsupported operator 'INVALID_OP'") +} diff --git a/catalog/internal/catalog/loader.go b/catalog/internal/catalog/loader.go index ca4a8f3281..b21e926bef 100644 --- a/catalog/internal/catalog/loader.go +++ b/catalog/internal/catalog/loader.go @@ -8,6 +8,7 @@ import ( "path/filepath" "sync" + mapset "github.com/deckarep/golang-set/v2" "github.com/golang/glog" dbmodels "github.com/kubeflow/model-registry/catalog/internal/db/models" "github.com/kubeflow/model-registry/catalog/internal/db/service" @@ -26,6 +27,9 @@ type ModelProviderRecord struct { // expected to spawn a goroutine and return immediately. The returned channel must // close when the goroutine ends. The goroutine should end when the context is // canceled, but may end sooner. +// +// The function may emit a record with a nil Model to indicate that the +// complete set of models has been sent. type ModelProviderFunc func(ctx context.Context, source *Source, reldir string) (<-chan ModelProviderRecord, error) var registeredModelProviders = map[string]ModelProviderFunc{} @@ -41,10 +45,17 @@ func RegisterModelProvider(name string, callback ModelProviderFunc) error { // LoaderEventHandler is the definition of a function called after a model is loaded. type LoaderEventHandler func(ctx context.Context, record ModelProviderRecord) error +// FieldFilter represents a single field filter within a named query +type FieldFilter struct { + Operator string `json:"operator" yaml:"operator"` + Value any `json:"value" yaml:"value"` +} + // sourceConfig is the structure for the catalog sources YAML file. type sourceConfig struct { - Catalogs []Source `json:"catalogs"` - Labels []map[string]any `json:"labels,omitempty"` + Catalogs []Source `json:"catalogs"` + Labels []map[string]any `json:"labels,omitempty"` + NamedQueries map[string]map[string]FieldFilter `json:"namedQueries,omitempty" yaml:"namedQueries,omitempty"` } // Source is a single entry from the catalog sources YAML file. @@ -120,8 +131,14 @@ func (l *Loader) Start(ctx context.Context) error { } } + // Delete models from unknown or disabled sources + err := l.removeModelsFromMissingSources() + if err != nil { + return fmt.Errorf("faied to remove models from missing sources: %w", err) + } + // Phase 2: Load models from merged sources (once, after all merging is complete) - err := l.loadAllModels(ctx) + err = l.loadAllModels(ctx) if err != nil { return err } @@ -199,6 +216,13 @@ func (l *Loader) read(path string) (*sourceConfig, error) { return nil, err } + // Validate named queries if present + if config.NamedQueries != nil { + if err := ValidateNamedQueries(config.NamedQueries); err != nil { + return nil, fmt.Errorf("invalid named queries in %s: %w", path, err) + } + } + // Note: We intentionally do NOT filter disabled sources or apply defaults here. // This allows field-level merging in SourceCollection to work correctly: // - A base source with enabled=false can be enabled by a user override with just id + enabled=true @@ -232,6 +256,10 @@ func (l *Loader) updateSources(path string, config *sourceConfig) error { glog.Infof("loaded source %s of type %s", id, source.Type) } + // Use MergeWithNamedQueries if named queries exist, otherwise use regular Merge + if config.NamedQueries != nil { + return l.Sources.MergeWithNamedQueries(path, sources, config.NamedQueries) + } return l.Sources.Merge(path, sources) } @@ -269,6 +297,9 @@ func (l *Loader) updateDatabase(ctx context.Context) error { go func() { for record := range records { + if record.Model == nil { + continue + } attr := record.Model.GetAttributes() if attr == nil || attr.Name == nil { continue @@ -377,7 +408,30 @@ func (l *Loader) readProviderRecords(ctx context.Context) <-chan ModelProviderRe wg.Add(1) go func() { defer wg.Done() + + modelNames := []string{} + for r := range records { + if r.Model == nil { + glog.V(2).Infof("%s: trigger cleanup", source.Id) + + // Copy the list of model names, then clear it. + modelNameSet := mapset.NewSet(modelNames...) + modelNames = modelNames[:0] + + go func() { + err := l.removeOrphanedModelsFromSource(source.Id, modelNameSet) + if err != nil { + glog.Errorf("error removing orphaned models: %v", err) + } + }() + continue + } + + if attr := r.Model.GetAttributes(); attr != nil && attr.Name != nil { + modelNames = append(modelNames, *attr.Name) + } + // Set source_id on every returned model. l.setModelSourceID(r.Model, source.Id) @@ -424,3 +478,57 @@ func (l *Loader) setModelSourceID(model dbmodels.CatalogModel, sourceID string) *props = append(*props, mrmodels.NewStringProperty("source_id", sourceID, false)) } + +func (l *Loader) removeModelsFromMissingSources() error { + enabledSourceIDs := mapset.NewSet[string]() + for id, source := range l.Sources.AllSources() { + if source.Enabled == nil || *source.Enabled { + enabledSourceIDs.Add(id) + } + } + + existingSourceIDs, err := l.services.CatalogModelRepository.GetDistinctSourceIDs() + if err != nil { + return fmt.Errorf("unable to retrieve existing source IDs: %w", err) + } + + for oldSource := range mapset.NewSet(existingSourceIDs...).Difference(enabledSourceIDs).Iter() { + glog.Infof("Removing models from source %s", oldSource) + + err = l.services.CatalogModelRepository.DeleteBySource(oldSource) + if err != nil { + return fmt.Errorf("unable to remove models from source %q: %w", oldSource, err) + } + } + + return nil +} + +func (l *Loader) removeOrphanedModelsFromSource(sourceID string, valid mapset.Set[string]) error { + list, err := l.services.CatalogModelRepository.List(dbmodels.CatalogModelListOptions{ + SourceIDs: &[]string{sourceID}, + }) + if err != nil { + return fmt.Errorf("unable to list models from source %q: %w", sourceID, err) + } + + for _, model := range list.Items { + attr := model.GetAttributes() + if attr == nil || attr.Name == nil || model.GetID() == nil { + continue + } + + if valid.Contains(*attr.Name) { + continue + } + + glog.Infof("Removing %s model %s", sourceID, *attr.Name) + + err = l.services.CatalogModelRepository.DeleteByID(*model.GetID()) + if err != nil { + return fmt.Errorf("unable to remove model %d (%s from source %s): %w", *model.GetID(), *attr.Name, sourceID, err) + } + } + + return nil +} diff --git a/catalog/internal/catalog/loader_test.go b/catalog/internal/catalog/loader_test.go new file mode 100644 index 0000000000..4e0c823699 --- /dev/null +++ b/catalog/internal/catalog/loader_test.go @@ -0,0 +1,226 @@ +package catalog + +import ( + "testing" + + mapset "github.com/deckarep/golang-set/v2" + "github.com/kubeflow/model-registry/catalog/internal/db/service" + apimodels "github.com/kubeflow/model-registry/catalog/pkg/openapi" + "github.com/kubeflow/model-registry/internal/apiutils" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/yaml" +) + +func TestRemoveModelsFromMissingSources(t *testing.T) { + // This test verifies the current behavior of removeModelsFromMissingSources. + // The method removes models from sources that are either: + // 1. Not present in the current configuration + // 2. Explicitly disabled (Enabled = false) + // Note: Sources with nil Enabled are filtered out by AllSources() - see bug comment below + + tests := []struct { + name string + enabledSources map[string]*bool // source ID -> enabled status (nil means default true) + existingSourceIDs []string // source IDs currently in database + expectedDeletedSources []string // source IDs that should be deleted + repositoryError string // if set, repository returns this error + expectError bool + }{ + { + name: "removes models from sources not in config", + enabledSources: map[string]*bool{ + "source1": apiutils.Of(true), + "source2": apiutils.Of(true), + }, + existingSourceIDs: []string{"source1", "source2", "source3", "source4"}, + expectedDeletedSources: []string{"source3", "source4"}, // source3 and source4 not in config + }, + { + name: "no deletion when all database sources are in config", + enabledSources: map[string]*bool{ + "source1": apiutils.Of(true), + "source2": apiutils.Of(true), + "source3": apiutils.Of(true), + }, + existingSourceIDs: []string{"source1", "source2"}, + expectedDeletedSources: []string{}, // no deletions - all database sources are in config + }, + { + name: "handles empty existing sources", + enabledSources: map[string]*bool{ + "source1": apiutils.Of(true), + }, + existingSourceIDs: []string{}, + expectedDeletedSources: []string{}, // no deletions needed - no existing sources + }, + { + name: "handles empty config sources", + enabledSources: map[string]*bool{}, // no sources in config + existingSourceIDs: []string{"source1", "source2"}, + expectedDeletedSources: []string{"source1", "source2"}, // all existing sources deleted + }, + { + name: "correctly handles default enabled sources", + enabledSources: map[string]*bool{ + "source1": nil, // default enabled - converted to true by applyDefaults + "source2": apiutils.Of(true), // explicitly enabled + }, + existingSourceIDs: []string{"source1", "source2", "source3"}, + expectedDeletedSources: []string{"source3"}, // only source3 (not in config) gets deleted + }, + { + name: "handles repository error on GetDistinctSourceIDs", + enabledSources: map[string]*bool{ + "source1": apiutils.Of(true), + }, + existingSourceIDs: []string{"source1"}, + repositoryError: "get_distinct_source_ids_error", + expectError: true, + }, + { + name: "handles repository error on DeleteBySource", + enabledSources: map[string]*bool{ + "source1": apiutils.Of(true), + }, + existingSourceIDs: []string{"source1", "source2"}, + repositoryError: "delete_by_source_error", + expectedDeletedSources: []string{"source2"}, // source2 should be attempted for deletion + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create mock repository with test data + mockModelRepo := &MockCatalogModelRepositoryWithSourceTracking{ + ExistingSourceIDs: tt.existingSourceIDs, + DeletedSources: []string{}, + ErrorType: tt.repositoryError, + } + + services := service.NewServices( + mockModelRepo, + &MockCatalogArtifactRepository{}, + &MockCatalogModelArtifactRepository{}, + &MockCatalogMetricsArtifactRepository{}, + &MockPropertyOptionsRepository{}, + ) + + // Create loader and populate sources + loader := NewLoader(services, []string{}) + + // Add all test sources to the loader's source collection in a single Merge call + sourcesMap := make(map[string]Source) + for sourceID, enabled := range tt.enabledSources { + source := apimodels.CatalogSource{ + Id: sourceID, + Name: "Test " + sourceID, + Enabled: enabled, + } + + sourcesMap[sourceID] = Source{ + CatalogSource: source, + Type: "test", + } + } + + if len(sourcesMap) > 0 { + err := loader.Sources.Merge("test-path", sourcesMap) + if err != nil { + t.Fatalf("Failed to add sources: %v", err) + } + } + + // Call the method under test + err := loader.removeModelsFromMissingSources() + + // Verify error expectation + if tt.expectError { + if err == nil { + t.Errorf("Expected error but got none") + } + return + } + if err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + // Verify which sources were deleted + expectedSet := mapset.NewSet(tt.expectedDeletedSources...) + actualSet := mapset.NewSet(mockModelRepo.DeletedSources...) + + if !expectedSet.Equal(actualSet) { + t.Errorf("Expected deleted sources %v, got %v", + tt.expectedDeletedSources, mockModelRepo.DeletedSources) + } + }) + } +} + +// MockCatalogModelRepositoryWithSourceTracking extends the existing mock to add source tracking +type MockCatalogModelRepositoryWithSourceTracking struct { + MockCatalogModelRepository + ExistingSourceIDs []string + DeletedSources []string + ErrorType string // "get_distinct_source_ids_error" or "delete_by_source_error" +} + +func (m *MockCatalogModelRepositoryWithSourceTracking) GetDistinctSourceIDs() ([]string, error) { + if m.ErrorType == "get_distinct_source_ids_error" { + return nil, NewMockError("failed to get distinct source IDs") + } + return m.ExistingSourceIDs, nil +} + +func (m *MockCatalogModelRepositoryWithSourceTracking) DeleteBySource(sourceID string) error { + if m.ErrorType == "delete_by_source_error" { + return NewMockError("failed to delete models from source: " + sourceID) + } + m.DeletedSources = append(m.DeletedSources, sourceID) + return nil +} + +// Helper function to create mock errors +func NewMockError(message string) error { + return &MockRepositoryError{Message: message} +} + +type MockRepositoryError struct { + Message string +} + +func (e *MockRepositoryError) Error() string { + return e.Message + +} + +func TestSourceConfigNamedQueries(t *testing.T) { + yamlContent := ` +catalogs: [] +namedQueries: + validation-default: + ttft_p90: + operator: '<' + value: 70 + workload_type: + operator: '=' + value: "Chat" + high-performance: + performance_score: + operator: '>' + value: 0.95 +` + var config sourceConfig + err := yaml.UnmarshalStrict([]byte(yamlContent), &config) + assert.NoError(t, err) + assert.NotNil(t, config.NamedQueries) + assert.Len(t, config.NamedQueries, 2) + + validationQuery := config.NamedQueries["validation-default"] + assert.NotNil(t, validationQuery) + assert.Equal(t, "<", validationQuery["ttft_p90"].Operator) + assert.Equal(t, float64(70), validationQuery["ttft_p90"].Value) + assert.Equal(t, "=", validationQuery["workload_type"].Operator) + assert.Equal(t, "Chat", validationQuery["workload_type"].Value) +} diff --git a/catalog/internal/catalog/sources.go b/catalog/internal/catalog/sources.go index 57ef341ea8..50f21a05ba 100644 --- a/catalog/internal/catalog/sources.go +++ b/catalog/internal/catalog/sources.go @@ -19,8 +19,9 @@ type originEntry struct { // SourceCollection manages catalog sources from multiple origins with priority-based merging. // Later entries in the slice take precedence over earlier ones. type SourceCollection struct { - mu sync.RWMutex - entries []originEntry + mu sync.RWMutex + entries []originEntry + namedQueries map[string]map[string]FieldFilter } // NewSourceCollection creates a new SourceCollection with the given origin order. @@ -32,7 +33,10 @@ func NewSourceCollection(originOrder ...string) *SourceCollection { for i, origin := range originOrder { entries[i] = originEntry{origin: origin, sources: nil} } - return &SourceCollection{entries: entries} + return &SourceCollection{ + entries: entries, + namedQueries: make(map[string]map[string]FieldFilter), + } } // Merge adds sources from one origin (ordinarily, a file path--but any unique @@ -46,7 +50,34 @@ func NewSourceCollection(originOrder ...string) *SourceCollection { func (sc *SourceCollection) Merge(origin string, sources map[string]Source) error { sc.mu.Lock() defer sc.mu.Unlock() + return sc.mergeSourcesInternal(origin, sources) +} + +// MergeWithNamedQueries adds sources and named queries from one origin. +func (sc *SourceCollection) MergeWithNamedQueries(origin string, sources map[string]Source, namedQueries map[string]map[string]FieldFilter) error { + sc.mu.Lock() + defer sc.mu.Unlock() + + // Merge sources using existing logic + if err := sc.mergeSourcesInternal(origin, sources); err != nil { + return err + } + + // Merge named queries (later origins override earlier ones) + for queryName, fieldFilters := range namedQueries { + if sc.namedQueries[queryName] == nil { + sc.namedQueries[queryName] = make(map[string]FieldFilter) + } + for fieldName, filter := range fieldFilters { + sc.namedQueries[queryName][fieldName] = filter + } + } + return nil +} + +// mergeSourcesInternal extracts the internal logic from Merge +func (sc *SourceCollection) mergeSourcesInternal(origin string, sources map[string]Source) error { // Find existing entry for this origin for i := range sc.entries { if sc.entries[i].origin == origin { @@ -60,6 +91,22 @@ func (sc *SourceCollection) Merge(origin string, sources map[string]Source) erro return nil } +// GetNamedQueries returns all merged named queries +func (sc *SourceCollection) GetNamedQueries() map[string]map[string]FieldFilter { + sc.mu.RLock() + defer sc.mu.RUnlock() + + // Return a copy to prevent external modification + result := make(map[string]map[string]FieldFilter, len(sc.namedQueries)) + for queryName, fieldFilters := range sc.namedQueries { + result[queryName] = make(map[string]FieldFilter, len(fieldFilters)) + for fieldName, filter := range fieldFilters { + result[queryName][fieldName] = filter + } + } + return result +} + // mergeSources performs field-level merging of two Source structs. // Fields from 'override' take precedence over 'base' when they are explicitly set. // A field is considered "set" if: diff --git a/catalog/internal/catalog/sources_test.go b/catalog/internal/catalog/sources_test.go index bcbbd43ca5..7ddef7f31c 100644 --- a/catalog/internal/catalog/sources_test.go +++ b/catalog/internal/catalog/sources_test.go @@ -834,9 +834,9 @@ func TestSourceCollection_MergeOverride_Origin(t *testing.T) { // This is important for resolving relative paths in source properties tests := []struct { - name string - originOrder []string - mergeSequence []struct { + name string + originOrder []string + mergeSequence []struct { origin string sources map[string]Source } @@ -1136,3 +1136,32 @@ func TestSourceCollection_ByLabel_NullBehavior(t *testing.T) { }) } } + +func TestSourceCollection_NamedQueries(t *testing.T) { + sc := NewSourceCollection() + + sources := map[string]Source{ + "test1": {CatalogSource: model.CatalogSource{Id: "test1"}}, + } + namedQueries := map[string]map[string]FieldFilter{ + "validation-default": { + "ttft_p90": {Operator: "<", Value: 70}, + }, + } + + err := sc.MergeWithNamedQueries("origin1", sources, namedQueries) + if err != nil { + t.Fatalf("MergeWithNamedQueries failed: %v", err) + } + + queries := sc.GetNamedQueries() + if len(queries) != 1 { + t.Errorf("GetNamedQueries() returned %d queries, want 1", len(queries)) + } + if queries["validation-default"]["ttft_p90"].Operator != "<" { + t.Errorf("Expected operator '<', got '%s'", queries["validation-default"]["ttft_p90"].Operator) + } + if queries["validation-default"]["ttft_p90"].Value != 70 { + t.Errorf("Expected value 70, got %v", queries["validation-default"]["ttft_p90"].Value) + } +} diff --git a/catalog/internal/catalog/validation.go b/catalog/internal/catalog/validation.go new file mode 100644 index 0000000000..53106bf5a9 --- /dev/null +++ b/catalog/internal/catalog/validation.go @@ -0,0 +1,72 @@ +package catalog + +import ( + "fmt" + "strings" +) + +// supportedOperators defines the valid operators for named query field filters +var supportedOperators = map[string]bool{ + "=": true, + "!=": true, + ">": true, + "<": true, + ">=": true, + "<=": true, + "LIKE": true, + "ILIKE": true, + "IN": true, + "NOT IN": true, +} + +// ValidateNamedQueries validates the structure and content of named queries +func ValidateNamedQueries(namedQueries map[string]map[string]FieldFilter) error { + for queryName, fieldFilters := range namedQueries { + if queryName == "" { + return fmt.Errorf("named query name cannot be empty") + } + + if len(fieldFilters) == 0 { + return fmt.Errorf("named query '%s' must contain at least one field filter", queryName) + } + + for fieldName, filter := range fieldFilters { + if fieldName == "" { + return fmt.Errorf("field name cannot be empty in named query '%s'", queryName) + } + + if err := validateFieldFilter(queryName, fieldName, filter); err != nil { + return err + } + } + } + + return nil +} + +// validateFieldFilter validates a single field filter within a named query +func validateFieldFilter(queryName, fieldName string, filter FieldFilter) error { + if filter.Operator == "" { + return fmt.Errorf("operator cannot be empty for field '%s' in named query '%s'", fieldName, queryName) + } + + normalizedOperator := strings.ToUpper(filter.Operator) + if !supportedOperators[normalizedOperator] { + return fmt.Errorf("unsupported operator '%s' for field '%s' in named query '%s'", filter.Operator, fieldName, queryName) + } + + if filter.Value == nil { + return fmt.Errorf("value cannot be nil for field '%s' in named query '%s'", fieldName, queryName) + } + + // Additional validation based on operator type + switch normalizedOperator { + case "IN", "NOT IN": + // Value should be an array + if _, ok := filter.Value.([]any); !ok { + return fmt.Errorf("operator '%s' requires array value for field '%s' in named query '%s'", filter.Operator, fieldName, queryName) + } + } + + return nil +} diff --git a/catalog/internal/catalog/validation_test.go b/catalog/internal/catalog/validation_test.go new file mode 100644 index 0000000000..ca379a08c9 --- /dev/null +++ b/catalog/internal/catalog/validation_test.go @@ -0,0 +1,103 @@ +package catalog + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidateNamedQueries(t *testing.T) { + tests := []struct { + name string + namedQueries map[string]map[string]FieldFilter + expectError bool + errorContains string + }{ + { + name: "valid named queries", + namedQueries: map[string]map[string]FieldFilter{ + "test-query": { + "field1": {Operator: "=", Value: "value"}, + "field2": {Operator: ">", Value: 42}, + }, + }, + expectError: false, + }, + { + name: "invalid operator", + namedQueries: map[string]map[string]FieldFilter{ + "test-query": { + "field1": {Operator: "INVALID", Value: "value"}, + }, + }, + expectError: true, + errorContains: "unsupported operator 'INVALID'", + }, + { + name: "empty operator", + namedQueries: map[string]map[string]FieldFilter{ + "test-query": { + "field1": {Operator: "", Value: "value"}, + }, + }, + expectError: true, + errorContains: "operator cannot be empty", + }, + { + name: "nil value", + namedQueries: map[string]map[string]FieldFilter{ + "test-query": { + "field1": {Operator: "=", Value: nil}, + }, + }, + expectError: true, + errorContains: "value cannot be nil", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateNamedQueries(tt.namedQueries) + if tt.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.errorContains) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestLoaderValidationIntegration(t *testing.T) { + // Test that the validation logic works correctly + // (The loader integration is tested in the main loader tests) + + // Test with a valid config + validConfig := &sourceConfig{ + Catalogs: []Source{}, + NamedQueries: map[string]map[string]FieldFilter{ + "valid-query": { + "field1": {Operator: "=", Value: "value"}, + }, + }, + } + + // This should succeed (we're testing the validation logic, not the file I/O) + err := ValidateNamedQueries(validConfig.NamedQueries) + assert.NoError(t, err) + + // Test with an invalid config + invalidConfig := &sourceConfig{ + Catalogs: []Source{}, + NamedQueries: map[string]map[string]FieldFilter{ + "invalid-query": { + "field1": {Operator: "INVALID_OP", Value: "value"}, + }, + }, + } + + // This should fail + err = ValidateNamedQueries(invalidConfig.NamedQueries) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unsupported operator 'INVALID_OP'") +} diff --git a/catalog/internal/catalog/yaml_catalog.go b/catalog/internal/catalog/yaml_catalog.go index cf1da5223b..ae0b016f65 100644 --- a/catalog/internal/catalog/yaml_catalog.go +++ b/catalog/internal/catalog/yaml_catalog.go @@ -374,6 +374,12 @@ func (p *yamlModelProvider) emit(ctx context.Context, catalog *yamlCatalog, out return } } + + // Send an empty record to indicate that we're done with the batch. + select { + case out <- ModelProviderRecord{}: + case <-done: + } } func newYamlModelProvider(ctx context.Context, source *Source, reldir string) (<-chan ModelProviderRecord, error) { diff --git a/catalog/internal/catalog/yaml_catalog_test.go b/catalog/internal/catalog/yaml_catalog_test.go index 3d7c0590fd..2d4dd75aa1 100644 --- a/catalog/internal/catalog/yaml_catalog_test.go +++ b/catalog/internal/catalog/yaml_catalog_test.go @@ -598,7 +598,9 @@ func collectNamesWithFilter(t *testing.T, catalogPath string, filter *ModelFilte var names []string for record := range out { - names = append(names, modelNameFromRecord(t, record)) + if record.Model != nil { + names = append(names, modelNameFromRecord(t, record)) + } } return names @@ -616,7 +618,9 @@ func collectRecordsFromChannel(t *testing.T, records <-chan ModelProviderRecord, if !ok { t.Fatalf("channel closed before receiving %d records", expected) } - names = append(names, modelNameFromRecord(t, record)) + if record.Model != nil { + names = append(names, modelNameFromRecord(t, record)) + } case <-timeout: t.Fatalf("timed out waiting for %d records", expected) } diff --git a/catalog/internal/db/models/catalog_model.go b/catalog/internal/db/models/catalog_model.go index 1769e7d259..262af093e9 100644 --- a/catalog/internal/db/models/catalog_model.go +++ b/catalog/internal/db/models/catalog_model.go @@ -37,4 +37,7 @@ type CatalogModelRepository interface { GetByName(name string) (CatalogModel, error) List(listOptions CatalogModelListOptions) (*models.ListWrapper[CatalogModel], error) Save(model CatalogModel) (CatalogModel, error) + DeleteBySource(sourceID string) error + DeleteByID(id int32) error + GetDistinctSourceIDs() ([]string, error) } diff --git a/catalog/internal/db/service/catalog_model.go b/catalog/internal/db/service/catalog_model.go index 7be94889ca..cc031fb032 100644 --- a/catalog/internal/db/service/catalog_model.go +++ b/catalog/internal/db/service/catalog_model.go @@ -130,6 +130,76 @@ func (r *CatalogModelRepositoryImpl) lookupModelByName(name string) (*schema.Con return &entity, nil } +func (r *CatalogModelRepositoryImpl) DeleteBySource(sourceID string) error { + config := r.GetConfig() + + // Delete all Context records where there's a ContextProperty with name='source_id' and string_value=sourceID + query := `DELETE FROM "Context" WHERE id IN ( + SELECT "Context".id + FROM "Context" + INNER JOIN "ContextProperty" ON "Context".id="ContextProperty".context_id + AND "ContextProperty".name='source_id' + WHERE "ContextProperty".string_value=? + AND "Context".type_id=? + )` + + return config.DB.Exec(query, sourceID, config.TypeID).Error +} + +func (r *CatalogModelRepositoryImpl) DeleteByID(id int32) error { + config := r.GetConfig() + + // Delete the Context record by ID and type_id + // ContextProperty records will be deleted via foreign key cascade + result := config.DB.Where("id = ? AND type_id = ?", id, config.TypeID).Delete(&schema.Context{}) + + if result.Error != nil { + return result.Error + } + + if result.RowsAffected == 0 { + return fmt.Errorf("%w: id %d", config.NotFoundError, id) + } + + return nil +} + +// GetDistinctSourceIDs retrieves all unique source_id values from catalog models. +// This method queries the ContextProperty table to find distinct string_value entries +// where the property name is 'source_id'. +func (r *CatalogModelRepositoryImpl) GetDistinctSourceIDs() ([]string, error) { + config := r.GetConfig() + + var sourceIDs []string + + // Execute the SQL query to get distinct source_id values + query := `SELECT DISTINCT string_value FROM "ContextProperty" WHERE name='source_id'` + + rows, err := config.DB.Raw(query).Rows() + if err != nil { + // Sanitize database errors to avoid exposing internal details to users + err = dbutil.SanitizeDatabaseError(err) + return nil, fmt.Errorf("error querying distinct source IDs: %w", err) + } + defer rows.Close() + + for rows.Next() { + var sourceID string + if err := rows.Scan(&sourceID); err != nil { + err = dbutil.SanitizeDatabaseError(err) + return nil, fmt.Errorf("error scanning source ID: %w", err) + } + sourceIDs = append(sourceIDs, sourceID) + } + + if err := rows.Err(); err != nil { + err = dbutil.SanitizeDatabaseError(err) + return nil, fmt.Errorf("error iterating source ID rows: %w", err) + } + + return sourceIDs, nil +} + func applyCatalogModelListFilters(query *gorm.DB, listOptions *models.CatalogModelListOptions) *gorm.DB { contextTable := utils.GetTableName(query.Statement.DB, &schema.Context{}) diff --git a/catalog/internal/db/service/catalog_model_test.go b/catalog/internal/db/service/catalog_model_test.go index 80070f6dfd..47c1319cb8 100644 --- a/catalog/internal/db/service/catalog_model_test.go +++ b/catalog/internal/db/service/catalog_model_test.go @@ -1,6 +1,7 @@ package service_test import ( + "errors" "fmt" "strings" "testing" @@ -840,6 +841,162 @@ func TestCatalogModelRepository(t *testing.T) { } } }) + + t.Run("TestDeleteBySource", func(t *testing.T) { + // Setup: Create models with different source IDs + sourceID1 := "test_source_1" + sourceID2 := "test_source_2" + + model1 := &models.CatalogModelImpl{ + Attributes: &models.CatalogModelAttributes{ + Name: apiutils.Of("model-source-1"), + }, + Properties: &[]dbmodels.Properties{ + { + Name: "source_id", + StringValue: &sourceID1, + }, + }, + } + + model2 := &models.CatalogModelImpl{ + Attributes: &models.CatalogModelAttributes{ + Name: apiutils.Of("model-source-1-second"), + }, + Properties: &[]dbmodels.Properties{ + { + Name: "source_id", + StringValue: &sourceID1, + }, + }, + } + + model3 := &models.CatalogModelImpl{ + Attributes: &models.CatalogModelAttributes{ + Name: apiutils.Of("model-source-2"), + }, + Properties: &[]dbmodels.Properties{ + { + Name: "source_id", + StringValue: &sourceID2, + }, + }, + } + + // Save all models + saved1, err := repo.Save(model1) + require.NoError(t, err) + saved2, err := repo.Save(model2) + require.NoError(t, err) + saved3, err := repo.Save(model3) + require.NoError(t, err) + + // Delete by source_id + err = repo.DeleteBySource(sourceID1) + require.NoError(t, err) + + // Verify models from source1 are deleted + _, err = repo.GetByID(*saved1.GetID()) + assert.Error(t, err) + assert.True(t, errors.Is(err, service.ErrCatalogModelNotFound)) + + _, err = repo.GetByID(*saved2.GetID()) + assert.Error(t, err) + assert.True(t, errors.Is(err, service.ErrCatalogModelNotFound)) + + // Verify model from source2 still exists + retrieved, err := repo.GetByID(*saved3.GetID()) + require.NoError(t, err) + assert.Equal(t, "model-source-2", *retrieved.GetAttributes().Name) + }) + + t.Run("TestDeleteByID", func(t *testing.T) { + // Setup: Create a model + model := &models.CatalogModelImpl{ + Attributes: &models.CatalogModelAttributes{ + Name: apiutils.Of("model-to-delete"), + }, + Properties: &[]dbmodels.Properties{ + { + Name: "description", + StringValue: apiutils.Of("Model that will be deleted"), + }, + }, + } + + saved, err := repo.Save(model) + require.NoError(t, err) + + // Delete by ID + err = repo.DeleteByID(*saved.GetID()) + require.NoError(t, err) + + // Verify model is deleted + _, err = repo.GetByID(*saved.GetID()) + assert.Error(t, err) + assert.True(t, errors.Is(err, service.ErrCatalogModelNotFound)) + }) + + t.Run("TestDeleteBySourceNonExistent", func(t *testing.T) { + // Test deleting by non-existent source - should not error + err := repo.DeleteBySource("non-existent-source") + require.NoError(t, err) + }) + + t.Run("TestDeleteByIDNonExistent", func(t *testing.T) { + // Test deleting non-existent ID - should return NotFoundError + err := repo.DeleteByID(999999) + require.Error(t, err) + assert.True(t, errors.Is(err, service.ErrCatalogModelNotFound)) + }) + + t.Run("TestGetDistinctSourceIDs", func(t *testing.T) { + // Get initial count of source IDs + initialSourceIDs, err := repo.GetDistinctSourceIDs() + assert.NoError(t, err) + initialCount := len(initialSourceIDs) + + // Create test data with different source_ids (use unique prefixes to avoid collision with other tests) + testSourceID1 := "test-distinct-source-1" + testSourceID2 := "test-distinct-source-2" + + model1 := createTestCatalogModelWithSourceID(t, testSourceID1) + model2 := createTestCatalogModelWithSourceID(t, testSourceID2) + model3 := createTestCatalogModelWithSourceID(t, testSourceID1) // duplicate + + _, err = repo.Save(model1) + assert.NoError(t, err) + _, err = repo.Save(model2) + assert.NoError(t, err) + _, err = repo.Save(model3) + assert.NoError(t, err) + + // Test distinct source_ids - should have 2 new source IDs added + sourceIDs, err := repo.GetDistinctSourceIDs() + assert.NoError(t, err) + assert.Len(t, sourceIDs, initialCount+2, "Should have exactly 2 new distinct source IDs") + assert.Contains(t, sourceIDs, testSourceID1) + assert.Contains(t, sourceIDs, testSourceID2) + }) +} + +func createTestCatalogModelWithSourceID(t *testing.T, sourceID string) models.CatalogModel { + model := &models.CatalogModelImpl{ + Attributes: &models.CatalogModelAttributes{ + Name: apiutils.Of(fmt.Sprintf("test-model-%s", sourceID)), + }, + } + + // Add source_id as a property + properties := []dbmodels.Properties{ + { + Name: "source_id", + StringValue: &sourceID, + }, + } + model.Properties = &properties + + return model } // Helper function to get or create CatalogModel type ID @@ -872,8 +1029,8 @@ func TestCatalogModelRepository_TimestampPreservation(t *testing.T) { t.Run("Preserve historical timestamps from YAML catalog", func(t *testing.T) { // Simulate loading a model from YAML with historical timestamps - historicalCreateTime := int64(1739776988000) // From YAML example - historicalUpdateTime := int64(1746720264000) // From YAML example + historicalCreateTime := int64(1739776988000) // From YAML example + historicalUpdateTime := int64(1746720264000) // From YAML example catalogModel := &models.CatalogModelImpl{ Attributes: &models.CatalogModelAttributes{ diff --git a/catalog/internal/server/openapi/.openapi-generator/FILES b/catalog/internal/server/openapi/.openapi-generator/FILES index a01668ebbf..37cbf4edba 100644 --- a/catalog/internal/server/openapi/.openapi-generator/FILES +++ b/catalog/internal/server/openapi/.openapi-generator/FILES @@ -22,6 +22,7 @@ model_catalog_source_list.go model_catalog_source_preview_response.go model_catalog_source_preview_response_all_of_summary.go model_error.go +model_field_filter.go model_filter_option.go model_filter_option_range.go model_filter_options_list.go diff --git a/catalog/internal/server/openapi/type_asserts.go b/catalog/internal/server/openapi/type_asserts.go index c2efe7964a..0300876da4 100644 --- a/catalog/internal/server/openapi/type_asserts.go +++ b/catalog/internal/server/openapi/type_asserts.go @@ -374,6 +374,26 @@ func AssertErrorRequired(obj model.Error) error { return nil } +// AssertFieldFilterConstraints checks if the values respects the defined constraints +func AssertFieldFilterConstraints(obj model.FieldFilter) error { + return nil +} + +// AssertFieldFilterRequired checks if the required fields are not zero-ed +func AssertFieldFilterRequired(obj model.FieldFilter) error { + elements := map[string]interface{}{ + "operator": obj.Operator, + "value": obj.Value, + } + for name, el := range elements { + if isZero := IsZeroValue(el); isZero { + return &RequiredError{Field: name} + } + } + + return nil +} + // AssertFilterOptionRangeConstraints checks if the values respects the defined constraints func AssertFilterOptionRangeConstraints(obj model.FilterOptionRange) error { return nil diff --git a/catalog/pkg/openapi/.openapi-generator/FILES b/catalog/pkg/openapi/.openapi-generator/FILES index 219a90503a..4ca5504b57 100644 --- a/catalog/pkg/openapi/.openapi-generator/FILES +++ b/catalog/pkg/openapi/.openapi-generator/FILES @@ -19,6 +19,7 @@ model_catalog_source_list.go model_catalog_source_preview_response.go model_catalog_source_preview_response_all_of_summary.go model_error.go +model_field_filter.go model_filter_option.go model_filter_option_range.go model_filter_options_list.go diff --git a/catalog/pkg/openapi/model_field_filter.go b/catalog/pkg/openapi/model_field_filter.go new file mode 100644 index 0000000000..370cfad264 --- /dev/null +++ b/catalog/pkg/openapi/model_field_filter.go @@ -0,0 +1,150 @@ +/* +Model Catalog REST API + +REST API for Model Registry to create and manage ML model metadata + +API version: v1alpha1 +*/ + +// Code generated by OpenAPI Generator (https://openapi-generator.tech); DO NOT EDIT. + +package openapi + +import ( + "encoding/json" +) + +// checks if the FieldFilter type satisfies the MappedNullable interface at compile time +var _ MappedNullable = &FieldFilter{} + +// FieldFilter struct for FieldFilter +type FieldFilter struct { + // Filter operator (e.g., '<', '=', '>', 'IN') + Operator string `json:"operator"` + // Filter value (can be number, string, or array) + Value interface{} `json:"value"` +} + +type _FieldFilter FieldFilter + +// NewFieldFilter instantiates a new FieldFilter object +// This constructor will assign default values to properties that have it defined, +// and makes sure properties required by API are set, but the set of arguments +// will change when the set of required properties is changed +func NewFieldFilter(operator string, value interface{}) *FieldFilter { + this := FieldFilter{} + this.Operator = operator + this.Value = value + return &this +} + +// NewFieldFilterWithDefaults instantiates a new FieldFilter object +// This constructor will only assign default values to properties that have it defined, +// but it doesn't guarantee that properties required by API are set +func NewFieldFilterWithDefaults() *FieldFilter { + this := FieldFilter{} + return &this +} + +// GetOperator returns the Operator field value +func (o *FieldFilter) GetOperator() string { + if o == nil { + var ret string + return ret + } + + return o.Operator +} + +// GetOperatorOk returns a tuple with the Operator field value +// and a boolean to check if the value has been set. +func (o *FieldFilter) GetOperatorOk() (*string, bool) { + if o == nil { + return nil, false + } + return &o.Operator, true +} + +// SetOperator sets field value +func (o *FieldFilter) SetOperator(v string) { + o.Operator = v +} + +// GetValue returns the Value field value +// If the value is explicit nil, the zero value for interface{} will be returned +func (o *FieldFilter) GetValue() interface{} { + if o == nil { + var ret interface{} + return ret + } + + return o.Value +} + +// GetValueOk returns a tuple with the Value field value +// and a boolean to check if the value has been set. +// NOTE: If the value is an explicit nil, `nil, true` will be returned +func (o *FieldFilter) GetValueOk() (*interface{}, bool) { + if o == nil || IsNil(o.Value) { + return nil, false + } + return &o.Value, true +} + +// SetValue sets field value +func (o *FieldFilter) SetValue(v interface{}) { + o.Value = v +} + +func (o FieldFilter) MarshalJSON() ([]byte, error) { + toSerialize, err := o.ToMap() + if err != nil { + return []byte{}, err + } + return json.Marshal(toSerialize) +} + +func (o FieldFilter) ToMap() (map[string]interface{}, error) { + toSerialize := map[string]interface{}{} + toSerialize["operator"] = o.Operator + if o.Value != nil { + toSerialize["value"] = o.Value + } + return toSerialize, nil +} + +type NullableFieldFilter struct { + value *FieldFilter + isSet bool +} + +func (v NullableFieldFilter) Get() *FieldFilter { + return v.value +} + +func (v *NullableFieldFilter) Set(val *FieldFilter) { + v.value = val + v.isSet = true +} + +func (v NullableFieldFilter) IsSet() bool { + return v.isSet +} + +func (v *NullableFieldFilter) Unset() { + v.value = nil + v.isSet = false +} + +func NewNullableFieldFilter(val *FieldFilter) *NullableFieldFilter { + return &NullableFieldFilter{value: val, isSet: true} +} + +func (v NullableFieldFilter) MarshalJSON() ([]byte, error) { + return json.Marshal(v.value) +} + +func (v *NullableFieldFilter) UnmarshalJSON(src []byte) error { + v.isSet = true + return json.Unmarshal(src, &v.value) +} diff --git a/catalog/pkg/openapi/model_filter_options_list.go b/catalog/pkg/openapi/model_filter_options_list.go index c5862fbe54..d30cfbf81e 100644 --- a/catalog/pkg/openapi/model_filter_options_list.go +++ b/catalog/pkg/openapi/model_filter_options_list.go @@ -21,6 +21,8 @@ var _ MappedNullable = &FilterOptionsList{} type FilterOptionsList struct { // A single filter option. Filters *map[string]FilterOption `json:"filters,omitempty"` + // Predefined named queries for common filtering scenarios + NamedQueries *map[string]map[string]FieldFilter `json:"namedQueries,omitempty"` } // NewFilterOptionsList instantiates a new FilterOptionsList object @@ -72,6 +74,38 @@ func (o *FilterOptionsList) SetFilters(v map[string]FilterOption) { o.Filters = &v } +// GetNamedQueries returns the NamedQueries field value if set, zero value otherwise. +func (o *FilterOptionsList) GetNamedQueries() map[string]map[string]FieldFilter { + if o == nil || IsNil(o.NamedQueries) { + var ret map[string]map[string]FieldFilter + return ret + } + return *o.NamedQueries +} + +// GetNamedQueriesOk returns a tuple with the NamedQueries field value if set, nil otherwise +// and a boolean to check if the value has been set. +func (o *FilterOptionsList) GetNamedQueriesOk() (*map[string]map[string]FieldFilter, bool) { + if o == nil || IsNil(o.NamedQueries) { + return nil, false + } + return o.NamedQueries, true +} + +// HasNamedQueries returns a boolean if a field has been set. +func (o *FilterOptionsList) HasNamedQueries() bool { + if o != nil && !IsNil(o.NamedQueries) { + return true + } + + return false +} + +// SetNamedQueries gets a reference to the given map[string]map[string]FieldFilter and assigns it to the NamedQueries field. +func (o *FilterOptionsList) SetNamedQueries(v map[string]map[string]FieldFilter) { + o.NamedQueries = &v +} + func (o FilterOptionsList) MarshalJSON() ([]byte, error) { toSerialize, err := o.ToMap() if err != nil { @@ -85,6 +119,9 @@ func (o FilterOptionsList) ToMap() (map[string]interface{}, error) { if !IsNil(o.Filters) { toSerialize["filters"] = o.Filters } + if !IsNil(o.NamedQueries) { + toSerialize["namedQueries"] = o.NamedQueries + } return toSerialize, nil } diff --git a/clients/ui/bff/internal/mocks/static_data_mock.go b/clients/ui/bff/internal/mocks/static_data_mock.go index caf0cdf18e..e7b217d020 100644 --- a/clients/ui/bff/internal/mocks/static_data_mock.go +++ b/clients/ui/bff/internal/mocks/static_data_mock.go @@ -1406,12 +1406,76 @@ func GetModelsWithInclusionStatusListMocks() []models.CatalogSourcePreviewModel Name: "sample-source/model-1", Included: true, }, + { + Name: "sample-source/model-2", + Included: true, + }, + { + Name: "sample-source/model-3", + Included: true, + }, + { + Name: "sample-source/model-4", + Included: true, + }, + { + Name: "sample-source/model-5", + Included: true, + }, + { + Name: "sample-source/model-6", + Included: false, + }, + { + Name: "adminModel1/model-1", + Included: true, + }, { Name: "adminModel1/model-2", Included: true, }, { Name: "adminModel1/model-3", + Included: true, + }, + { + Name: "adminModel1/model-4", + Included: true, + }, + { + Name: "adminModel1/model-5", + Included: true, + }, + { + Name: "adminModel1/model-6", + Included: true, + }, + { + Name: "adminModel1/model-7", + Included: true, + }, + { + Name: "adminModel1/model-8", + Included: true, + }, + { + Name: "adminModel1/model-9", + Included: true, + }, + { + Name: "adminModel1/model-10", + Included: false, + }, + { + Name: "adminModel1/model-11", + Included: false, + }, + { + Name: "adminModel1/model-12", + Included: false, + }, + { + Name: "adminModel1/model-13", Included: false, }, } @@ -1419,9 +1483,9 @@ func GetModelsWithInclusionStatusListMocks() []models.CatalogSourcePreviewModel func GetCatalogSourcePreviewSummaryMock() models.CatalogSourcePreviewSummary { return models.CatalogSourcePreviewSummary{ - TotalModels: 1500, - IncludedModels: 850, - ExcludedModels: 650, + TotalModels: 20, + IncludedModels: 15, + ExcludedModels: 5, } } diff --git a/clients/ui/frontend/src/app/api/modelCatalogSettings/service.ts b/clients/ui/frontend/src/app/api/modelCatalogSettings/service.ts index ce94eedb21..f39932809f 100644 --- a/clients/ui/frontend/src/app/api/modelCatalogSettings/service.ts +++ b/clients/ui/frontend/src/app/api/modelCatalogSettings/service.ts @@ -12,6 +12,8 @@ import { CatalogSourceConfig, CatalogSourceConfigList, CatalogSourceConfigPayload, + CatalogSourcePreviewRequest, + CatalogSourcePreviewResult, } from '~/app/modelCatalogTypes'; export const getCatalogSourceConfigs = @@ -74,3 +76,15 @@ export const deleteCatalogSourceConfig = (hostPath: string, queryParams: Record = {}) => (opts: APIOptions, sourceId: string): Promise => handleRestFailures(restDELETE(hostPath, `/source_configs/${sourceId}`, {}, queryParams, opts)); + +export const previewCatalogSource = + (hostPath: string, queryParams: Record = {}) => + (opts: APIOptions, data: CatalogSourcePreviewRequest): Promise => + handleRestFailures( + restCREATE(hostPath, '/source_preview', assembleModArchBody(data), queryParams, opts), + ).then((response) => { + if (isModArchResponse(response)) { + return response.data; + } + throw new Error('Invalid response format'); + }); diff --git a/clients/ui/frontend/src/app/hooks/modelCatalogSettings/useModelCatalogSettingsAPIState.tsx b/clients/ui/frontend/src/app/hooks/modelCatalogSettings/useModelCatalogSettingsAPIState.tsx index 583a2100c3..30dc5dae3f 100644 --- a/clients/ui/frontend/src/app/hooks/modelCatalogSettings/useModelCatalogSettingsAPIState.tsx +++ b/clients/ui/frontend/src/app/hooks/modelCatalogSettings/useModelCatalogSettingsAPIState.tsx @@ -6,6 +6,7 @@ import { getCatalogSourceConfig, getCatalogSourceConfigs, updateCatalogSourceConfig, + previewCatalogSource, } from '~/app/api/modelCatalogSettings/service'; import { ModelCatalogSettingsAPIs } from '~/app/modelCatalogTypes'; @@ -22,6 +23,7 @@ const useModelCatalogSettingsAPIState = ( getCatalogSourceConfig: getCatalogSourceConfig(path, queryParameters), updateCatalogSourceConfig: updateCatalogSourceConfig(path, queryParameters), deleteCatalogSourceConfig: deleteCatalogSourceConfig(path, queryParameters), + previewCatalogSource: previewCatalogSource(path, queryParameters), }), [queryParameters], ); diff --git a/clients/ui/frontend/src/app/modelCatalogTypes.ts b/clients/ui/frontend/src/app/modelCatalogTypes.ts index 2585406ce0..2868a0f96c 100644 --- a/clients/ui/frontend/src/app/modelCatalogTypes.ts +++ b/clients/ui/frontend/src/app/modelCatalogTypes.ts @@ -284,10 +284,43 @@ export type UpdateCatalogSourceConfig = ( ) => Promise; export type DeleteCatalogSourceConfig = (opts: APIOptions, sourceId: string) => Promise; +// Preview types +export type CatalogSourcePreviewRequest = { + type: string; + includedModels?: string[]; + excludedModels?: string[]; + properties?: Record; +}; + +export type CatalogSourcePreviewModel = { + name: string; + included: boolean; +}; + +export type CatalogSourcePreviewSummary = { + totalModels: number; + includedModels: number; + excludedModels: number; +}; + +export type CatalogSourcePreviewResult = { + items: CatalogSourcePreviewModel[]; + summary: CatalogSourcePreviewSummary; + nextPageToken: string; + pageSize: number; + size: number; +}; + +export type PreviewCatalogSource = ( + opts: APIOptions, + data: CatalogSourcePreviewRequest, +) => Promise; + export type ModelCatalogSettingsAPIs = { getCatalogSourceConfigs: GetCatalogSourceConfigs; createCatalogSourceConfig: CreateCatalogSourceConfig; getCatalogSourceConfig: GetCatalogSourceConfig; updateCatalogSourceConfig: UpdateCatalogSourceConfig; deleteCatalogSourceConfig: DeleteCatalogSourceConfig; + previewCatalogSource: PreviewCatalogSource; }; diff --git a/clients/ui/frontend/src/app/pages/modelCatalogSettings/components/CredentialsSection.tsx b/clients/ui/frontend/src/app/pages/modelCatalogSettings/components/CredentialsSection.tsx index eae6685180..631e406f8d 100644 --- a/clients/ui/frontend/src/app/pages/modelCatalogSettings/components/CredentialsSection.tsx +++ b/clients/ui/frontend/src/app/pages/modelCatalogSettings/components/CredentialsSection.tsx @@ -29,34 +29,27 @@ import { type CredentialsSectionProps = { formData: ManageSourceFormData; setData: UpdateObjectAtPropAndValue; + onValidate: () => Promise; + isValidating: boolean; + validationError?: Error; + isValidationSuccess: boolean; + onClearValidationSuccess: () => void; }; -const CredentialsSection: React.FC = ({ formData, setData }) => { +const CredentialsSection: React.FC = ({ + formData, + setData, + onValidate, + isValidating, + validationError, + isValidationSuccess, + onClearValidationSuccess, +}) => { const [isOrganizationTouched, setIsOrganizationTouched] = React.useState(false); const [isAccessTokenTouched, setIsAccessTokenTouched] = React.useState(false); const isOrganizationValid = validateOrganization(formData.organization); const isAccessTokenValid = validateAccessToken(formData.accessToken); - const [validationError, setValidationError] = React.useState(undefined); - const [isValidating, setIsValidating] = React.useState(false); - const [isValidationSuccess, setIsValidationSuccess] = React.useState(false); - - const handleValidate = async () => { - // setIsValidating(true); - // setValidationError(undefined); - - // TODO: Implement validation logic - // setShowAlert(true); - - // if success - setValidationError(undefined); - setIsValidationSuccess(true); - setIsValidating(false); - - //if fails - // setValidationError(new Error('error')); - // setIsValidationSuccess(false); - }; const organizationInput = ( = ({ formData, setDa {validationError && ( - The system cannot establish a connection to the source. Ensure that the organization and - access token are accurate, then try again. + {validationError.message} )} {isValidationSuccess && ( @@ -137,7 +129,7 @@ const CredentialsSection: React.FC = ({ formData, setDa variant="success" className="pf-v5-u-mt-md" title="Validation successful" - actionClose={ setIsValidationSuccess(false)} />} + actionClose={} > The organization and accessToken are valid for connection. @@ -145,9 +137,9 @@ const CredentialsSection: React.FC = ({ formData, setDa ); diff --git a/clients/ui/frontend/src/app/pages/modelCatalogSettings/components/PreviewPanel.tsx b/clients/ui/frontend/src/app/pages/modelCatalogSettings/components/PreviewPanel.tsx index 1fc509bd8e..4372447eab 100644 --- a/clients/ui/frontend/src/app/pages/modelCatalogSettings/components/PreviewPanel.tsx +++ b/clients/ui/frontend/src/app/pages/modelCatalogSettings/components/PreviewPanel.tsx @@ -8,58 +8,250 @@ import { Flex, FlexItem, Title, + Tabs, + Tab, + TabTitleText, + Alert, + List, + ListItem, + Spinner, + Pagination, + PaginationVariant, + AlertActionLink, } from '@patternfly/react-core'; -import { CubesIcon } from '@patternfly/react-icons'; +import { CubesIcon, CheckCircleIcon, TimesCircleIcon } from '@patternfly/react-icons'; import { PAGE_TITLES } from '~/app/pages/modelCatalogSettings/constants'; +import { CatalogSourcePreviewResult } from '~/app/modelCatalogTypes'; import PreviewButton from './PreviewButton'; type PreviewPanelProps = { isPreviewEnabled: boolean; + isLoading: boolean; onPreview: () => void; + previewResult?: CatalogSourcePreviewResult; + previewError?: Error; + hasFormChanged: boolean; }; -const PreviewPanel: React.FC = ({ isPreviewEnabled, onPreview }) => ( -
- - - - {PAGE_TITLES.MODEL_CATALOG_PREVIEW} - - - - - - - - - To view the models from this source that will appear in the model catalog with your current - configuration, complete all required fields, then click Preview. - - - +const PreviewPanel: React.FC = ({ + isPreviewEnabled, + isLoading, + onPreview, + previewResult, + previewError, + hasFormChanged, +}) => { + const [activeTabKey, setActiveTabKey] = React.useState(0); + const [page, setPage] = React.useState(1); + const [perPage, setPerPage] = React.useState(10); + + const handleTabSelect = (_event: React.MouseEvent, tabIndex: string | number) => { + setActiveTabKey(tabIndex); + setPage(1); // Reset to first page when switching tabs + }; + + const filteredItems = React.useMemo(() => { + if (!previewResult) { + return []; + } + if (activeTabKey === 0) { + return previewResult.items.filter((item) => item.included); + } + return previewResult.items.filter((item) => !item.included); + }, [previewResult, activeTabKey]); + + const paginatedItems = React.useMemo(() => { + const startIdx = (page - 1) * perPage; + const endIdx = startIdx + perPage; + return filteredItems.slice(startIdx, endIdx); + }, [filteredItems, page, perPage]); + + const onSetPage = ( + _event: React.MouseEvent | React.KeyboardEvent | MouseEvent, + newPage: number, + ) => { + setPage(newPage); + }; + + const onPerPageSelect = ( + _event: React.MouseEvent | React.KeyboardEvent | MouseEvent, + newPerPage: number, + ) => { + setPerPage(newPerPage); + setPage(1); + }; + + const renderEmptyState = () => { + if (previewError) { + return ( + + {previewError.message} + + + + + + + ); + } + + return ( + + + To view the models from this source that will appear in the model catalog with your + current configuration, complete all required fields, then click Preview. + + + + + + + + ); + }; + + const renderContent = () => { + if (isLoading) { + return ( +
+ +
+ ); + } + + if (!previewResult || previewError) { + return renderEmptyState(); + } + + return ( + <> + + Models included} /> + Models excluded} /> + +
+ {hasFormChanged && ( + + Refresh the preview + + } + /> + )} + {paginatedItems.length > 0 ? ( + <> + + + + {activeTabKey === 0 + ? `${previewResult.summary.includedModels} of ${previewResult.summary.totalModels} models included:` + : `${previewResult.summary.excludedModels} of ${previewResult.summary.totalModels} models excluded:`} + + + + + + + + {paginatedItems.map((model) => ( + + ) : ( + + ) + } + > + {model.name} + + ))} + + + + ) : ( + + + No models from this source are visible in the model catalog + + + )} +
+ + ); + }; + + return ( +
+ + + + {PAGE_TITLES.MODEL_CATALOG_PREVIEW} + + + - - - -
-); + + + {renderContent()} +
+ ); +}; export default PreviewPanel; diff --git a/go.mod b/go.mod index 0296df5234..cb23d5ed94 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.24.6 require ( github.com/DATA-DOG/go-sqlmock v1.5.2 github.com/alecthomas/participle/v2 v2.1.4 + github.com/deckarep/golang-set/v2 v2.8.0 github.com/go-chi/chi/v5 v5.2.3 github.com/go-chi/cors v1.2.2 github.com/go-logr/logr v1.4.3 diff --git a/go.sum b/go.sum index 2c48392509..128bbaf02f 100644 --- a/go.sum +++ b/go.sum @@ -117,6 +117,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/deckarep/golang-set/v2 v2.8.0 h1:swm0rlPCmdWn9mESxKOjWk8hXSqoxOp+ZlfuyaAdFlQ= +github.com/deckarep/golang-set/v2 v2.8.0/go.mod h1:VAky9rY/yGXJOLEDv3OMci+7wtDpOF4IN+y82NBOac4= github.com/dhui/dktest v0.4.5 h1:uUfYBIVREmj/Rw6MvgmqNAYzTiKOHJak+enB5Di73MM= github.com/dhui/dktest v0.4.5/go.mod h1:tmcyeHDKagvlDrz7gDKq4UAJOLIfVZYkfD5OnHDwcCo= github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= diff --git a/jobs/async-upload/poetry.lock b/jobs/async-upload/poetry.lock index c4cf80f764..fc924e70dc 100644 --- a/jobs/async-upload/poetry.lock +++ b/jobs/async-upload/poetry.lock @@ -1676,21 +1676,21 @@ typing-extensions = ">=4.12.0" [[package]] name = "urllib3" -version = "2.5.0" +version = "2.6.0" description = "HTTP library with thread-safe connection pooling, file post, and more." optional = false python-versions = ">=3.9" groups = ["main", "integration"] files = [ - {file = "urllib3-2.5.0-py3-none-any.whl", hash = "sha256:e6b01673c0fa6a13e374b50871808eb3bf7046c4b125b216f6bf1cc604cff0dc"}, - {file = "urllib3-2.5.0.tar.gz", hash = "sha256:3fc47733c7e419d4bc3f6b3dc2b4f890bb743906a30d56ba4a5bfa4bbff92760"}, + {file = "urllib3-2.6.0-py3-none-any.whl", hash = "sha256:c90f7a39f716c572c4e3e58509581ebd83f9b59cced005b7db7ad2d22b0db99f"}, + {file = "urllib3-2.6.0.tar.gz", hash = "sha256:cb9bcef5a4b345d5da5d145dc3e30834f58e8018828cbc724d30b4cb7d4d49f1"}, ] [package.extras] -brotli = ["brotli (>=1.0.9) ; platform_python_implementation == \"CPython\"", "brotlicffi (>=0.8.0) ; platform_python_implementation != \"CPython\""] +brotli = ["brotli (>=1.2.0) ; platform_python_implementation == \"CPython\"", "brotlicffi (>=1.2.0.0) ; platform_python_implementation != \"CPython\""] h2 = ["h2 (>=4,<5)"] socks = ["pysocks (>=1.5.6,!=1.5.7,<2.0)"] -zstd = ["zstandard (>=0.18.0)"] +zstd = ["backports-zstd (>=1.0.0) ; python_version < \"3.14\""] [[package]] name = "websocket-client" diff --git a/jobs/async-upload/requirements.txt b/jobs/async-upload/requirements.txt index d74a3ffa29..baed6a61d2 100644 --- a/jobs/async-upload/requirements.txt +++ b/jobs/async-upload/requirements.txt @@ -43,5 +43,5 @@ tqdm==4.67.1 ; python_version >= "3.11" and python_version < "4.0" typer-slim==0.20.0 ; python_version >= "3.11" and python_version < "4.0" typing-extensions==4.15.0 ; python_version >= "3.11" and python_version < "4.0" typing-inspection==0.4.2 ; python_version >= "3.11" and python_version < "4.0" -urllib3==2.5.0 ; python_version >= "3.11" and python_version < "4.0" +urllib3==2.6.0 ; python_version >= "3.11" and python_version < "4.0" yarl==1.20.1 ; python_version >= "3.11" and python_version < "4.0" diff --git a/tilt-controller.dockerfile b/tilt-controller.dockerfile index d6e8aea1ef..cdcefb88dc 100644 --- a/tilt-controller.dockerfile +++ b/tilt-controller.dockerfile @@ -1,4 +1,4 @@ -FROM alpine:3.22 +FROM alpine:3.23 WORKDIR / diff --git a/tilt-ui.dockerfile b/tilt-ui.dockerfile index d46f29a776..15d36d5701 100644 --- a/tilt-ui.dockerfile +++ b/tilt-ui.dockerfile @@ -1,4 +1,4 @@ -FROM alpine:3.22 +FROM alpine:3.23 WORKDIR / diff --git a/tilt.dockerfile b/tilt.dockerfile index a87131a48d..dddacef139 100644 --- a/tilt.dockerfile +++ b/tilt.dockerfile @@ -1,4 +1,4 @@ -FROM alpine:3.22 +FROM alpine:3.23 WORKDIR /