Merge pull request #40 from cardmagic/fix-race-condition-bug-deleting-categories

Fix race condition bug deleting categories

Author: cardmagic

.github/workflows/ruby.yml

@@ -22,7 +22,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        ruby-version: ['2.7', '3.0', '3.1', '3.2', 'head']
+        ruby-version: ['2.7', 'head']
 
     steps:
     - uses: actions/checkout@v4

Gemfile

@@ -3,3 +3,4 @@ gemspec
 
 gem 'fast-stemmer'
 gem 'matrix'
+gem 'mutex_m'

Gemfile.lock

@@ -1,8 +1,9 @@
 PATH
   remote: .
   specs:
-    classifier (1.4.0)
-      fast-stemmer (~> 1.0.0)
+    classifier (1.4.2)
+      fast-stemmer (~> 1.0)
+      mutex_m (~> 0.2)
       rake
 
 GEM
@@ -11,6 +12,7 @@ GEM
     fast-stemmer (1.0.2)
     matrix (0.4.2)
     minitest (5.18.1)
+    mutex_m (0.2.0)
     psych (5.1.2)
       stringio
     rake (13.0.6)
@@ -28,6 +30,7 @@ DEPENDENCIES
   fast-stemmer
   matrix
   minitest
+  mutex_m
   rdoc
 
 BUNDLED WITH

classifier.gemspec

@@ -1,6 +1,6 @@
 Gem::Specification.new do |s|
   s.name        = 'classifier'
-  s.version     = '1.4.1'
+  s.version     = '1.4.2'
   s.summary     = 'A general classifier module to allow Bayesian and other types of classifications.'
   s.description = 'A general classifier module to allow Bayesian and other types of classifications.'
   s.author = 'Lucas Carlson'
@@ -9,7 +9,8 @@ Gem::Specification.new do |s|
   s.files = Dir['{lib}/**/*.rb', 'bin/*', 'LICENSE', '*.md', 'test/*']
   s.license = 'LGPL'
 
-  s.add_dependency 'fast-stemmer', '~> 1.0.0'
+  s.add_dependency 'fast-stemmer', '~> 1.0'
+  s.add_dependency 'mutex_m', '~> 0.2'
   s.add_dependency 'rake'
   s.add_development_dependency 'minitest'
   s.add_development_dependency 'rdoc'

lib/classifier/bayes.rb

@@ -152,10 +152,11 @@ def remove_category(category)
       category = category.prepare_category_name
       raise StandardError, "No such category: #{category}" unless @categories.key?(category)
 
+      @total_words -= @category_word_count[category].to_i
+
       @categories.delete(category)
       @category_counts.delete(category)
       @category_word_count.delete(category)
-      @total_words -= @category_word_count[category].to_i
     end
   end
 end

test/bayes/bayesian_test.rb

@@ -54,12 +54,6 @@ def test_remove_category
     assert_equal ['Interesting'], @classifier.categories
   end
 
-  def test_remove_nonexistent_category
-    assert_raises(StandardError) do
-      @classifier.remove_category 'NonexistentCategory'
-    end
-  end
-
   def test_remove_category_affects_classification
     @classifier.train_interesting 'This is interesting content'
     @classifier.train_uninteresting 'This is uninteresting content'
@@ -94,4 +88,34 @@ def test_remove_category_preserves_other_category_data
 
     assert_equal interesting_classification, @classifier.classify('This is interesting')
   end
+
+  def test_remove_category_check_counts
+    initial_total_words = @classifier.instance_variable_get(:@total_words)
+    category_word_count = @classifier.instance_variable_get(:@category_word_count)['Interesting']
+
+    @classifier.remove_category('Interesting')
+
+    assert_nil @classifier.instance_variable_get(:@categories)['Interesting']
+    assert_equal @classifier.instance_variable_get(:@category_counts)['Interesting'], 0
+    assert_equal @classifier.instance_variable_get(:@category_word_count)['Interesting'], 0
+
+    new_total_words = @classifier.instance_variable_get(:@total_words)
+    assert_equal initial_total_words - category_word_count, new_total_words
+  end
+
+  def test_remove_category_updates_total_words_before_deletion
+    initial_total_words = @classifier.instance_variable_get(:@total_words)
+    category_word_count = @classifier.instance_variable_get(:@category_word_count)['Interesting']
+
+    @classifier.remove_category('Interesting')
+
+    new_total_words = @classifier.instance_variable_get(:@total_words)
+    assert_equal initial_total_words - category_word_count, new_total_words
+  end
+
+  def test_remove_nonexistent_category
+    assert_raises(StandardError, 'No such category: Nonexistent Category') do
+      @classifier.remove_category('Nonexistent Category')
+    end
+  end
 end