feat(cron): enhance job validation and refactor core logic

[pcfreak30]

• Added ValidateCronJob function in core/cron.go to centralize job validation
• Introduced GetCronJobOrigin and GetCronJobSourceID helper functions
• Moved validation logic from service/cron.go to core package
• Improved retry policy validation in standalone_coordinator.go
• Updated job source ID handling in user cron and workflow cron services
• Fixed operation filter handling in request and TUS services

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of optional fields to prevent runtime errors when filtering requests.
  • Enhanced validation for cron job retry policies to catch invalid configurations earlier.

• Refactor

  • Streamlined cron job registration and validation for better reliability and maintainability.
  • Updated job source identifier handling for cron jobs to ensure consistent internal processing.

• Chores

  • Removed unused imports and made minor formatting adjustments for code clarity.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 4808376

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

The changes introduce centralized validation for cron jobs, add helper functions for extracting job origin and source ID, and update job registration to use the new validation logic. Retry policy validation is refactored for clarity, and pointer usage for the Operation field is standardized across request and TUS service logic. Some job constructors now use the new source ID extraction function.

Changes

File(s)	Change Summary
core/cron.go	Added ValidateCronJob, GetCronJobOrigin, and GetCronJobSourceID functions; minor formatting.
service/cron.go	Updated RegisterJob to use core.ValidateCronJob; removed redundant validation and unused import.
service/internal/cron/standalone_coordinator.go	Refactored retry policy validation logic for early error returns and clearer control flow.
service/internal/user/cron.go service/workflow_cron.go	Changed job constructors to use core.GetCronJobSourceID for source ID argument.
service/request.go	Updated Operation field checks to handle pointer types safely.
service/tus.go	Changed assignment of Operation in filters to use string pointers instead of direct values.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CronService
    participant Core
    participant DB
    participant Coordinator

    User->>CronService: RegisterJob(job, retryPolicy)
    CronService->>Core: ValidateCronJob(job)
    Core-->>CronService: Validation result
    CronService->>DB: Check for existing job
    DB-->>CronService: Exists/Not exists
    CronService->>DB: Create job record
    DB-->>CronService: Job record created
    CronService->>Coordinator: Enqueue job
    Coordinator-->>CronService: Enqueue result
    CronService-->>User: RegisterJob result

Loading

Poem

A bunny hops with code so neat,
Cron jobs now validate before they repeat!
With pointers in fields and helpers anew,
Retry flows clearer, and bugs are few.
From job source IDs to checks so strong—
This rabbit’s code will hop along! 🐇✨

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d83e332 and 4808376.

📒 Files selected for processing (7)

• core/cron.go (3 hunks)
• service/cron.go (2 hunks)
• service/internal/cron/standalone_coordinator.go (1 hunks)
• service/internal/user/cron.go (1 hunks)
• service/request.go (1 hunks)
• service/tus.go (3 hunks)
• service/workflow_cron.go (1 hunks)

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: pcfreak30
PR: LumeWeb/portal#825
File: service/internal/cron/redis/transport.go:32-40
Timestamp: 2025-06-28T05:34:11.246Z
Learning: In the portal project cron service overhaul, commented Redis code (like the transport implementation) should be ignored during review since it's not currently active and will be addressed when the cluster functionality is implemented later.

Learnt from: pcfreak30
PR: LumeWeb/portal#825
File: db/migrations/mysql/20250308183521_init.sql:67-82
Timestamp: 2025-06-28T05:32:32.043Z
Learning: For the cron service architectural overhaul in the portal project, no database migration code is planned to transform existing cron jobs from the old `function` field to the new schema columns (`origin`, `source_id`, `job_type`, `sched_def`). This is a clean slate approach where existing cron jobs will not be migrated.

Learnt from: pcfreak30
PR: LumeWeb/portal#825
File: service/internal/cron/cluster_coordinator.go:84-96
Timestamp: 2025-06-28T05:34:17.782Z
Learning: The ClusterCoordinator code in service/internal/cron/cluster_coordinator.go is intentionally commented out placeholder code for future clustered deployments and should be ignored for now. Missing references like c.scheduler and c.runJob in this commented code are expected since it's not meant to be functional yet.

service/internal/cron/standalone_coordinator.go (1)

Learnt from: pcfreak30
PR: LumeWeb/portal#825
File: service/internal/cron/cluster_coordinator.go:84-96
Timestamp: 2025-06-28T05:34:17.782Z
Learning: The ClusterCoordinator code in service/internal/cron/cluster_coordinator.go is intentionally commented out placeholder code for future clustered deployments and should be ignored for now. Missing references like c.scheduler and c.runJob in this commented code are expected since it's not meant to be functional yet.

core/cron.go (1)

Learnt from: pcfreak30
PR: LumeWeb/portal#825
File: db/migrations/mysql/20250308183521_init.sql:67-82
Timestamp: 2025-06-28T05:32:32.043Z
Learning: For the cron service architectural overhaul in the portal project, no database migration code is planned to transform existing cron jobs from the old `function` field to the new schema columns (`origin`, `source_id`, `job_type`, `sched_def`). This is a clean slate approach where existing cron jobs will not be migrated.

