Merge branch 'fj-refactor-web-ide-counter' into 'master'

Refactor WebIdeCommitsCounter and RedisCounter

See merge request gitlab-org/gitlab-ce!30850

lib/api/commits.rb

@@ -126,7 +126,7 @@ module API
         if result[:status] == :success
           commit_detail = user_project.repository.commit(result[:result])
 
-          Gitlab::UsageDataCounters::WebIdeCommitsCounter.increment if find_user_from_warden
+          Gitlab::UsageDataCounters::WebIdeCounter.increment_commits_count if find_user_from_warden
 
           present commit_detail, with: Entities::CommitDetail, stats: params[:stats]
         else

lib/gitlab/usage_data.rb

@@ -130,7 +130,7 @@ module Gitlab
 
       def usage_counters
         {
-          web_ide_commits: Gitlab::UsageDataCounters::WebIdeCommitsCounter.total_count
+          web_ide_commits: Gitlab::UsageDataCounters::WebIdeCounter.total_commits_count
         }
       end
 

lib/gitlab/usage_data_counters/redis_counter.rb

@@ -3,17 +3,13 @@
 module Gitlab
   module UsageDataCounters
     module RedisCounter
-      def increment
+      def increment(redis_counter_key)
         Gitlab::Redis::SharedState.with { |redis| redis.incr(redis_counter_key) }
       end
 
-      def total_count
+      def total_count(redis_counter_key)
         Gitlab::Redis::SharedState.with { |redis| redis.get(redis_counter_key).to_i }
       end
-
-      def redis_counter_key
-        raise NotImplementedError
-      end
     end
   end
 end

lib/gitlab/usage_data_counters/web_ide_commits_counter.rb → lib/gitlab/usage_data_counters/web_ide_counter.rb

@@ -2,11 +2,19 @@
 
 module Gitlab
   module UsageDataCounters
-    class WebIdeCommitsCounter
+    class WebIdeCounter
       extend RedisCounter
 
-      def self.redis_counter_key
-        'WEB_IDE_COMMITS_COUNT'
+      COMMITS_COUNT_KEY = 'WEB_IDE_COMMITS_COUNT'
+
+      class << self
+        def increment_commits_count
+          increment(COMMITS_COUNT_KEY)
+        end
+
+        def total_commits_count
+          total_count(COMMITS_COUNT_KEY)
+        end
       end
     end
   end

spec/lib/gitlab/usage_data_counters/redis_counter_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shared_state do
-  context 'when redis_key is not defined' do
-    subject do
-      Class.new.extend(described_class)
-    end
-
-    describe '.increment' do
-      it 'raises a NotImplementedError exception' do
-        expect { subject.increment}.to raise_error(NotImplementedError)
-      end
-    end
-
-    describe '.total_count' do
-      it 'raises a NotImplementedError exception' do
-        expect { subject.total_count}.to raise_error(NotImplementedError)
-      end
-    end
-  end
-
-  context 'when redis_key is defined' do
-    subject do
-      counter_module = described_class
-
-      Class.new do
-        extend counter_module
-
-        def self.redis_counter_key
-          'foo_redis_key'
-        end
-      end
-    end
-
-    describe '.increment' do
-      it 'increments the web ide commits counter by 1' do
-        expect do
-          subject.increment
-        end.to change { subject.total_count }.from(0).to(1)
-      end
-    end
-
-    describe '.total_count' do
-      it 'returns the total amount of web ide commits' do
-        subject.increment
-        subject.increment
-
-        expect(subject.total_count).to eq(2)
-      end
-    end
-  end
-end

spec/lib/gitlab/usage_data_counters/web_ide_counter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::UsageDataCounters::WebIdeCounter, :clean_gitlab_redis_shared_state do
+  describe '.increment_commits_count' do
+    it 'increments the web ide commits counter by 1' do
+      expect do
+        described_class.increment_commits_count
+      end.to change { described_class.total_commits_count }.by(1)
+    end
+  end
+
+  describe '.total_commits_count' do
+    it 'returns the total amount of web ide commits' do
+      2.times { described_class.increment_commits_count }
+
+      expect(described_class.total_commits_count).to eq(2)
+    end
+  end
+end

spec/requests/api/commits_spec.rb

@@ -281,7 +281,7 @@ describe API::Commits do
       end
 
       it 'does not increment the usage counters using access token authentication' do
-        expect(::Gitlab::UsageDataCounters::WebIdeCommitsCounter).not_to receive(:increment)
+        expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count)
 
         post api(url, user), params: valid_c_params
       end