summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTiago Botelho <tiagonbotelho@hotmail.com>2018-10-02 16:33:43 +0100
committerTiago Botelho <tiagonbotelho@hotmail.com>2018-10-03 11:34:48 +0100
commit1e662293e8fc2f2eba9657dd27449e966736a14a (patch)
tree564290907477e5fee4ebabf2ad75fcc44086c938
parent9b2e17ac71ee446da0f34dada41401803af816c7 (diff)
downloadgitlab-ce-1e662293e8fc2f2eba9657dd27449e966736a14a.tar.gz
Implements Web IDE commits counter in Redis
This makes a temporary implementation of the Web IDE commits counter using Redis while https://gitlab.com/gitlab-org/gitlab-ce/issues/52096 is being discussed further for a more generic approach to counters
-rw-r--r--app/models/usage_counters.rb42
-rw-r--r--db/migrate/20180929102611_create_usage_counters.rb13
-rw-r--r--db/schema.rb8
-rw-r--r--lib/api/commits.rb2
-rw-r--r--lib/gitlab/usage_data.rb4
-rw-r--r--lib/gitlab/web_ide_commits_counter.rb17
-rw-r--r--spec/factories/usage_counters.rb6
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml3
-rw-r--r--spec/lib/gitlab/web_ide_commits_counter_spec.rb19
-rw-r--r--spec/models/usage_counters_spec.rb31
-rw-r--r--spec/requests/api/commits_spec.rb4
-rw-r--r--spec/services/projects/usage_counters_increment_service_spec.rb27
12 files changed, 43 insertions, 133 deletions
diff --git a/app/models/usage_counters.rb b/app/models/usage_counters.rb
deleted file mode 100644
index 90f9c2a976e..00000000000
--- a/app/models/usage_counters.rb
+++ /dev/null
@@ -1,42 +0,0 @@
-# frozen_string_literal: true
-
-class UsageCounters < ActiveRecord::Base
- RECORD_LIMIT = 1.freeze
- BY = 1.freeze
- BLACKLIST_ATTRIBUTES = %w(id created_at updated_at).freeze
-
- validate :ensure_only_one, on: :create
-
- default_value_for :web_ide_commits, 0
-
- # This method supports concurrency so that several
- # requests are able to increment the counter without
- # us having inconsistent data
- def increment_counters(attrs)
- # We want to be able to use the service to increment
- # both a single and multiple counters
- attrs = Array(attrs)
-
- attrs_with_by =
- attrs.each_with_object({}) do |attr, hsh|
- hsh[attr] = BY
- end
-
- self.class.update_counters(id, attrs_with_by)
- end
-
- # Every attribute in this table except the blacklisted
- # attributes is a counter
- def totals
- attributes.except(*BLACKLIST_ATTRIBUTES).symbolize_keys
- end
-
- private
-
- # We only want one UsageCounters per instance
- def ensure_only_one
- return unless UsageCounters.count >= RECORD_LIMIT
-
- errors.add(:base, 'There can only be one usage counters record per instance')
- end
-end
diff --git a/db/migrate/20180929102611_create_usage_counters.rb b/db/migrate/20180929102611_create_usage_counters.rb
deleted file mode 100644
index f17486bb513..00000000000
--- a/db/migrate/20180929102611_create_usage_counters.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-# frozen_string_literal: true
-
-class CreateUsageCounters < ActiveRecord::Migration
- DOWNTIME = false
-
- def change
- create_table :usage_counters do |t|
- t.integer :web_ide_commits
-
- t.timestamps_with_timezone null: false
- end
- end
-end
diff --git a/db/schema.rb b/db/schema.rb
index fb57f2452d7..f92d8005dfb 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20180929102611) do
+ActiveRecord::Schema.define(version: 20180917172041) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -2080,12 +2080,6 @@ ActiveRecord::Schema.define(version: 20180929102611) do
add_index "uploads", ["model_id", "model_type"], name: "index_uploads_on_model_id_and_model_type", using: :btree
add_index "uploads", ["uploader", "path"], name: "index_uploads_on_uploader_and_path", using: :btree
- create_table "usage_counters", force: :cascade do |t|
- t.integer "web_ide_commits"
- t.datetime_with_timezone "created_at", null: false
- t.datetime_with_timezone "updated_at", null: false
- end
-
create_table "user_agent_details", force: :cascade do |t|
t.string "user_agent", null: false
t.string "ip_address", null: false
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index e16dd29d138..ec44936d114 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -109,7 +109,7 @@ module API
if result[:status] == :success
commit_detail = user_project.repository.commit(result[:result])
- UsageCounters.first_or_create.increment_counters(:web_ide_commits) if find_user_from_warden
+ Gitlab::WebIdeCommitsCounter.increment if find_user_from_warden
present commit_detail, with: Entities::CommitDetail
else
diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb
index afab36e89dd..5097c3253c9 100644
--- a/lib/gitlab/usage_data.rb
+++ b/lib/gitlab/usage_data.rb
@@ -108,7 +108,9 @@ module Gitlab
end
def usage_counters
- UsageCounters.first_or_create.totals
+ {
+ web_ide_commits: Gitlab::WebIdeCommitsCounter.total_count
+ }
end
def components_usage_data
diff --git a/lib/gitlab/web_ide_commits_counter.rb b/lib/gitlab/web_ide_commits_counter.rb
new file mode 100644
index 00000000000..1cd9b5295b9
--- /dev/null
+++ b/lib/gitlab/web_ide_commits_counter.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module WebIdeCommitsCounter
+ WEB_IDE_COMMITS_KEY = "WEB_IDE_COMMITS_COUNT".freeze
+
+ class << self
+ def increment
+ Gitlab::Redis::SharedState.with { |redis| redis.incr(WEB_IDE_COMMITS_KEY) }
+ end
+
+ def total_count
+ Gitlab::Redis::SharedState.with { |redis| redis.get(WEB_IDE_COMMITS_KEY).to_i }
+ end
+ end
+ end
+end
diff --git a/spec/factories/usage_counters.rb b/spec/factories/usage_counters.rb
deleted file mode 100644
index 23277fd741e..00000000000
--- a/spec/factories/usage_counters.rb
+++ /dev/null
@@ -1,6 +0,0 @@
-# frozen_string_literal: true
-
-FactoryBot.define do
- factory :usage_counters, class: 'UsageCounters' do
- end
-end
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index b2a0afcb827..ec2bdbe22e1 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -330,6 +330,3 @@ resource_label_events:
- merge_request
- epic
- label
-usage_counters:
-- project
-- web_ide_commits
diff --git a/spec/lib/gitlab/web_ide_commits_counter_spec.rb b/spec/lib/gitlab/web_ide_commits_counter_spec.rb
new file mode 100644
index 00000000000..c51889a1c63
--- /dev/null
+++ b/spec/lib/gitlab/web_ide_commits_counter_spec.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::WebIdeCommitsCounter, :clean_gitlab_redis_shared_state do
+ describe '.increment' do
+ it 'increments the web ide commits counter by 1' do
+ expect do
+ described_class.increment
+ end.to change { described_class.total_count }.from(0).to(1)
+ end
+ end
+
+ describe '.total_count' do
+ it 'returns the total amount of web ide commits' do
+ expect(described_class.total_count).to eq(0)
+ end
+ end
+end
diff --git a/spec/models/usage_counters_spec.rb b/spec/models/usage_counters_spec.rb
deleted file mode 100644
index 0adbfe3cef7..00000000000
--- a/spec/models/usage_counters_spec.rb
+++ /dev/null
@@ -1,31 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe UsageCounters do
- let!(:usage_counters) { create(:usage_counters) }
-
- describe 'maximum number of records' do
- it 'allows for one single record to be created' do
- expect do
- described_class.create!
- end.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: There can only be one usage counters record per instance')
- end
- end
-
- describe '#totals' do
- subject { usage_counters.totals }
-
- it 'returns counters' do
- is_expected.to include(web_ide_commits: 0)
- end
- end
-
- describe '#increment_counters' do
- it 'increments specified counters by 1' do
- expect do
- usage_counters.increment_counters(:web_ide_commits)
- end.to change { usage_counters.reload.web_ide_commits }.from(0).to(1)
- end
- end
-end
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index aebc24dd9a2..06ccf383362 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -279,9 +279,9 @@ describe API::Commits do
end
it 'does not increment the usage counters using access token authentication' do
- post api(url, user), valid_c_params
+ expect(::Gitlab::WebIdeCommitsCounter).not_to receive(:increment)
- expect_any_instance_of(::UsageCounters).not_to receive(:increment_counters)
+ post api(url, user), valid_c_params
end
it 'a new file in project repo' do
diff --git a/spec/services/projects/usage_counters_increment_service_spec.rb b/spec/services/projects/usage_counters_increment_service_spec.rb
deleted file mode 100644
index d82a7337665..00000000000
--- a/spec/services/projects/usage_counters_increment_service_spec.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Projects::UsageCountersIncrementService do
- let(:project) { create(:usage_counters) }
-
- subject(:service) { described_class.new(project) }
-
- context '#execute' do
- context 'when single attribute is passed' do
- it 'increments attribute' do
- expect do
- service.execute(:web_ide_commits)
- end.to change { project.usage_counters.reload.web_ide_commits }.from(0).to(1)
- end
- end
-
- context 'when array is passed' do
- it 'increments specified attributes' do
- expect do
- service.execute(%i(web_ide_commits))
- end.to change { project.usage_counters.reload.web_ide_commits }.from(0).to(1)
- end
- end
- end
-end