service/cron.go (2)

Learnt from: pcfreak30
PR: LumeWeb/portal#825
File: db/migrations/mysql/20250308183521_init.sql:67-82
Timestamp: 2025-06-28T05:32:32.043Z
Learning: For the cron service architectural overhaul in the portal project, no database migration code is planned to transform existing cron jobs from the old `function` field to the new schema columns (`origin`, `source_id`, `job_type`, `sched_def`). This is a clean slate approach where existing cron jobs will not be migrated.

Learnt from: pcfreak30
PR: LumeWeb/portal#825
File: service/internal/cron/cluster_coordinator.go:84-96
Timestamp: 2025-06-28T05:34:17.782Z
Learning: The ClusterCoordinator code in service/internal/cron/cluster_coordinator.go is intentionally commented out placeholder code for future clustered deployments and should be ignored for now. Missing references like c.scheduler and c.runJob in this commented code are expected since it's not meant to be functional yet.

🧬 Code Graph Analysis (5)

service/tus.go (1)

core/operation.go (1)

• Operation (47-51)

service/workflow_cron.go (1)

core/cron.go (1)

• GetCronJobSourceID (235-241)

service/internal/user/cron.go (1)

core/cron.go (1)

• GetCronJobSourceID (235-241)

core/cron.go (2)

db/models/cron_job.go (1)

• CronJob (38-53)

core/plugin.go (1)

• PluginExists (141-146)

service/cron.go (1)

core/cron.go (1)

• ValidateCronJob (168-211)

🔇 Additional comments (12)

service/tus.go (3)

70-70: LGTM: Consistent pointer usage for Operation field.

The change to pass &opName instead of opName aligns with the standardization of the Operation field as a pointer type in core.RequestFilter. This provides better handling of optional filter values.

---

92-92: LGTM: Consistent pointer usage for Operation field.

The change to pass &opName instead of opName aligns with the standardization of the Operation field as a pointer type in core.RequestFilter. This provides better handling of optional filter values.

---

119-119: LGTM: Consistent pointer usage for Operation field.

The change to pass &opName instead of opName aligns with the standardization of the Operation field as a pointer type in core.RequestFilter. This provides better handling of optional filter values.

service/request.go (1)

617-618: LGTM: Safe pointer handling for Operation field.

The changes properly handle the Operation field as a pointer by:

1. Checking for nil before dereferencing (filter.Operation != nil)
2. Safely dereferencing the pointer when passing to CreateRequestModel (*filter.Operation)

This aligns with the standardization of the Operation field across the codebase and prevents potential runtime panics.

core/cron.go (3)

52-56: Good formatting improvement

The alignment of field values in the DefaultRetryPolicy initialization improves readability.

---

168-211: Excellent centralization of validation logic

The ValidateCronJob function provides comprehensive validation that covers all necessary checks:

• Nil job validation
• Origin validation (core/plugin)
• Source ID validation with existence checks for plugins
• Job type format validation
• Namespace prefix enforcement

This consolidation improves maintainability and ensures consistent validation across the codebase.

---

221-241: Well-designed helper functions for job type parsing

The GetCronJobOrigin and GetCronJobSourceID functions provide clean abstractions for extracting metadata from job type strings. They properly handle edge cases and support the standardization of job metadata extraction across the codebase.

service/workflow_cron.go (1)

21-21: Correct use of source ID extraction

The change to use core.GetCronJobSourceID(workflowStepExecutorJobType) properly extracts "workflow" as the source ID from the job type string "core.workflow.step_executor". This aligns with the refactoring to standardize job metadata handling and is more semantically correct than passing the entire job type string.

service/internal/user/cron.go (1)

22-22: Consistent source ID extraction implementation

The change to use core.GetCronJobSourceID(ProcessAccountDeletionRequestsJobType) properly extracts "user" as the source ID from the job type string "core.user.process_account_deletion_requests". This maintains consistency with the refactoring approach used in other files and ensures proper job metadata handling.

service/internal/cron/standalone_coordinator.go (1)

197-208: Improved retry policy validation logic

The refactoring enhances the validation by:

• Moving the MaxRetries < 0 check earlier with immediate error return
• Adding an early return for MaxRetries == 0 to skip unnecessary delay/backoff validation when retries are disabled
• Making the validation logic more efficient and readable

This is a sensible optimization that improves both performance and code clarity.

service/cron.go (2)

252-254: Excellent refactoring to centralize validation

Replacing the manual validation logic with core.ValidateCronJob(job) significantly improves the code by:

• Eliminating duplicate validation code
• Centralizing validation logic for consistency
• Reducing method complexity
• Improving maintainability

This is a clean architectural improvement that leverages the comprehensive validation function introduced in core/cron.go.

---

308-308: Good cleanup of unnecessary variable

The change from using a jobType variable to calling job.Type() directly is a clean improvement that eliminates unnecessary code now that the validation logic is centralized.

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.