Repository: go-co-op/gocron
Pull request #263 (2021-12-02 22:28:05)

Allow removing Jobs that match multiple tags
### What does this do?
This changes `RemoveByTag` to accept multiple tags to match. All tags must be matched by the Job to be removed.


### Which issue(s) does this PR fix/relate to?
N/A


### List any changes that modify/break current functionality
These changes should not break any current functionality because the function name is unchanged and variadic parameter allows one or more inputs.


### Have you included tests for your changes?
Yes


### Did you document any new/modified functionality?

- [ ] Updated `example_test.go`
- [ ] Updated `README.md`

### Notes
If it would be better to rename the function to `RemoveByTags`, I can do that, but I didn't want to make a breaking change.


JohnRoesler: Great contribution! Thank you so much!

Patch:
diff --git a/job.go b/job.go
index c8df85c..c5e7729 100644
--- a/job.go
+++ b/job.go
@@ -137,6 +137,24 @@ func (j *Job) setDuration(t time.Duration) {
 	j.duration = t
 }
 
+// hasTags returns true if all tags are matched on this Job
+func (j *Job) hasTags(tags ...string) bool {
+	// Build map of all Job tags for easy comparison
+	jobTags := map[string]int{}
+	for _, tag := range j.tags {
+		jobTags[tag] = 0
+	}
+
+	// Loop through required tags and if one doesn't exist, return false
+	for _, tag := range tags {
+		_, ok := jobTags[tag]
+		if !ok {
+			return false
+		}
+	}
+	return true
+}
+
 // Error returns an error if one occurred while creating the Job.
 // If multiple errors occurred, they will be wrapped and can be
 // checked using the standard unwrap options.
diff --git a/scheduler.go b/scheduler.go
index 47656e4..1ab7543 100644
--- a/scheduler.go
+++ b/scheduler.go
@@ -619,9 +619,14 @@ func (s *Scheduler) removeByCondition(shouldRemove func(*Job) bool) {
 	s.setJobs(retainedJobs)
 }
 
