Commit 4b8e33a8 authored by Alexandru Croitor's avatar Alexandru Croitor

Exclude ProjectNamespace from Namespace fetches

Currently Namespace are not meant to include anything other
than type `Group`, `User` or `NULL`. By introducing
`ProjectNamespace` we'll break this assumption in multiple places
so we need to filter out `ProjectNamespace` when fetching
namespaces.
parent ebd17d9f
...@@ -11,6 +11,9 @@ RSpec.describe API::Namespaces do ...@@ -11,6 +11,9 @@ RSpec.describe API::Namespaces do
let_it_be(:group1, reload: true) { create(:group, name: 'test.test-group.2') } let_it_be(:group1, reload: true) { create(:group, name: 'test.test-group.2') }
let_it_be(:group2) { create(:group, :nested) } let_it_be(:group2) { create(:group, :nested) }
let_it_be(:ultimate_plan) { create(:ultimate_plan) } let_it_be(:ultimate_plan) { create(:ultimate_plan) }
let_it_be(:project) { create(:project, namespace: group2) }
let_it_be(:project) { create(:project, namespace: group2, name: group2.name, path: group2.path) }
let_it_be(:project_namespace) { create(:project_namespace, project: project) }
describe "GET /namespaces" do describe "GET /namespaces" do
context "when authenticated as admin" do context "when authenticated as admin" do
...@@ -324,6 +327,15 @@ RSpec.describe API::Namespaces do ...@@ -324,6 +327,15 @@ RSpec.describe API::Namespaces do
end end
end end
context 'when project namespace is passed' do
it 'returns 404' do
put api("/namespaces/#{project_namespace.id}", admin), params: params
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response).to eq('message' => '404 Namespace Not Found')
end
end
context 'when invalid params' do context 'when invalid params' do
where(:attr) do where(:attr) do
[ [
...@@ -485,6 +497,28 @@ RSpec.describe API::Namespaces do ...@@ -485,6 +497,28 @@ RSpec.describe API::Namespaces do
expect(group1.gitlab_subscription).to be_present expect(group1.gitlab_subscription).to be_present
end end
end end
context 'when namespace does not exist' do
it 'creates a subscription using full_path when the namespace path contains dots' do
post api("/namespaces/#{non_existing_record_id}/gitlab_subscription", admin), params: params
aggregate_failures do
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response).to eq('message' => '404 Namespace Not Found')
end
end
end
context 'when creating subscription for project namespace' do
it 'creates a subscription using full_path when the namespace path contains dots' do
post api("/namespaces/#{project_namespace.id}/gitlab_subscription", admin), params: params
aggregate_failures do
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response).to eq('message' => '404 Namespace Not Found')
end
end
end
end end
end end
...@@ -538,6 +572,15 @@ RSpec.describe API::Namespaces do ...@@ -538,6 +572,15 @@ RSpec.describe API::Namespaces do
expect(json_response['billing'].keys).to match_array(%w[subscription_start_date subscription_end_date trial_ends_on]) expect(json_response['billing'].keys).to match_array(%w[subscription_start_date subscription_end_date trial_ends_on])
end end
end end
context 'when namespace is a project namespace' do
it 'returns a 404 error' do
get api("/namespaces/#{project_namespace.id}/gitlab_subscription", admin)
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response).to eq('message' => '404 Namespace Not Found')
end
end
end end
describe 'PUT :id/gitlab_subscription' do describe 'PUT :id/gitlab_subscription' do
...@@ -585,6 +628,15 @@ RSpec.describe API::Namespaces do ...@@ -585,6 +628,15 @@ RSpec.describe API::Namespaces do
end end
end end
context 'when namespace is a project namespace' do
it 'returns a 404 error' do
do_put(project_namespace.id, admin, params)
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response).to eq('message' => '404 Namespace Not Found')
end
end
context 'when params are invalid' do context 'when params are invalid' do
it 'returns a 400 error' do it 'returns a 400 error' do
do_put(namespace.id, admin, params.merge(seats: nil)) do_put(namespace.id, admin, params.merge(seats: nil))
......
...@@ -174,9 +174,9 @@ module API ...@@ -174,9 +174,9 @@ module API
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def find_namespace(id) def find_namespace(id)
if id.to_s =~ /^\d+$/ if id.to_s =~ /^\d+$/
Namespace.find_by(id: id) Namespace.without_project_namespaces.find_by(id: id)
else else
Namespace.find_by_full_path(id) find_namespace_by_path(id)
end end
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
...@@ -186,7 +186,7 @@ module API ...@@ -186,7 +186,7 @@ module API
end end
def find_namespace_by_path(path) def find_namespace_by_path(path)
Namespace.find_by_full_path(path) Namespace.without_project_namespaces.find_by_full_path(path)
end end
def find_namespace_by_path!(path) def find_namespace_by_path!(path)
......
...@@ -37,7 +37,7 @@ module API ...@@ -37,7 +37,7 @@ module API
namespaces = current_user.admin ? Namespace.all : current_user.namespaces(owned_only: owned_only) namespaces = current_user.admin ? Namespace.all : current_user.namespaces(owned_only: owned_only)
namespaces = namespaces.include_route namespaces = namespaces.without_project_namespaces.include_route
namespaces = namespaces.include_gitlab_subscription_with_hosted_plan if Gitlab.ee? namespaces = namespaces.include_gitlab_subscription_with_hosted_plan if Gitlab.ee?
......
...@@ -3,10 +3,12 @@ ...@@ -3,10 +3,12 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe API::Namespaces do RSpec.describe API::Namespaces do
let(:admin) { create(:admin) } let_it_be(:admin) { create(:admin) }
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let!(:group1) { create(:group, name: 'group.one') } let_it_be(:group1) { create(:group, name: 'group.one') }
let!(:group2) { create(:group, :nested) } let_it_be(:group2) { create(:group, :nested) }
let_it_be(:project) { create(:project, namespace: group2, name: group2.name, path: group2.path) }
let_it_be(:project_namespace) { create(:project_namespace, project: project) }
describe "GET /namespaces" do describe "GET /namespaces" do
context "when unauthenticated" do context "when unauthenticated" do
...@@ -26,7 +28,7 @@ RSpec.describe API::Namespaces do ...@@ -26,7 +28,7 @@ RSpec.describe API::Namespaces do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_pagination_headers expect(response).to include_pagination_headers
expect(group_kind_json_response.keys).to include('id', 'kind', 'name', 'path', 'full_path', expect(group_kind_json_response.keys).to include('id', 'kind', 'name', 'path', 'full_path',
'parent_id', 'members_count_with_descendants') 'parent_id', 'members_count_with_descendants')
expect(user_kind_json_response.keys).to include('id', 'kind', 'name', 'path', 'full_path', 'parent_id') expect(user_kind_json_response.keys).to include('id', 'kind', 'name', 'path', 'full_path', 'parent_id')
end end
...@@ -37,7 +39,8 @@ RSpec.describe API::Namespaces do ...@@ -37,7 +39,8 @@ RSpec.describe API::Namespaces do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_pagination_headers expect(response).to include_pagination_headers
expect(json_response).to be_an Array expect(json_response).to be_an Array
expect(json_response.length).to eq(Namespace.count) # project namespace is excluded
expect(json_response.length).to eq(Namespace.count - 1)
end end
it "admin: returns an array of matched namespaces" do it "admin: returns an array of matched namespaces" do
...@@ -61,7 +64,7 @@ RSpec.describe API::Namespaces do ...@@ -61,7 +64,7 @@ RSpec.describe API::Namespaces do
owned_group_response = json_response.find { |resource| resource['id'] == group1.id } owned_group_response = json_response.find { |resource| resource['id'] == group1.id }
expect(owned_group_response.keys).to include('id', 'kind', 'name', 'path', 'full_path', expect(owned_group_response.keys).to include('id', 'kind', 'name', 'path', 'full_path',
'parent_id', 'members_count_with_descendants') 'parent_id', 'members_count_with_descendants')
end end
it "returns correct attributes when user cannot admin group" do it "returns correct attributes when user cannot admin group" do
...@@ -109,7 +112,8 @@ RSpec.describe API::Namespaces do ...@@ -109,7 +112,8 @@ RSpec.describe API::Namespaces do
describe 'GET /namespaces/:id' do describe 'GET /namespaces/:id' do
let(:owned_group) { group1 } let(:owned_group) { group1 }
let(:user2) { create(:user) }
let_it_be(:user2) { create(:user) }
shared_examples 'can access namespace' do shared_examples 'can access namespace' do
it 'returns namespace details' do it 'returns namespace details' do
...@@ -144,6 +148,16 @@ RSpec.describe API::Namespaces do ...@@ -144,6 +148,16 @@ RSpec.describe API::Namespaces do
it_behaves_like 'can access namespace' it_behaves_like 'can access namespace'
end end
context 'when requesting project_namespace' do
let(:namespace_id) { project_namespace.id }
it 'returns not-found' do
get api("/namespaces/#{namespace_id}", request_actor)
expect(response).to have_gitlab_http_status(:not_found)
end
end
end end
context 'when requested by path' do context 'when requested by path' do
...@@ -159,6 +173,16 @@ RSpec.describe API::Namespaces do ...@@ -159,6 +173,16 @@ RSpec.describe API::Namespaces do
it_behaves_like 'can access namespace' it_behaves_like 'can access namespace'
end end
context 'when requesting project_namespace' do
let(:namespace_id) { project_namespace.full_path }
it 'returns not-found' do
get api("/namespaces/#{namespace_id}", request_actor)
expect(response).to have_gitlab_http_status(:not_found)
end
end
end end
end end
...@@ -177,6 +201,12 @@ RSpec.describe API::Namespaces do ...@@ -177,6 +201,12 @@ RSpec.describe API::Namespaces do
expect(response).to have_gitlab_http_status(:unauthorized) expect(response).to have_gitlab_http_status(:unauthorized)
end end
it 'returns authentication error' do
get api("/namespaces/#{project_namespace.id}")
expect(response).to have_gitlab_http_status(:unauthorized)
end
end end
context 'when authenticated as regular user' do context 'when authenticated as regular user' do
...@@ -231,10 +261,10 @@ RSpec.describe API::Namespaces do ...@@ -231,10 +261,10 @@ RSpec.describe API::Namespaces do
end end
describe 'GET /namespaces/:namespace/exists' do describe 'GET /namespaces/:namespace/exists' do
let!(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') } let_it_be(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') }
let!(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') } let_it_be(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') }
let!(:namespace1sub) { create(:group, name: 'Sub Namespace 1', path: 'sub-namespace-1', parent: namespace1) } let_it_be(:namespace1sub) { create(:group, name: 'Sub Namespace 1', path: 'sub-namespace-1', parent: namespace1) }
let!(:namespace2sub) { create(:group, name: 'Sub Namespace 2', path: 'sub-namespace-2', parent: namespace2) } let_it_be(:namespace2sub) { create(:group, name: 'Sub Namespace 2', path: 'sub-namespace-2', parent: namespace2) }
context 'when unauthenticated' do context 'when unauthenticated' do
it 'returns authentication error' do it 'returns authentication error' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment