Fix/schedule cron review findings - #129
Merged
Merged
Conversation
isDue() combined all five cron fields with AND, so an expression like "0 9 1 * 1" (meant to mean "9am on the 1st of the month OR every Monday", per standard cron) only matched on the rare day that was both at once — a silent behavior mismatch for anyone coming from standard crontab semantics. Track whether the day-of-month/day-of-week fields were literally restricted (not the bare "*"), and OR them together when both are; otherwise keep the existing AND (the unrestricted field is always true, so the two agree, no special-casing needed).
4 tests: both-restricted fields OR together (e.g. "0 9 1 * 1"), either field alone still acts as AND against a wildcard sibling, and a step field (e.g. "*/2") still counts as "restricted" for this rule.
[ClassName::class, 'method'] only worked when "method" was static: is_callable() is false for a class-string paired with an instance method, so the documented array-callback form threw "Scheduled task callback is not callable." for the common case of a regular (non-static) method. Instantiate the class ourselves in that case, same as 'Class@method' already did, so both forms work identically regardless of whether the method is static. Since there's no dependency container here, that instantiation still only supports a no-argument constructor — but a class that needs one now fails with a clear, actionable RuntimeException pointing at the [$instance, 'method'] workaround, instead of a bare ArgumentCountError leaking out of `new $class()`. Documented both the supported forms and this limitation on ScheduleEvent's constructor.
5 tests: [Class::class, method] now works for non-static methods, still works for static ones, 'Class@method' still works, a class needing constructor arguments fails with an actionable message, and the documented workaround (pre-built instance) bypasses that limit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.