From 4226bfdaa05717a639df506853733f066164f6a8 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 7 Aug 2017 15:36:29 +0000 Subject: [PATCH] Ensure Site obj in global scope is NilSite and never persisted. --- app/models/account.rb | 8 ++++ app/models/nil_site.rb | 59 +++++++++++++++++++++++++++ app/models/site.rb | 1 + app/models/user.rb | 2 +- spec/models/account_spec.rb | 22 ++++++++++ spec/models/nil_site_spec.rb | 78 ++++++++++++++++++++++++++++++++++++ spec/models/site_spec.rb | 14 ++++++- spec/models/user_spec.rb | 12 +++++- 8 files changed, 192 insertions(+), 4 deletions(-) create mode 100644 app/models/nil_site.rb create mode 100644 spec/models/nil_site_spec.rb diff --git a/app/models/account.rb b/app/models/account.rb index 59f11e69ac..715230ae93 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -63,6 +63,14 @@ def self.single_tenant_default end end + # @return [Boolean] whether this Account is the global tenant in a multitenant environment + def self.global_tenant? + # Global tenant only exists when multitenancy is enabled and NOT in test environment + # (Test environment always acts like a single tenant, i.e. tenant switching not possible) + return false unless Settings.multitenancy.enabled && !Rails.env.test? + Apartment::Tenant.default_tenant == Apartment::Tenant.current + end + def solr_endpoint super || NilSolrEndpoint.new end diff --git a/app/models/nil_site.rb b/app/models/nil_site.rb new file mode 100644 index 0000000000..32d790287c --- /dev/null +++ b/app/models/nil_site.rb @@ -0,0 +1,59 @@ +# NilSite is used to represent the Site in the global tenant in a multitenant environment +# (i.e. Sites only exist on individual tenants and never globally) +class NilSite + class << self + # NilSite must be a singleton like Site + attr_writer :instance + def instance + @instance ||= NilSite.new + end + end + + def id + nil + end + + def account + nil + end + + def application_name + nil + end + + def institution_name + nil + end + + def institution_name_full + nil + end + + def reload + NilSite.instance + end + + def update(*) + false + end + + def admin_emails + [] + end + + def admin_emails=(value) + value + end + + def banner_image? + false + end + + def banner_image + nil + end + + def primary_key + nil + end +end diff --git a/app/models/site.rb b/app/models/site.rb index 514b239e74..91acb73935 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -15,6 +15,7 @@ class << self to: :instance def instance + return NilSite.instance if Account.global_tenant? first_or_create end end diff --git a/app/models/user.rb b/app/models/user.rb index d0f961876e..2193bb1a19 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -51,6 +51,6 @@ def groups private def add_default_roles - add_role :admin, Site.instance unless self.class.any? + add_role :admin, Site.instance unless self.class.any? || Account.global_tenant? end end diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index a9e2d92169..e316425060 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -256,4 +256,26 @@ expect(account.admin_emails).to match_array(["newadmin@here.org"]) end end + + describe '#global_tenant?' do + subject { described_class.global_tenant? } + context 'default setting for test environment' do + it { is_expected.to be false } + end + context 'single tenant in production environment' do + before do + allow(Settings.multitenancy).to receive(:enabled).and_return false + allow(Rails.env).to receive(:test?).and_return false + end + it { is_expected.to be false } + end + context 'default tenant in a multitenant production environment' do + before do + allow(Settings.multitenancy).to receive(:enabled).and_return true + allow(Rails.env).to receive(:test?).and_return false + allow(Apartment::Tenant).to receive(:current_tenant).and_return Apartment::Tenant.default_tenant + end + it { is_expected.to be true } + end + end end diff --git a/spec/models/nil_site_spec.rb b/spec/models/nil_site_spec.rb new file mode 100644 index 0000000000..14e4d7cd05 --- /dev/null +++ b/spec/models/nil_site_spec.rb @@ -0,0 +1,78 @@ +require 'spec_helper' + +RSpec.describe NilSite do + let(:instance) { described_class.instance } + + describe "#instance" do + subject { described_class.instance } + # 'instance' should always return same obj (i.e. singleton) + it { is_expected.to be instance } + end + + describe "#id" do + subject { instance.id } + it { is_expected.to be nil } + end + + describe "#account" do + subject { instance.account } + it { is_expected.to be nil } + end + + describe "#application_name" do + subject { instance.application_name } + it { is_expected.to be nil } + end + + describe "#institution_name" do + subject { instance.institution_name } + it { is_expected.to be nil } + end + + describe "#institution_name_full" do + subject { instance.institution_name_full } + it { is_expected.to be nil } + end + + describe "#reload" do + subject { instance.reload } + it { is_expected.to be described_class.instance } + end + + describe "#update" do + subject { instance.update(param: 'one') } + it { is_expected.to be false } + end + + describe "#admin_emails" do + context "default value" do + subject { instance.admin_emails } + it { is_expected.to be_empty } + end + context "set a value" do + before { instance.admin_emails = "test@test.org" } + subject { instance.admin_emails } + it { is_expected.to be_empty } + end + end + + describe "#admin_emails=" do + subject { instance.admin_emails = "one@two.org" } + it { is_expected.to eq("one@two.org") } + end + + describe "#banner_image?" do + subject { instance.banner_image? } + it { is_expected.to be false } + end + + describe "#banner_image" do + subject { instance.banner_image } + it { is_expected.to be nil } + end + + describe "#primary_key" do + subject { instance.primary_key } + it { is_expected.to be nil } + end +end diff --git a/spec/models/site_spec.rb b/spec/models/site_spec.rb index c9d3ce8cfa..15f4ef36db 100644 --- a/spec/models/site_spec.rb +++ b/spec/models/site_spec.rb @@ -4,8 +4,18 @@ let(:admin3) { FactoryGirl.create(:user, email: 'i@was_here.net') } describe ".instance" do - it "is a singleton site" do - expect(described_class.instance).to eq(described_class.instance) + context "on global tenant" do + before do + allow(Account).to receive(:global_tenant?).and_return true + end + it "is a NilSite" do + expect(described_class.instance).to eq(NilSite.instance) + end + end + context "on a specific tenant" do + it "is a singleton site" do + expect(described_class.instance).to eq(described_class.instance) + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4f6c6cdc56..30d77aed32 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,5 +1,15 @@ RSpec.describe User, type: :model do - context 'the first created user' do + context 'the first created user in global tenant' do + subject { FactoryGirl.create(:base_user) } + before do + allow(Account).to receive(:global_tenant?).and_return true + end + it 'does not get the admin role' do + expect(subject).not_to have_role :admin + end + end + + context 'the first created user on a tenant' do subject { FactoryGirl.create(:base_user) } it 'is given the admin role' do expect(subject).to have_role :admin, Site.instance