-// RemoveByTag will remove a job by a given tag.
+// RemoveByTag will remove Jobs that match the given tag.
 func (s *Scheduler) RemoveByTag(tag string) error {
-	jobs, err := s.findJobsByTag(tag)
+	return s.RemoveByTags(tag)
+}
+
+// RemoveByTags will remove Jobs that match all given tags.
+func (s *Scheduler) RemoveByTags(tags ...string) error {
+	jobs, err := s.findJobsByTag(tags...)
 	if err != nil {
 		return err
 	}
@@ -632,17 +637,14 @@ func (s *Scheduler) RemoveByTag(tag string) error {
 	return nil
 }
 
-func (s *Scheduler) findJobsByTag(tag string) ([]*Job, error) {
+func (s *Scheduler) findJobsByTag(tags ...string) ([]*Job, error) {
 	var jobs []*Job
 
 Jobs:
 	for _, job := range s.Jobs() {
-		tags := job.Tags()
-		for _, t := range tags {
-			if t == tag {
-				jobs = append(jobs, job)
-				continue Jobs
-			}
+		if job.hasTags(tags...) {
+			jobs = append(jobs, job)
+			continue Jobs
 		}
 	}
 


Test patch:
diff --git a/job_test.go b/job_test.go
index b09c708..b910e6e 100644
--- a/job_test.go
+++ b/job_test.go
@@ -21,6 +21,72 @@ func TestTags(t *testing.T) {
 	assert.ElementsMatch(t, j.Tags(), []string{"tags", "tag", "some"})
 }
 
+func TestHasTags(t *testing.T) {
+	tests := []struct {
+		name      string
+		jobTags   []string
+		matchTags []string
+		expected  bool
+	}{
+		{
+			"OneTagMatch",
+			[]string{"tag1"},
+			[]string{"tag1"},
+			true,
+		},
+		{
+			"OneTagNoMatch",
+			[]string{"tag1"},
+			[]string{"tag2"},
+			false,
+		},
+		{
+			"DuplicateJobTagsMatch",
+			[]string{"tag1", "tag1"},
+			[]string{"tag1"},
+			true,
+		},
+		{
+			"DuplicateInputTagsMatch",
+			[]string{"tag1"},
+			[]string{"tag1", "tag1"},
+			true,
+		},
+		{
+			"MultipleTagsMatch",
+			[]string{"tag1", "tag2"},
+			[]string{"tag2", "tag1"},
+			true,
+		},
+		{
+			"MultipleTagsNoMatch",
+			[]string{"tag1", "tag2"},
+			[]string{"tag2", "tag1", "tag3"},
+			false,
+		},
+		{
+			"MultipleDuplicateTagsMatch",
+			[]string{"tag1", "tag1", "tag1", "tag2"},
+			[]string{"tag1", "tag2"},
+			true,
+		},
+		{
+			"MultipleDuplicateTagsNoMatch",
+			[]string{"tag1", "tag1", "tag1"},
+			[]string{"tag1", "tag1", "tag3"},
+			false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			j, _ := NewScheduler(time.UTC).Every(1).Minute().Do(task)
+			j.Tag(tt.jobTags...)
+			assert.Equal(t, tt.expected, j.hasTags(tt.matchTags...))
+		})
+	}
+}
+
 func TestJob_IsRunning(t *testing.T) {
 	s := NewScheduler(time.UTC)
 	j, err := s.Every(10).Seconds().Do(func() { time.Sleep(2 * time.Second) })
diff --git a/scheduler_test.go b/scheduler_test.go
index 6a1803a..2232a01 100644
--- a/scheduler_test.go
+++ b/scheduler_test.go
@@ -405,7 +405,7 @@ func TestScheduler_RemoveByReference(t *testing.T) {
 	})
 }
 
-func TestScheduler_RemoveByTag(t *testing.T) {
+func TestScheduler_RemoveByTags(t *testing.T) {
 	t.Run("non unique tags", func(t *testing.T) {
 		s := NewScheduler(time.UTC)
 
@@ -420,7 +420,7 @@ func TestScheduler_RemoveByTag(t *testing.T) {
 		// check Jobs()[0] tags is equal with tag "a" (tag1)
 		assert.Equal(t, s.Jobs()[0].Tags()[0], tag1, "Job With Tag 'a' is removed from index 0")
 
-		err = s.RemoveByTag(tag1)
+		err = s.RemoveByTags(tag1)
 		require.NoError(t, err)
 		assert.Equal(t, 1, s.Len(), "Incorrect number of jobs after removing 1 job")
 
@@ -428,7 +428,7 @@ func TestScheduler_RemoveByTag(t *testing.T) {
 		assert.Equal(t, s.Jobs()[0].Tags()[0], tag2, "Job With Tag 'tag two' is removed from index 0")
 
 		// Removing Non Existent Job with "a" because already removed above (will not removing any jobs because tag not match)
-		err = s.RemoveByTag(tag1)
+		err = s.RemoveByTags(tag1)
 		assert.EqualError(t, err, ErrJobNotFoundWithTag.Error())
 	})
 
@@ -444,7 +444,57 @@ func TestScheduler_RemoveByTag(t *testing.T) {
 		_, err = s.Every(1).Second().Tag(tag2).Do(taskWithParams, 2, "world") // index 1
 		require.NoError(t, err)
 
-		err = s.RemoveByTag("tag one")
+		err = s.RemoveByTags("tag one")
+		require.NoError(t, err)
+
+		// Adding job with tag after removing by tag, assuming the unique tag has been removed as well
+		_, err = s.Every(1).Second().Tag(tag1).Do(taskWithParams, 1, "hello")
+		assert.Nil(t, err, "Unique tag is not deleted when removing by tag")
+	})
+
+	t.Run("multiple non unique tags", func(t *testing.T) {
+		s := NewScheduler(time.UTC)
+
+		// Creating 2 Jobs with different tags
+		tag1 := "a"
+		tag2 := "ab"
+		tag3 := "abc"
+		_, err := s.Every(1).Second().Tag(tag1, tag3).Do(taskWithParams, 1, "hello") // index 0
+		require.NoError(t, err)
+		_, err = s.Every(1).Second().Tag(tag1, tag2).Do(taskWithParams, 2, "world") // index 1
+		require.NoError(t, err)
+
+		// check Jobs()[0] tags contains tag "a" (tag1) and "abc" (tag3)
+		assert.Contains(t, s.Jobs()[0].Tags(), tag1, "Job With Tag 'a' is removed from index 0")
+		assert.Contains(t, s.Jobs()[0].Tags(), tag3, "Job With Tag 'abc' is removed from index 0")
+
+		err = s.RemoveByTags(tag1, tag3)
+		require.NoError(t, err)
+		assert.Equal(t, 1, s.Len(), "Incorrect number of jobs after removing 1 job")
+
+		// check Jobs()[0] tags is equal with tag "a" (tag1) and "ab" (tag2) after removing "a"+"abc"
+		assert.Contains(t, s.Jobs()[0].Tags(), tag1, "Job With Tag 'a' is removed from index 0")
+		assert.Contains(t, s.Jobs()[0].Tags(), tag2, "Job With Tag 'ab' is removed from index 0")
+
+		// Removing Non Existent Job with "a"+"abc" because already removed above (will not removing any jobs because tag not match)
+		err = s.RemoveByTags(tag1, tag3)
+		assert.EqualError(t, err, ErrJobNotFoundWithTag.Error())
+	})
+
+	t.Run("multiple unique tags", func(t *testing.T) {
+		s := NewScheduler(time.UTC)
+		s.TagsUnique()
+
+		// Creating 2 Jobs with unique tags
+		tag1 := "tag one"
+		tag2 := "tag two"
+		tag3 := "tag three"
+		_, err := s.Every(1).Second().Tag(tag1, tag3).Do(taskWithParams, 1, "hello") // index 0
+		require.NoError(t, err)
+		_, err = s.Every(1).Second().Tag(tag2).Do(taskWithParams, 2, "world") // index 1
+		require.NoError(t, err)
+
+		err = s.RemoveByTags(tag1, tag3)
 		require.NoError(t, err)
 
 		// Adding job with tag after removing by tag, assuming the unique tag has been removed as well