Conversation
Adds complete execution support for service-bound applications including:
- Multi-job scheduling (one job per service)
- Service-specific configuration and schedules
- On-demand triggering per service
- Service context tracking in job execution
- Backward compatible with existing global apps
Key changes:
- AbstractServiceNativeApplication: Full implementation of install, uninstall, trigger
- AppScheduler: Service-aware scheduling methods (scheduleApplicationForService, etc.)
- OmAppJobListener: Service context detection and configuration loading
- AbstractNativeApplicationBase: Service context support in execute()
- ApplicationHandler: Service-specific API methods
Each service gets its own Quartz job with identity pattern: {appName}-{serviceId}
Service context passed via JobDataMap with SERVICE_ID key.
Run records track serviceId in properties map.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit completes the migration to the new app-bound configuration architecture by updating all existing apps and infrastructure. Database Changes: - Migrate from Flyway to native migrations for app configuration - Update MySQL and PostgreSQL schema changes for nested config structure - Remove old Flyway migration files Legacy Method Migration (12 apps + utilities): - Update all app implementations to use AppBoundConfigurationUtil * AutoPilotApp, CacheWarmupApp, DataContractValidationApp * DataRetention, DataInsightsApp, DataInsightsReportApp * McpApplication, RdfIndexApp, SearchIndexApp, NoOpTestApplication - Fix AppRepository to use utility methods for config access - Fix OpenMetadataOperations (reindex, data insights operations) - Fix RunAppImpl (workflow automation) - Fix QuartzOrchestratorContext (search indexing) Base Class Updates: - Update AbstractNativeApplication to extend new base - Update AbstractGlobalNativeApplication with proper imports - Update AppService with configuration utility integration - Update ApplicationContext for new base class structure App Configuration Data: - Migrate all app JSON definitions to nested config structure - Update SearchIndexingApplication, DataRetentionApplication - Update DataInsightsApplication, DataContractValidationApplication - Update RdfIndexApp configuration Schema Updates: - Update app.json schema with boundType and configuration structure - Update createAppRequest.json for new configuration fields UI Updates: - Update AppDetails component for nested config display - Update AppSchedule component to use new schedule structure - Update ApplicationConfiguration for config rendering - Update AppInstall page for new configuration flow - Update yarn.lock dependencies All apps now use: - AppBoundConfigurationUtil.getAppConfiguration() for config access - AppBoundConfigurationUtil.getAppSchedule() for schedule access - AppBoundConfigurationUtil.getPrivateConfiguration() for private config Backward compatible - all existing global apps continue working. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix AppUpdater to track configuration and boundType changes, resolving service config persistence issue where createOrUpdate silently skipped updates because no field changes were detected - Fix AppMapper to set boundType and configuration from CreateApp request - Add POST/DELETE endpoints for service-configuration management on service-bound apps in AppResource - Refactor AppScheduler for service-bound job identity, stop, and delete - Fix OmAppJobListener to populate services array in run records - Fix AppBoundConfigurationUtil setConfig and NPE bugs - Replace TODO stubs in AppService with real implementations - Add ServiceHealthCheckApplication as a minimal service-bound test app with seed data, marketplace definition, and UI schema - Add 7 AppScheduler tests and 12 AppBoundConfigurationUtil tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # bootstrap/sql/migrations/native/1.13.0/mysql/schemaChanges.sql # bootstrap/sql/migrations/native/1.13.0/postgres/schemaChanges.sql # openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/autoPilot/AutoPilotApp.java # openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/insights/DataInsightsApp.java # openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/AppRepository.java # openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java # openmetadata-ui-core-components/src/main/resources/ui/yarn.lock
This file was referenced by UI components modified in commit d94f64e but was accidentally left out of that commit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix duplicate service configuration on re-POST (upsert semantics) - Return App JSON from service-configuration endpoints instead of strings - Handle schedule removal via scheduleTimeline=None (delete Quartz job) - Add serviceId validation for trigger/stop on service-bound apps (400 vs NPE) - Add boundType to AppMarketPlaceDefinition and CreateAppMarketPlaceDefinitionReq schemas - Update AppMarketPlaceMapper to copy boundType from seed data - Update AppMapper to fallback to marketplace boundType when not in request - Fix ServiceHealthCheckApp to use delaySeconds config for iteration count - Remove incorrect apps_marketplace migration (boundType/configuration) - Fix AppRepository formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
aji-aju
requested review from
a team,
chirag-madlani and
karanh37
as code owners
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
aji-aju
changed the title
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Existing apps_marketplace rows don't have boundType set, which causes app installation to fail because AppMapper falls back to the marketplace definition's boundType (null → defaults to Global → prevents service-bound apps from being recognized). This migration sets boundType='Global' on all marketplace entries that lack it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
The seed JSON was missing boundType, so Jackson deserialized it as null (JSON Schema defaults don't apply during deserialization). This caused the app to be treated as Global after seed loading, failing install with "Cannot get service configurations for global app". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
…d and UI Route all app configuration access through AppBoundConfigurationUtil instead of direct flat field access. This ensures internal/global apps use the nested configuration structure while external apps continue using flat fields. Backend: - AbstractNativeApplication: use routing utility for config reads and writes - ApplicationHandler: encrypt/decrypt through routing utility - AppRepository: normalize config in prepare(), migrate in setFields() - AppBoundConfigurationUtil: add isExternalApp(), flat fallbacks for external apps, null-safe unsetPrivateConfiguration, simplified migration logic SQL migrations: - Skip external apps during flat-to-nested migration - Fix privateConfig -> privateConfiguration field name UI: - AppConfigUtils: mirror backend routing logic with AppType-aware accessors - AppRunsHistory: use getAppConfig() instead of direct appConfiguration - LogsViewerPage: use getAppSchedule() instead of direct appSchedule Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
aji-aju
added
UI
- Add boundType and serviceId query parameters to GET /v1/apps endpoint - Implement JSON path queries in ListFilter for MySQL (JSON_EXTRACT/JSON_SEARCH) and PostgreSQL (jsonb operators/jsonb_array_elements) - Fix status string case mismatch in ServiceBoundAppsIT (lowercase enum values) - Add Group 8 IT tests: filterByBoundTypeService, filterByBoundTypeGlobal, filterByServiceId, filterByServiceId_excludesUnconfiguredApps Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppMarketPlaceMapper.java # openmetadata-spec/src/main/resources/json/schema/entity/applications/marketplace/appMarketPlaceDefinition.json # openmetadata-spec/src/main/resources/json/schema/entity/applications/marketplace/createAppMarketPlaceDefinitionReq.json
- Add authorization checks to addServiceConfiguration/removeServiceConfiguration endpoints - Fix UnsupportedOperationException on List.of() in removeServiceConfiguration - Fix useMemo dependency causing recomputation every render in AppSchedule - Fix migration WHERE clause to include privateConfiguration-only apps - Validate boundType query param returns 400 for invalid values - Fix McpServer compilation error (AbstractNativeApplication → AbstractNativeApplicationBase) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds first-class support for service-bound internal apps in OpenMetadata, introducing a nested configuration model (configuration.globalAppConfig / configuration.serviceAppConfig) and extending scheduling, triggering, stopping, filtering, and run-history retrieval to work per service.
Changes:
- Introduces schema + seed-data changes for
boundTypeand nestedconfiguration(global + per-service). - Extends backend scheduling/execution pipeline to support service-scoped Quartz jobs and serviceId-filtered run history.
- Updates UI to read/write app config/schedule from the new nested structure while preserving external-app behavior.
Reviewed changes
Copilot reviewed 59 out of 60 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/utils/ApplicationSchemas/ServiceHealthCheckApplication.json | Adds UI JSON schema for the new service-bound test app configuration. |
| openmetadata-ui/src/main/resources/ui/src/utils/AppConfigUtils.ts | Adds UI-side routing helpers to read/write config/schedule from nested vs flat app fields. |
| openmetadata-ui/src/main/resources/ui/src/pages/LogsViewerPage/LogsViewerPage.tsx | Switches schedule access to getAppSchedule() for nested-config compatibility. |
| openmetadata-ui/src/main/resources/ui/src/pages/AppInstall/AppInstall.component.tsx | Uses getAppConfig() when preparing install payloads. |
| openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppSchedule/AppSchedule.component.tsx | Reads schedule via helper and adjusts memo deps accordingly. |
| openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppRunsHistory/AppRunsHistory.component.tsx | Uses getAppConfig() for current-config synthetic run record. |
| openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/ApplicationConfiguration/ApplicationConfiguration.tsx | Uses getAppConfig() to populate form data for nested config. |
| openmetadata-ui/src/main/resources/ui/src/components/Settings/Applications/AppDetails/AppDetails.component.tsx | Writes config/schedule via helper functions to target nested structure. |
| openmetadata-ui-core-components/src/main/resources/ui/yarn.lock | Lockfile update for CJS package aliases metadata. |
| openmetadata-spec/src/main/resources/json/schema/entity/applications/serviceAppConfig.json | New schema for per-service application configuration entries. |
| openmetadata-spec/src/main/resources/json/schema/entity/applications/scheduleTimeLine.json | New standalone schema for schedule timeline enum. |
| openmetadata-spec/src/main/resources/json/schema/entity/applications/marketplace/createAppMarketPlaceDefinitionReq.json | Adds boundType to marketplace create request schema. |
| openmetadata-spec/src/main/resources/json/schema/entity/applications/marketplace/appMarketPlaceDefinition.json | Adds boundType to marketplace definition schema. |
| openmetadata-spec/src/main/resources/json/schema/entity/applications/globalAppConfig.json | New schema for global nested app configuration. |
| openmetadata-spec/src/main/resources/json/schema/entity/applications/createAppRequest.json | Adds boundType and nested configuration to app create request schema. |
| openmetadata-spec/src/main/resources/json/schema/entity/applications/appSchedule.json | New schema for AppSchedule. |
| openmetadata-spec/src/main/resources/json/schema/entity/applications/app.json | Adds appBoundType + appBoundConfiguration definitions and top-level fields. |
| openmetadata-service/src/test/java/org/openmetadata/service/util/AppBoundConfigurationUtilTest.java | Adds unit tests for nested config routing and service config management. |
| openmetadata-service/src/test/java/org/openmetadata/service/apps/scheduler/AppSchedulerTest.java | Adds tests for job identity matching and deletion across global/service/on-demand patterns. |
| openmetadata-service/src/main/resources/json/data/appMarketPlaceDefinition/ServiceHealthCheckApplication.json | Seeds marketplace definition for service-bound health check app. |
| openmetadata-service/src/main/resources/json/data/app/ServiceHealthCheckApplication.json | Seeds installed app definition for service-bound health check app. |
| openmetadata-service/src/main/resources/json/data/app/SearchIndexingApplication.json | Migrates seeded global app config/schedule into nested globalAppConfig. |
| openmetadata-service/src/main/resources/json/data/app/RdfIndexApp.json | Migrates seeded global app config/schedule into nested globalAppConfig. |
| openmetadata-service/src/main/resources/json/data/app/DataRetentionApplication.json | Migrates seeded global app config/schedule into nested globalAppConfig. |
| openmetadata-service/src/main/resources/json/data/app/DataInsightsApplication.json | Migrates seeded global app config/schedule into nested globalAppConfig. |
| openmetadata-service/src/main/resources/json/data/app/DataContractValidationApplication.json | Migrates seeded global app config/schedule into nested globalAppConfig. |
| openmetadata-service/src/main/java/org/openmetadata/service/util/OpenMetadataOperations.java | Switches internal operations to read config via AppBoundConfigurationUtil. |
| openmetadata-service/src/main/java/org/openmetadata/service/util/AppBoundConfigurationUtil.java | New core utility for nested/flat config routing + service-config helpers + legacy migration. |
| openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java | Adds API filters (boundType/serviceId) + service-configuration endpoints + serviceId-aware trigger/stop/run-status. |
| openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppMarketPlaceMapper.java | Maps marketplace boundType into entity. |
| openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppMapper.java | Maps boundType and nested configuration from create requests into App entity. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ListFilter.java | Adds SQL conditions for filtering apps by boundType and configured serviceId. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/AppRepository.java | Migrates legacy config on read, normalizes internal apps to nested-only on write, and records nested config changes. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/automatedTask/runApp/RunAppImpl.java | Uses AppBoundConfigurationUtil for config access and mutation. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/scheduler/OmAppJobListener.java | Adds serviceId-aware config/schedule selection and persists service context into run records. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/scheduler/AppScheduler.java | Adds service-bound scheduling, on-demand triggering, stop-by-service, and delete-all-jobs behavior. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/test/NoOpTestApplication.java | Updates test app base class to global-native base. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/serviceHealthCheck/ServiceHealthCheckApp.java | New service-bound internal app used for validation/testing of the feature. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/SearchIndexApp.java | Updates to global-native base + nested config access. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/searchIndex/QuartzOrchestratorContext.java | Reads/writes config via AppBoundConfigurationUtil. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/rdf/RdfIndexApp.java | Updates to global-native base + nested config access. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/mcp/McpApplication.java | Updates to global-native base. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/insights/DataInsightsReportApp.java | Updates to global-native base + nested config access. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/insights/DataInsightsApp.java | Updates to global-native base + nested config access. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/dataRetention/DataRetention.java | Updates to global-native base + nested config access. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/dataContracts/DataContractValidationApp.java | Updates to global-native base. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/cache/CacheWarmupApp.java | Updates to global-native base + nested config access. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/autoPilot/AutoPilotApp.java | Updates to global-native base + nested config access. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/AppService.java | New service layer encapsulating app CRUD + execution + service-config lifecycle. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/ApplicationHandler.java | Adds service-bound trigger path, config routing for encryption/decryption, and job cleanup updates. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/ApplicationContext.java | Updates registry types to AbstractNativeApplicationBase. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractServiceNativeApplication.java | New base class for service-bound internal apps (install/uninstall/schedule/trigger per service). |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplicationBase.java | New base class unifying execution/trigger logic and serviceId job context handling. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplication.java | Refactors legacy base to extend new base and to use nested config access. |
| openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractGlobalNativeApplication.java | New base class for global apps (install/schedule/trigger). |
| openmetadata-mcp/src/main/java/org/openmetadata/mcp/McpServer.java | Updates MCP server to work with the new application base type. |
| openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ServiceBoundAppsIT.java | Adds full integration coverage for service config lifecycle + trigger/stop/filtering + global regression checks. |
| openmetadata-integration-tests/pom.xml | Ensures new IT is included/excluded consistently across profiles. |
| bootstrap/sql/migrations/native/1.13.0/postgres/schemaChanges.sql | Migrates installed_apps to nested config and adds generated column/index for boundType filtering. |
| bootstrap/sql/migrations/native/1.13.0/mysql/schemaChanges.sql | Same as Postgres migration for MySQL. |
| if (app.getBoundType() != AppBoundType.Service) { | ||
| throw new BadRequestException( | ||
| "Cannot add service configuration to non-service-bound application: " + name); | ||
| } | ||
|
|
||
| UUID serviceId = serviceConfig.getServiceRef().getId(); | ||
|
|
||
| AppBoundConfigurationUtil.addServiceConfiguration(app, serviceConfig.getServiceRef()); |
There was a problem hiding this comment.
serviceConfig.getServiceRef().getId() can throw a NullPointerException if the request body omits serviceRef (or its id). Please validate serviceConfig, serviceConfig.getServiceRef(), and getId() and return a 400 with a clear message before using them.
| validateServerExecutableApp(getAppRuntime(getApp())); | ||
|
|
||
| Map<String, Object> serviceAppConfig = | ||
| JsonUtils.getMap(AppBoundConfigurationUtil.getAppConfiguration(getApp(), serviceId)); | ||
| if (config != null) { | ||
| serviceAppConfig.putAll(config); | ||
| } |
There was a problem hiding this comment.
JsonUtils.getMap(AppBoundConfigurationUtil.getAppConfiguration(...)) can return null when the service configuration exists but has no config set. In that case serviceAppConfig.putAll(config) will NPE. Initialize serviceAppConfig to an empty Map (e.g., new HashMap) when the stored config is missing/null before merging overrides and validating.
| ) | ||
| WHERE (jsonb_exists(json, 'appConfiguration') | ||
| OR jsonb_exists(json, 'appSchedule') | ||
| OR jsonb_exists(json, 'privateConfiguration')) |
There was a problem hiding this comment.
This migration overwrites boundType and the entire configuration object for any internal app that still has legacy appConfiguration/appSchedule/privateConfiguration keys. If an installation already has the new nested configuration (e.g., from a pre-release / partial rollout) this UPDATE can clobber existing nested config. Consider guarding with WHERE NOT jsonb_exists(json,'configuration') (and similarly for boundType) or merging only missing fields.
| OR jsonb_exists(json, 'privateConfiguration')) | |
| OR jsonb_exists(json, 'privateConfiguration')) | |
| AND NOT jsonb_exists(json, 'configuration') | |
| AND NOT jsonb_exists(json, 'boundType') |
| ) | ||
| WHERE (JSON_CONTAINS_PATH(json, 'one', '$.appConfiguration') | ||
| OR JSON_CONTAINS_PATH(json, 'one', '$.appSchedule') | ||
| OR JSON_CONTAINS_PATH(json, 'one', '$.privateConfiguration')) |
There was a problem hiding this comment.
This MySQL migration unconditionally sets $.boundType and replaces $.configuration for internal apps that still have legacy fields. If any rows already contain a populated configuration (e.g., from a prior/partial migration), this can overwrite it. Consider adding a guard like AND NOT JSON_CONTAINS_PATH(json,'one','$.configuration') (and/or merging only missing nested fields) to avoid clobbering existing config.
| OR JSON_CONTAINS_PATH(json, 'one', '$.privateConfiguration')) | |
| OR JSON_CONTAINS_PATH(json, 'one', '$.privateConfiguration')) | |
| AND NOT JSON_CONTAINS_PATH(json, 'one', '$.configuration') |
| // ApplicationContext registration temporarily disabled for compatibility | ||
| // TODO: Update ApplicationContext to work with AbstractNativeApplicationBase | ||
| } | ||
|
|
||
| @Override | ||
| public void uninstall() { | ||
| // ApplicationContext unregistration temporarily disabled for compatibility | ||
| // TODO: Update ApplicationContext to work with AbstractNativeApplicationBase | ||
| } |
There was a problem hiding this comment.
The comments in init()/uninstall() say ApplicationContext registration is “temporarily disabled” and include a TODO to update ApplicationContext for AbstractNativeApplicationBase, but ApplicationContext is already updated to use AbstractNativeApplicationBase. Please remove/update these comments to avoid misleading future maintenance.
| // ApplicationContext registration temporarily disabled for compatibility | |
| // TODO: Update ApplicationContext to work with AbstractNativeApplicationBase | |
| } | |
| @Override | |
| public void uninstall() { | |
| // ApplicationContext unregistration temporarily disabled for compatibility | |
| // TODO: Update ApplicationContext to work with AbstractNativeApplicationBase | |
| } | |
| } | |
| @Override | |
| public void uninstall() {} |
✅ TypeScript Types Auto-UpdatedThe generated TypeScript types have been automatically updated based on JSON schema changes in this PR. |
Code Review
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
| .withStartTime(pipelineStatus.getStartDate()) | ||
| .withExecutionTime( | ||
| pipelineStatus.getEndDate() == null | ||
| ? System.currentTimeMillis() - pipelineStatus.getStartDate() |
There was a problem hiding this comment.
⚠️ Bug: NPE in convertPipelineStatus when startDate is null
In AppService.convertPipelineStatus(), pipelineStatus.getStartDate() returns a boxed Long that may be null. The expression System.currentTimeMillis() - pipelineStatus.getStartDate() will throw a NullPointerException during auto-unboxing if startDate is null. While endDate is null-checked, startDate is not.
Suggested fix:
.withExecutionTime(
pipelineStatus.getStartDate() == null
? 0L
: pipelineStatus.getEndDate() == null
? System.currentTimeMillis() - pipelineStatus.getStartDate()
: pipelineStatus.getEndDate() - pipelineStatus.getStartDate())
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
| App app = appRepository.get(uriInfo, id, appRepository.getFields("bot"), Include.ALL, false); | ||
| String userName = securityContext.getUserPrincipal().getName(); | ||
|
|
||
| executorService.submit( |
There was a problem hiding this comment.
⚠️ Bug: Request-scoped SecurityContext used in background thread
In deleteAppAsync(), the JAX-RS SecurityContext is captured by the lambda and used inside executorService.submit(...). SecurityContext is a request-scoped proxy — once the HTTP request completes (which happens immediately since the method returns Response.accepted()), accessing it from the background thread is undefined behavior. It may return stale data, null, or throw. Both deleteAppInternal(securityContext, app) and WebsocketNotificationHandler.sendDeleteOperationCompleteNotification(jobId, securityContext, ...) use it after the request is gone.
Suggested fix:
Extract the needed values (e.g., userName) from securityContext
before submitting the async task:
String userName = securityContext.getUserPrincipal().getName();
executorService.submit(() -> {
// use userName instead of securityContext
...
});
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
| throw new IllegalArgumentException( | ||
| CatalogExceptionMessage.systemEntityModifyNotAllowed(app.getName(), "SystemApp")); | ||
| } | ||
| AppScheduler.getInstance().deleteAllApplicationJobs(app); |
There was a problem hiding this comment.
⚠️ Bug: patchApp deletes scheduler jobs before confirming patch succeeds
In patchApp(), AppScheduler.getInstance().deleteAllApplicationJobs(app) is called at line 183 before appRepository.patch(...) at line 186. If the patch operation throws an exception, all Quartz jobs for the app have already been deleted with no rollback mechanism. The app will silently become unscheduled. The same pattern exists in AppResource.java patch endpoints at lines 813 and 861.
Suggested fix:
Move the deleteAllApplicationJobs call to after the patch
succeeds, or wrap the sequence in a try-catch that re-schedules
on failure.
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
Jest test CoverageUI tests summary
|
|
|
🔴 Playwright Results — 6 failure(s), 21 flaky✅ 3371 passed · ❌ 6 failed · 🟡 21 flaky · ⏭️ 189 skipped
Genuine Failures (failed on all attempts)❌
|
Describe your changes:
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
AbstractServiceNativeApplicationbase class for service-specific scheduling and triggeringServiceHealthCheckApplicationtest app demonstrating service-bound functionalityAppBoundConfigurationUtilhandles nested vs. flat config access for global/service-bound appsconfigurationobject/service-configurationPOST/DELETE for managing per-service app configurationsboundTypeandserviceIdlist filters for apps endpointboundTypecolumn andappBoundTypevirtual index toinstalled_appsconfiguration.serviceAppConfigarrayServiceBoundAppsITcovering configuration lifecycle, triggering, stopping, and filteringThis will update automatically on new commits.