Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.every functions adding the ability to run a job every amount in a given interval #97

Closed
wants to merge 31 commits into from

Conversation

MahdiBM
Copy link

@MahdiBM MahdiBM commented Apr 15, 2021

What does this PR add to the queues package?

This PR adds the ability to schedule a ScheduledJob to be run at different times in any interval. For example you can call the .every(_:in:) functions like so .every(.seconds(5), in: .minutes(1)) and expect the schedule to be run every 5 seconds in a minute, indefinitely.

Motivation

I've not only seen other people be looking for such option in a long time, but i myself have been looking forward to having such options as well. I also saw the open issue here on github, and saw tanner suggesting the syntax .every for a function which allows people do the same thing that i implemented. That led me to think about contributing to the package by implementing this feature myself.

What exactly is implemented?

  • First of all, a generalized .every(_:in:underestimatedCount:) to be able to flexibly schedule a task to be done every while in an interval. This function is declared in the class ScheduleContainer.Builder, which is the equivalent to the ScheduleBuilder class prior to this PR. This every function is declared in ScheduleContainer as well for more convenience.
  • Second, convenience functions on top of Secondly, Minutely and Hourly to schedule a task every input amount of times in those types' respective intervals.

How was this implementations made possible?

When we want to schedule a job, we need to do like so: app.queues.schedule(ScheduledJob())
which prior to this PR, would return a class ScheduleBuilder. We then can add time specifications to
the builder using the available functions like .at(Date()) or .minutely().at(5). The first problem in the way
of implementing .every functions is that a builder is supposed to only contain one job and one time, and the ScheduleBuilder neither can contain more than one time for a job, nor can produce more schedules of its own kind which have different times than itself. To fix this, i came up with 2-3 different ways, and chose the one that you see. I decided to make a change as described below:

  • Make a ScheduleContainer so it can contain more than one builder.
  • return the ScheduleContainer instead of the builder when users do app.queues.schedule(_:)

What ScheduleContainer does is to hold multiple values of ScheduleContainer.Builder, plus the job that the builders
are supposed to be scheduled with. So you'll see this in declaration of ScheduleContainer:

public let job: ScheduledJob
public var builders: [Builder] = []
    
public init(job: ScheduledJob) {
    self.job = job
}

I also changed Builder a little bit to hold a reference to its container, as well as using uuid to be able to identify builders
that are in a container:

public let id = UUID()
public let container: ScheduleContainer

I also changed the way that a builder holds the time that it should be scheduled at, by introducing an enum:

public let id = UUID()
public let container: ScheduleContainer
private var _timeValue: TimeValue? = nil

TimeValue:

public enum TimeValue {
            case exact(date: Date)
            case intervalBased(offset: TimeInterval,
                               interval: TimeInterval,
                               isFirstLifecycle: Bool = true)
            case componentBased(month: Month? = nil,
                                day: Day? = nil,
                                weekday: Weekday? = nil,
                                time: Time? = nil,
                                minute: Minute? = nil,
                                second: Second? = nil,
                                nanosecond: Int? = nil)
}

Now a builder can clean-ly hold its time. This also helps when calculating the nextDate.

Another also, i declared convenience functions on top of ScheduleContainer which also avoids breaking people's codes.
So as an example, the first function is in Builder, and the second one is in ScheduleContainer:

// MARK: - In `ScheduleContainer.Builder`:
/// Creates a monthly scheduled job for further building
    public func monthly() -> Monthly {
        return Monthly(builder: self)
    }

// MARK: - In `ScheduleContainer`
/// Creates a monthly scheduled job for further building
    public func monthly() -> Builder.Monthly {
        return Builder.Monthly(builder: Builder(container: self))
    }

One thing remaining, is that now that we have containers, one problem appears. The problem is that when we call app.queues.schedule(JOB()), the AnyScheduledJobs must not be immediately created. Instead, they should be created after the user is done with adding their builders to the container. If the app tries to create AnyScheduledJobs out of a container's job and builders immediately after the .schedule(_:) call happens, when it checks for builders, it'll see an empty array and basically no jobs will be added to the QueuesConfiguration. To tackle this i changed the scheduler to store all containers, and only calculate AnyScheduledJobs when needed:

// Previously:
var scheduledJobs: [AnyScheduledJob]

// Right now:
var scheduledJobsContainers: [ScheduleContainer]
var scheduledJobs: [AnyScheduledJob] {
    scheduledJobsContainers.map { container in
        container.builders.map { builder in
            AnyScheduledJob(job: container.job, scheduler: builder)
        }
    }.reduce(into: [AnyScheduledJob]()) { $0 += $1 }
}

This is the summary of what i've done. You can dig into the files more, and i've left some comments there as well. Feel free to ask any questions you might have, or point out any issues with this implementation.

My questions from those who check the code

These are basically the more-important things that i think i could improve:

  • In ScheduleContainer.swift line 338, Is underestimatedCount justified?
  • Currently the schedules built using .every func always start from the current time. I don't think thats an issue, but what are your opinions?
  • I also saw a @discardableResult behind the declarations of the .minutely function. I removed that since i couldn't figure out why it was there. Was that a mistake?
  • I think it is possible to release any containers that are completely done. Is that a good thing to implement?
  • Also feel free to suggest better names for the new stuff you see, if the names are not good enough.

MahdiBM added 17 commits April 13, 2021 12:55
…ore corresponding job

QueuesConfiguration now stores all ScheduleBuilderContainer s
ScheduleBuilderContainer now store the `job: ScheduledJob`.
AnyScheduledJobs are only made when they are needed, to prevent being made __before__ the container is populated with builders
ScheduleBuilderContainer renamed to ScheduleContainer because since the last commit, ScheduleContainer actually contains the `job` and this name is more appropriate
 Fixed an issue with setting values to _timeValue when. Previously when both the current _timeValue and the newValue were `.componentBased`, the current values of _timeValue were ignored and were not added to the newValue.
Now after call to `startScheduledJobs`, QueuesConfiguration will optimize the number of containers and will release unneeded containers, for a better memory management.
@MahdiBM MahdiBM changed the title .every functions adding the ability to run a schedule every amount in a given interval .every functions adding the ability to run a job every amount in a given interval Apr 16, 2021
MahdiBM added 2 commits April 18, 2021 20:55
The `continuously()` func schedules a task to be done again right-away, every time it is finished.
Changing precision of the delay because scheduling with nanosecond precision is possible with the `every()` funcs
@MahdiBM
Copy link
Author

MahdiBM commented Apr 21, 2021

Going to #98

@MahdiBM MahdiBM closed this Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant