From 7619c2fa2f445f0c2725f2921377e89e6ef0a580 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan <tgx_world@hotmail.com> Date: Wed, 29 Jun 2016 13:55:17 +0800 Subject: [PATCH 1/2] FIX: Make sure we add a TTL when we enable readonly mode. --- lib/discourse.rb | 6 ++- spec/components/discourse_spec.rb | 85 +++++++++++++++++++------------ 2 files changed, 56 insertions(+), 35 deletions(-) diff --git a/lib/discourse.rb b/lib/discourse.rb index 762c1c0eaa5..fe0684a4b4e 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -215,8 +215,10 @@ module Discourse base_url_no_prefix + base_uri end + READONLY_MODE_KEY_TTL ||= 60 + def self.enable_readonly_mode - $redis.set(readonly_mode_key, 1) + $redis.setex(readonly_mode_key, READONLY_MODE_KEY_TTL, 1) MessageBus.publish(readonly_channel, true) keep_readonly_mode true @@ -226,7 +228,7 @@ module Discourse # extend the expiry by 1 minute every 30 seconds Thread.new do while readonly_mode? - $redis.expire(readonly_mode_key, 1.minute) + $redis.expire(readonly_mode_key, READONLY_MODE_KEY_TTL) sleep 30.seconds end end diff --git a/spec/components/discourse_spec.rb b/spec/components/discourse_spec.rb index b8b108519e0..49948f6c513 100644 --- a/spec/components/discourse_spec.rb +++ b/spec/components/discourse_spec.rb @@ -85,50 +85,69 @@ describe Discourse do end - context "#enable_readonly_mode" do + context 'readonly mode' do + let(:readonly_channel) { Discourse.readonly_channel } + let(:readonly_key) { Discourse.readonly_mode_key } + let(:readonly_mode_ttl) { Discourse::READONLY_MODE_KEY_TTL } - it "adds a key in redis and publish a message through the message bus" do - $redis.expects(:set).with(Discourse.readonly_mode_key, 1) - MessageBus.expects(:publish).with(Discourse.readonly_channel, true) - Discourse.enable_readonly_mode + after do + $redis.del(readonly_key) end - end - - context "#disable_readonly_mode" do - - it "removes a key from redis and publish a message through the message bus" do - $redis.expects(:del).with(Discourse.readonly_mode_key) - MessageBus.expects(:publish).with(Discourse.readonly_channel, false) - Discourse.disable_readonly_mode + def assert_readonly_mode(message, channel, key, ttl) + expect(message.channel).to eq(channel) + expect(message.data).to eq(true) + expect($redis.get(key)).to eq("1") + expect($redis.ttl(key)).to eq(ttl) end - end - - context "#readonly_mode?" do - it "is false by default" do - expect(Discourse.readonly_mode?).to eq(false) + def assert_readonly_mode_disabled(message, channel, key) + expect(message.channel).to eq(channel) + expect(message.data).to eq(false) + expect($redis.get(key)).to eq(nil) end - it "returns true when the key is present in redis" do - begin - $redis.set(Discourse.readonly_mode_key, 1) - expect(Discourse.readonly_mode?).to eq(true) - ensure - $redis.del(Discourse.readonly_mode_key) + context ".enable_readonly_mode" do + it "adds a key in redis and publish a message through the message bus" do + expect($redis.get(readonly_key)).to eq(nil) + message = MessageBus.track_publish { Discourse.enable_readonly_mode }.first + assert_readonly_mode(message, readonly_channel, readonly_key, readonly_mode_ttl) end end - it "returns true when Discourse is recently read only" do - Discourse.received_readonly! - expect(Discourse.readonly_mode?).to eq(true) - end - end + context ".disable_readonly_mode" do + it "removes a key from redis and publish a message through the message bus" do + Discourse.enable_readonly_mode - context ".received_readonly!" do - it "sets the right time" do - time = Discourse.received_readonly! - expect(Discourse.last_read_only['default']).to eq(time) + message = MessageBus.track_publish do + Discourse.disable_readonly_mode + end.first + + assert_readonly_mode_disabled(message, readonly_channel, readonly_key) + end + end + + context ".readonly_mode?" do + it "is false by default" do + expect(Discourse.readonly_mode?).to eq(false) + end + + it "returns true when the key is present in redis" do + $redis.set(readonly_key, 1) + expect(Discourse.readonly_mode?).to eq(true) + end + + it "returns true when Discourse is recently read only" do + Discourse.received_readonly! + expect(Discourse.readonly_mode?).to eq(true) + end + end + + context ".received_readonly!" do + it "sets the right time" do + time = Discourse.received_readonly! + expect(Discourse.last_read_only['default']).to eq(time) + end end end From 64858c10febafecf875a70a3d565b5a05e74e2e8 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan <tgx_world@hotmail.com> Date: Wed, 29 Jun 2016 14:19:18 +0800 Subject: [PATCH 2/2] FIX: Set a not expiring key for user enabled readonly mode. --- app/controllers/admin/backups_controller.rb | 2 +- lib/discourse.rb | 26 +++++++----- spec/components/discourse_spec.rb | 47 ++++++++++++++------- 3 files changed, 48 insertions(+), 27 deletions(-) diff --git a/app/controllers/admin/backups_controller.rb b/app/controllers/admin/backups_controller.rb index 3539e2fa479..b085dc81f68 100644 --- a/app/controllers/admin/backups_controller.rb +++ b/app/controllers/admin/backups_controller.rb @@ -95,7 +95,7 @@ class Admin::BackupsController < Admin::AdminController def readonly enable = params.fetch(:enable).to_s == "true" - enable ? Discourse.enable_readonly_mode : Discourse.disable_readonly_mode + enable ? Discourse.enable_readonly_mode(user_enabled: true) : Discourse.disable_readonly_mode(user_enabled: true) render nothing: true end diff --git a/lib/discourse.rb b/lib/discourse.rb index fe0684a4b4e..3539a7bbfec 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -216,11 +216,18 @@ module Discourse end READONLY_MODE_KEY_TTL ||= 60 + READONLY_MODE_KEY ||= 'readonly_mode'.freeze + USER_READONLY_MODE_KEY ||= 'readonly_mode:user'.freeze + + def self.enable_readonly_mode(user_enabled: false) + if user_enabled + $redis.set(USER_READONLY_MODE_KEY, 1) + else + $redis.setex(READONLY_MODE_KEY, READONLY_MODE_KEY_TTL, 1) + keep_readonly_mode + end - def self.enable_readonly_mode - $redis.setex(readonly_mode_key, READONLY_MODE_KEY_TTL, 1) MessageBus.publish(readonly_channel, true) - keep_readonly_mode true end @@ -228,20 +235,21 @@ module Discourse # extend the expiry by 1 minute every 30 seconds Thread.new do while readonly_mode? - $redis.expire(readonly_mode_key, READONLY_MODE_KEY_TTL) + $redis.expire(READONLY_MODE_KEY, READONLY_MODE_KEY_TTL) sleep 30.seconds end end end - def self.disable_readonly_mode - $redis.del(readonly_mode_key) + def self.disable_readonly_mode(user_enabled: false) + key = user_enabled ? USER_READONLY_MODE_KEY : READONLY_MODE_KEY + $redis.del(key) MessageBus.publish(readonly_channel, false) true end def self.readonly_mode? - recently_readonly? || !!$redis.get(readonly_mode_key) + recently_readonly? || !!$redis.get(READONLY_MODE_KEY) end def self.request_refresh! @@ -310,10 +318,6 @@ module Discourse Rails.configuration.action_controller.asset_host end - def self.readonly_mode_key - "readonly_mode" - end - def self.readonly_channel "/site/read-only" end diff --git a/spec/components/discourse_spec.rb b/spec/components/discourse_spec.rb index 49948f6c513..67499287872 100644 --- a/spec/components/discourse_spec.rb +++ b/spec/components/discourse_spec.rb @@ -86,36 +86,45 @@ describe Discourse do end context 'readonly mode' do - let(:readonly_channel) { Discourse.readonly_channel } - let(:readonly_key) { Discourse.readonly_mode_key } + let(:readonly_mode_key) { Discourse::READONLY_MODE_KEY } let(:readonly_mode_ttl) { Discourse::READONLY_MODE_KEY_TTL } + let(:user_readonly_mode_key) { Discourse::USER_READONLY_MODE_KEY } after do - $redis.del(readonly_key) + $redis.del(readonly_mode_key) + $redis.del(user_readonly_mode_key) end - def assert_readonly_mode(message, channel, key, ttl) - expect(message.channel).to eq(channel) + def assert_readonly_mode(message, key, ttl = -1) + expect(message.channel).to eq(Discourse.readonly_channel) expect(message.data).to eq(true) expect($redis.get(key)).to eq("1") expect($redis.ttl(key)).to eq(ttl) end - def assert_readonly_mode_disabled(message, channel, key) - expect(message.channel).to eq(channel) + def assert_readonly_mode_disabled(message, key) + expect(message.channel).to eq(Discourse.readonly_channel) expect(message.data).to eq(false) expect($redis.get(key)).to eq(nil) end - context ".enable_readonly_mode" do + describe ".enable_readonly_mode" do it "adds a key in redis and publish a message through the message bus" do - expect($redis.get(readonly_key)).to eq(nil) + expect($redis.get(readonly_mode_key)).to eq(nil) message = MessageBus.track_publish { Discourse.enable_readonly_mode }.first - assert_readonly_mode(message, readonly_channel, readonly_key, readonly_mode_ttl) + assert_readonly_mode(message, readonly_mode_key, readonly_mode_ttl) + end + + context 'user enabled readonly mode' do + it "adds a key in redis and publish a message through the message bus" do + expect($redis.get(user_readonly_mode_key)).to eq(nil) + message = MessageBus.track_publish { Discourse.enable_readonly_mode(user_enabled: true) }.first + assert_readonly_mode(message, user_readonly_mode_key) + end end end - context ".disable_readonly_mode" do + describe ".disable_readonly_mode" do it "removes a key from redis and publish a message through the message bus" do Discourse.enable_readonly_mode @@ -123,17 +132,25 @@ describe Discourse do Discourse.disable_readonly_mode end.first - assert_readonly_mode_disabled(message, readonly_channel, readonly_key) + assert_readonly_mode_disabled(message, readonly_mode_key) + end + + context 'user disabled readonly mode' do + it "removes readonly key in redis and publish a message through the message bus" do + Discourse.enable_readonly_mode(user_enabled: true) + message = MessageBus.track_publish { Discourse.disable_readonly_mode(user_enabled: true) }.first + assert_readonly_mode_disabled(message, user_readonly_mode_key) + end end end - context ".readonly_mode?" do + describe ".readonly_mode?" do it "is false by default" do expect(Discourse.readonly_mode?).to eq(false) end it "returns true when the key is present in redis" do - $redis.set(readonly_key, 1) + $redis.set(readonly_mode_key, 1) expect(Discourse.readonly_mode?).to eq(true) end @@ -143,7 +160,7 @@ describe Discourse do end end - context ".received_readonly!" do + describe ".received_readonly!" do it "sets the right time" do time = Discourse.received_readonly! expect(Discourse.last_read_only['default']).to eq(time)