You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by nir-sopher <gi...@git.apache.org> on 2017/03/15 21:13:03 UTC

[GitHub] incubator-trafficcontrol pull request #371: Tenancy table creation

GitHub user nir-sopher opened a pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371

    Tenancy table creation

    This PR is the first step in adding "org tenancy" to TC. It comes to start and replace PR-322
    It define the tenants table, including an "active" flag as well as "parent-tenant".
    This PR is pre-requisite for adding the tenancy to the Users/CDN/DS tables

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nir-sopher/incubator-trafficcontrol tenancy-table-creation

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafficcontrol/pull/371.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #371
    
----
commit 7c8fab02a56feb1d2cc76ba65c08cee74cc45c03
Author: nir-sopher <ni...@gmail.com>
Date:   2017-03-15T13:42:50Z

    Add 'tenant' table to the DB

commit 5647b27625319a0878eeb037dc79b9bfd4368f4e
Author: nir-sopher <ni...@gmail.com>
Date:   2017-03-15T13:47:28Z

    add the 'tenant' fixture

commit 4ed28007aa02cbbc22ca76d9722b5179e19cd1ee
Author: nir-sopher <ni...@gmail.com>
Date:   2017-03-15T13:50:58Z

    Tenant API

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371#discussion_r106613824
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,247 @@
    +package API::Tenant;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +use JSON;
    +use MojoPlugins::Response;
    +
    +my $finfo = __FILE__ . ":";
    +
    +sub getTenantName {
    +	my $self 		= shift;
    +	my $tenant_id		= shift;
    +	return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('name')->single() : "n/a";
    +}
    +
    +sub isRootTenant {
    +	my $self 	= shift;
    +	my $tenant_id	= shift;
    +	return !defined($self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('parent_id')->single());
    +}
    +
    +sub index {
    +	my $self 	= shift;
    +
    +	my @data;
    +	my $orderby = $self->param('orderby') || "name";
    +	my $rs_data = $self->db->resultset("Tenant")->search( undef, {order_by => 'me.' . $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub show {
    +	my $self = shift;
    +	my $id   = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("Tenant")->search( { 'me.id' => $id });
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub update {
    +	my $self   = shift;
    +	my $id     = $self->param('id');
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	if ( !defined( $params->{name} ) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +	
    +	if ( $params->{name} ne $self->getTenantName($id) ) {
    +	        my $name = $params->{name};
    +		my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +		if ($existing) {
    +			return $self->alert("A tenant with name \"$name\" already exists.");
    +		}	
    +	}	
    +
    +	if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +	
    +	my $is_active = $params->{active};
    +	
    +	if ( !$params->{active} && $self->isRootTenant($id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +
    +	if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
    +		return $self->alert("Only the \"root\" tenant can have no parent.");
    +	}
    +	
    +	my $values = {
    +		name      => $params->{name},
    +		active    => $params->{active},
    +		parent_id => $params->{parentId}
    +	};
    +
    +	my $rs = $tenant->update($values);
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          = $rs->id;
    +		$response->{name}        = $rs->name;
    +		$response->{active}      = \$rs->active;
    +		$response->{parentId}    = $rs->parent_id;
    +		$response->{lastUpdated} = $rs->last_updated;
    +		&log( $self, "Updated Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +		return $self->success( $response, "Tenant update was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant update failed.");
    +	}
    +
    +}
    +
    +
    +sub create {
    +	my $self   = shift;
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{name};
    +	if ( !defined($name) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +
    +	my $parent_id = $params->{parentId};
    +	if ( !defined($parent_id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +
    +	my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +	if ($existing) {
    +		return $self->alert("A tenant with name \"$name\" already exists.");
    +	}
    +
    +	my $is_active = exists($params->{active})? $params->{active} : 0; #optional, if not set use default
    +	
    +	if ( !$is_active && !defined($parent_id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +	my $values = {
    +		name 		=> $params->{name} ,
    +		active		=> $is_active,
    +		parent_id 	=> $params->{parentId}
    +	};
    +
    +	my $insert = $self->db->resultset('Tenant')->create($values);
    +	my $rs = $insert->insert();
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          	= $rs->id;
    +		$response->{name}        	= $rs->name;
    +		$response->{active}        	= \$rs->active;
    +		$response->{parentId}           = $rs->parent_id;
    +		$response->{lastUpdated} 	= $rs->last_updated;
    +
    +		&log( $self, "Created Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +
    +		return $self->success( $response, "Tenant create was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant create failed.");
    +	}
    +
    +}
    +
    +
    +sub delete {
    +	my $self = shift;
    +	my $id     = $self->param('id');
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}	
    +	my $name = $self->db->resultset('Tenant')->search( { id => $id } )->get_column('name')->single();
    +	
    +	my $existing_child = $self->db->resultset('Tenant')->search( { parent_id => $id } )->get_column('name')->first();
    +	if ($existing_child) {
    +		return $self->alert("Tenant '$name' has children tenant(s): e.g '$existing_child'. Please update these tenants and retry.");
    +	}
    +
    +	#The order of the below tests is intentional - allowing UT to cover all cases - TODO(nirs) remove this comment when a full "tenancy" UT is added, including permissions and such (no use in putting effort into it yet)
    +	#TODO(nirs) - add back when making available: my $existing_ds = $self->db->resultset('Deliveryservice')->search( { tenant_id => $id })->get_column('xml_id')->first();
    +	#TODO(nirs) - add back when making available: if ($existing_ds) {
    +	#TODO(nirs) - add back when making available: 	return $self->alert("Tenant '$name' is assign with delivery-services(s): e.g. '$existing_ds'. Please update/delete these delivery-services and retry.");
    +	#TODO(nirs) - add back when making available: }
    +
    +	#TODO(nirs) - add back when making available: my $existing_cdn = $self->db->resultset('Cdn')->search( { tenant_id => $id })->get_column('name')->first();
    --- End diff --
    
    No Problem. 
    Note that you'll need to be careful on the merge when the next 3 PRs are added - as the order of the tests is important for the UT, I hope to fix it later on.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371#discussion_r106541595
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,247 @@
    +package API::Tenant;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +use JSON;
    +use MojoPlugins::Response;
    +
    +my $finfo = __FILE__ . ":";
    +
    +sub getTenantName {
    +	my $self 		= shift;
    +	my $tenant_id		= shift;
    +	return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('name')->single() : "n/a";
    +}
    +
    +sub isRootTenant {
    +	my $self 	= shift;
    +	my $tenant_id	= shift;
    +	return !defined($self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('parent_id')->single());
    +}
    +
    +sub index {
    +	my $self 	= shift;
    +
    +	my @data;
    +	my $orderby = $self->param('orderby') || "name";
    +	my $rs_data = $self->db->resultset("Tenant")->search( undef, {order_by => 'me.' . $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub show {
    +	my $self = shift;
    +	my $id   = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("Tenant")->search( { 'me.id' => $id });
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub update {
    +	my $self   = shift;
    +	my $id     = $self->param('id');
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	if ( !defined( $params->{name} ) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +	
    +	if ( $params->{name} ne $self->getTenantName($id) ) {
    +	        my $name = $params->{name};
    +		my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +		if ($existing) {
    +			return $self->alert("A tenant with name \"$name\" already exists.");
    +		}	
    +	}	
    +
    +	if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +	
    +	my $is_active = $params->{active};
    +	
    +	if ( !$params->{active} && $self->isRootTenant($id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +
    +	if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
    +		return $self->alert("Only the \"root\" tenant can have no parent.");
    +	}
    +	
    +	my $values = {
    +		name      => $params->{name},
    +		active    => $params->{active},
    +		parent_id => $params->{parentId}
    +	};
    +
    +	my $rs = $tenant->update($values);
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          = $rs->id;
    +		$response->{name}        = $rs->name;
    +		$response->{active}      = \$rs->active;
    +		$response->{parentId}    = $rs->parent_id;
    +		$response->{lastUpdated} = $rs->last_updated;
    +		&log( $self, "Updated Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +		return $self->success( $response, "Tenant update was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant update failed.");
    +	}
    +
    +}
    +
    +
    +sub create {
    +	my $self   = shift;
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{name};
    +	if ( !defined($name) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +
    +	my $parent_id = $params->{parentId};
    +	if ( !defined($parent_id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +
    +	my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +	if ($existing) {
    +		return $self->alert("A tenant with name \"$name\" already exists.");
    +	}
    +
    +	my $is_active = exists($params->{active})? $params->{active} : 0; #optional, if not set use default
    +	
    +	if ( !$is_active && !defined($parent_id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +	my $values = {
    +		name 		=> $params->{name} ,
    +		active		=> $is_active,
    +		parent_id 	=> $params->{parentId}
    +	};
    +
    +	my $insert = $self->db->resultset('Tenant')->create($values);
    +	my $rs = $insert->insert();
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          	= $rs->id;
    +		$response->{name}        	= $rs->name;
    +		$response->{active}        	= \$rs->active;
    +		$response->{parentId}           = $rs->parent_id;
    +		$response->{lastUpdated} 	= $rs->last_updated;
    +
    +		&log( $self, "Created Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +
    +		return $self->success( $response, "Tenant create was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant create failed.");
    +	}
    +
    +}
    +
    +
    +sub delete {
    +	my $self = shift;
    +	my $id     = $self->param('id');
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}	
    +	my $name = $self->db->resultset('Tenant')->search( { id => $id } )->get_column('name')->single();
    +	
    +	my $existing_child = $self->db->resultset('Tenant')->search( { parent_id => $id } )->get_column('name')->first();
    +	if ($existing_child) {
    +		return $self->alert("Tenant '$name' has children tenant(s): e.g '$existing_child'. Please update these tenants and retry.");
    +	}
    +
    +	#The order of the below tests is intentional - allowing UT to cover all cases - TODO(nirs) remove this comment when a full "tenancy" UT is added, including permissions and such (no use in putting effort into it yet)
    +	#TODO(nirs) - add back when making available: my $existing_ds = $self->db->resultset('Deliveryservice')->search( { tenant_id => $id })->get_column('xml_id')->first();
    +	#TODO(nirs) - add back when making available: if ($existing_ds) {
    +	#TODO(nirs) - add back when making available: 	return $self->alert("Tenant '$name' is assign with delivery-services(s): e.g. '$existing_ds'. Please update/delete these delivery-services and retry.");
    +	#TODO(nirs) - add back when making available: }
    +
    +	#TODO(nirs) - add back when making available: my $existing_cdn = $self->db->resultset('Cdn')->search( { tenant_id => $id })->get_column('name')->first();
    +	#TODO(nirs) - add back when making available: if ($existing_cdn) {
    +	#TODO(nirs) - add back when making available: 	return $self->alert("Tenant '$name' is assign with CDNs(s): e.g. '$existing_cdn'. Please update/delete these CDNs and retry.");
    +	#TODO(nirs) - add back when making available: }
    +
    +	#TODO(nirs) - add back when making available: my $existing_user = $self->db->resultset('TmUser')->search( { tenant_id => $id })->get_column('username')->first();
    --- End diff --
    
    Can you just remove this part and then it will come in with your PR that adds tenant to user?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371#discussion_r106616105
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,247 @@
    +package API::Tenant;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +use JSON;
    +use MojoPlugins::Response;
    +
    +my $finfo = __FILE__ . ":";
    +
    +sub getTenantName {
    +	my $self 		= shift;
    +	my $tenant_id		= shift;
    +	return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('name')->single() : "n/a";
    +}
    +
    +sub isRootTenant {
    +	my $self 	= shift;
    +	my $tenant_id	= shift;
    +	return !defined($self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('parent_id')->single());
    +}
    +
    +sub index {
    +	my $self 	= shift;
    +
    +	my @data;
    +	my $orderby = $self->param('orderby') || "name";
    +	my $rs_data = $self->db->resultset("Tenant")->search( undef, {order_by => 'me.' . $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub show {
    +	my $self = shift;
    +	my $id   = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("Tenant")->search( { 'me.id' => $id });
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub update {
    +	my $self   = shift;
    +	my $id     = $self->param('id');
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	if ( !defined( $params->{name} ) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +	
    +	if ( $params->{name} ne $self->getTenantName($id) ) {
    +	        my $name = $params->{name};
    +		my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +		if ($existing) {
    +			return $self->alert("A tenant with name \"$name\" already exists.");
    +		}	
    +	}	
    +
    +	if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +	
    +	my $is_active = $params->{active};
    +	
    +	if ( !$params->{active} && $self->isRootTenant($id)) {
    --- End diff --
    
    I do not fully understand your point here. 
    The highlighted block does not come to verify that the parameter is set. It comes to block the ability to disable a "root" tenant - a change that cannot be fixed back without working directly with the DB.
    I also believe "active" should be mandatory. Should I test the the params->{active} "exists"?
    Nir


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #371: [TC-184] Tenancy table creation

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/371
  
    Also, you forgot one thing....documentation. I'll help you get started on that...I'll send a PR your way.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371#discussion_r106575505
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,247 @@
    +package API::Tenant;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +use JSON;
    +use MojoPlugins::Response;
    +
    +my $finfo = __FILE__ . ":";
    +
    +sub getTenantName {
    +	my $self 		= shift;
    +	my $tenant_id		= shift;
    +	return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('name')->single() : "n/a";
    +}
    +
    +sub isRootTenant {
    +	my $self 	= shift;
    +	my $tenant_id	= shift;
    +	return !defined($self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('parent_id')->single());
    +}
    +
    +sub index {
    +	my $self 	= shift;
    +
    +	my @data;
    +	my $orderby = $self->param('orderby') || "name";
    +	my $rs_data = $self->db->resultset("Tenant")->search( undef, {order_by => 'me.' . $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub show {
    +	my $self = shift;
    +	my $id   = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("Tenant")->search( { 'me.id' => $id });
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub update {
    +	my $self   = shift;
    +	my $id     = $self->param('id');
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	if ( !defined( $params->{name} ) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +	
    +	if ( $params->{name} ne $self->getTenantName($id) ) {
    +	        my $name = $params->{name};
    +		my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +		if ($existing) {
    +			return $self->alert("A tenant with name \"$name\" already exists.");
    +		}	
    +	}	
    +
    +	if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +	
    +	my $is_active = $params->{active};
    +	
    +	if ( !$params->{active} && $self->isRootTenant($id)) {
    --- End diff --
    
    maybe you should just make "active" required on update. otherwise, if tenant.active=true (and not root) and I send thru no active parameter, I end up with tenant.active=false. it doesn't seem like a lot to ask of the api consumer to pass thru active=true|false...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371#discussion_r106541559
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,247 @@
    +package API::Tenant;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +use JSON;
    +use MojoPlugins::Response;
    +
    +my $finfo = __FILE__ . ":";
    +
    +sub getTenantName {
    +	my $self 		= shift;
    +	my $tenant_id		= shift;
    +	return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('name')->single() : "n/a";
    +}
    +
    +sub isRootTenant {
    +	my $self 	= shift;
    +	my $tenant_id	= shift;
    +	return !defined($self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('parent_id')->single());
    +}
    +
    +sub index {
    +	my $self 	= shift;
    +
    +	my @data;
    +	my $orderby = $self->param('orderby') || "name";
    +	my $rs_data = $self->db->resultset("Tenant")->search( undef, {order_by => 'me.' . $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub show {
    +	my $self = shift;
    +	my $id   = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("Tenant")->search( { 'me.id' => $id });
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub update {
    +	my $self   = shift;
    +	my $id     = $self->param('id');
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	if ( !defined( $params->{name} ) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +	
    +	if ( $params->{name} ne $self->getTenantName($id) ) {
    +	        my $name = $params->{name};
    +		my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +		if ($existing) {
    +			return $self->alert("A tenant with name \"$name\" already exists.");
    +		}	
    +	}	
    +
    +	if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +	
    +	my $is_active = $params->{active};
    +	
    +	if ( !$params->{active} && $self->isRootTenant($id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +
    +	if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
    +		return $self->alert("Only the \"root\" tenant can have no parent.");
    +	}
    +	
    +	my $values = {
    +		name      => $params->{name},
    +		active    => $params->{active},
    +		parent_id => $params->{parentId}
    +	};
    +
    +	my $rs = $tenant->update($values);
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          = $rs->id;
    +		$response->{name}        = $rs->name;
    +		$response->{active}      = \$rs->active;
    +		$response->{parentId}    = $rs->parent_id;
    +		$response->{lastUpdated} = $rs->last_updated;
    +		&log( $self, "Updated Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +		return $self->success( $response, "Tenant update was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant update failed.");
    +	}
    +
    +}
    +
    +
    +sub create {
    +	my $self   = shift;
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{name};
    +	if ( !defined($name) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +
    +	my $parent_id = $params->{parentId};
    +	if ( !defined($parent_id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +
    +	my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +	if ($existing) {
    +		return $self->alert("A tenant with name \"$name\" already exists.");
    +	}
    +
    +	my $is_active = exists($params->{active})? $params->{active} : 0; #optional, if not set use default
    +	
    +	if ( !$is_active && !defined($parent_id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +	my $values = {
    +		name 		=> $params->{name} ,
    +		active		=> $is_active,
    +		parent_id 	=> $params->{parentId}
    +	};
    +
    +	my $insert = $self->db->resultset('Tenant')->create($values);
    +	my $rs = $insert->insert();
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          	= $rs->id;
    +		$response->{name}        	= $rs->name;
    +		$response->{active}        	= \$rs->active;
    +		$response->{parentId}           = $rs->parent_id;
    +		$response->{lastUpdated} 	= $rs->last_updated;
    +
    +		&log( $self, "Created Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +
    +		return $self->success( $response, "Tenant create was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant create failed.");
    +	}
    +
    +}
    +
    +
    +sub delete {
    +	my $self = shift;
    +	my $id     = $self->param('id');
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}	
    +	my $name = $self->db->resultset('Tenant')->search( { id => $id } )->get_column('name')->single();
    +	
    +	my $existing_child = $self->db->resultset('Tenant')->search( { parent_id => $id } )->get_column('name')->first();
    +	if ($existing_child) {
    +		return $self->alert("Tenant '$name' has children tenant(s): e.g '$existing_child'. Please update these tenants and retry.");
    +	}
    +
    +	#The order of the below tests is intentional - allowing UT to cover all cases - TODO(nirs) remove this comment when a full "tenancy" UT is added, including permissions and such (no use in putting effort into it yet)
    +	#TODO(nirs) - add back when making available: my $existing_ds = $self->db->resultset('Deliveryservice')->search( { tenant_id => $id })->get_column('xml_id')->first();
    +	#TODO(nirs) - add back when making available: if ($existing_ds) {
    +	#TODO(nirs) - add back when making available: 	return $self->alert("Tenant '$name' is assign with delivery-services(s): e.g. '$existing_ds'. Please update/delete these delivery-services and retry.");
    +	#TODO(nirs) - add back when making available: }
    +
    +	#TODO(nirs) - add back when making available: my $existing_cdn = $self->db->resultset('Cdn')->search( { tenant_id => $id })->get_column('name')->first();
    --- End diff --
    
    Can you just remove this part and then it will come in with your PR that adds tenant to cdn?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371#discussion_r106573144
  
    --- Diff: traffic_ops/app/lib/Fixtures/Tenant.pm ---
    @@ -0,0 +1,46 @@
    +package Fixtures::Tenant;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +use Moose;
    +extends 'DBIx::Class::EasyFixture';
    +use namespace::autoclean;
    +use Digest::SHA1 qw(sha1_hex);
    +
    +my %definition_for = (
    +	## id => 1
    --- End diff --
    
    can you get rid of this comment? it's misleading.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #371: [TC-184] Tenancy table creation

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/371
  
    I created a PR into your branch for the documentation - https://github.com/nir-sopher/incubator-trafficcontrol/pull/4
    
    If it looks ok to you, just hit the merge button and my commit will be added to this PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #371: [TC-184] Tenancy table creation

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/371
  
    As far as the "active" flag being required on update, it's no big deal. either was is fine with me. i guess i was just thinking it might be confusing to a user that if I omit the active flag on an update, it would flip tenant.active back to false. 
    
    It makes sense on create - if you don't supply it, it gets its default value of false. but i'm not sure about update...
    
    up to  you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #371: [TC-184] Tenancy table creation

Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/371
  
    I'll add the test, also as all other fields in the tenant module are tested upon update, so it is more consistent.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafficcontrol/pull/371


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371#discussion_r106613763
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,247 @@
    +package API::Tenant;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +use JSON;
    +use MojoPlugins::Response;
    +
    +my $finfo = __FILE__ . ":";
    +
    +sub getTenantName {
    +	my $self 		= shift;
    +	my $tenant_id		= shift;
    +	return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('name')->single() : "n/a";
    +}
    +
    +sub isRootTenant {
    +	my $self 	= shift;
    +	my $tenant_id	= shift;
    +	return !defined($self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('parent_id')->single());
    +}
    +
    +sub index {
    +	my $self 	= shift;
    +
    +	my @data;
    +	my $orderby = $self->param('orderby') || "name";
    +	my $rs_data = $self->db->resultset("Tenant")->search( undef, {order_by => 'me.' . $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub show {
    +	my $self = shift;
    +	my $id   = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("Tenant")->search( { 'me.id' => $id });
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub update {
    +	my $self   = shift;
    +	my $id     = $self->param('id');
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	if ( !defined( $params->{name} ) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +	
    +	if ( $params->{name} ne $self->getTenantName($id) ) {
    +	        my $name = $params->{name};
    +		my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +		if ($existing) {
    +			return $self->alert("A tenant with name \"$name\" already exists.");
    +		}	
    +	}	
    +
    +	if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +	
    +	my $is_active = $params->{active};
    +	
    +	if ( !$params->{active} && $self->isRootTenant($id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +
    +	if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
    +		return $self->alert("Only the \"root\" tenant can have no parent.");
    +	}
    +	
    +	my $values = {
    +		name      => $params->{name},
    +		active    => $params->{active},
    +		parent_id => $params->{parentId}
    +	};
    +
    +	my $rs = $tenant->update($values);
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          = $rs->id;
    +		$response->{name}        = $rs->name;
    +		$response->{active}      = \$rs->active;
    +		$response->{parentId}    = $rs->parent_id;
    +		$response->{lastUpdated} = $rs->last_updated;
    +		&log( $self, "Updated Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +		return $self->success( $response, "Tenant update was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant update failed.");
    +	}
    +
    +}
    +
    +
    +sub create {
    +	my $self   = shift;
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{name};
    +	if ( !defined($name) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +
    +	my $parent_id = $params->{parentId};
    +	if ( !defined($parent_id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +
    +	my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +	if ($existing) {
    +		return $self->alert("A tenant with name \"$name\" already exists.");
    +	}
    +
    +	my $is_active = exists($params->{active})? $params->{active} : 0; #optional, if not set use default
    +	
    +	if ( !$is_active && !defined($parent_id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +	my $values = {
    +		name 		=> $params->{name} ,
    +		active		=> $is_active,
    +		parent_id 	=> $params->{parentId}
    +	};
    +
    +	my $insert = $self->db->resultset('Tenant')->create($values);
    +	my $rs = $insert->insert();
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          	= $rs->id;
    +		$response->{name}        	= $rs->name;
    +		$response->{active}        	= \$rs->active;
    +		$response->{parentId}           = $rs->parent_id;
    +		$response->{lastUpdated} 	= $rs->last_updated;
    +
    +		&log( $self, "Created Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +
    +		return $self->success( $response, "Tenant create was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant create failed.");
    +	}
    +
    +}
    +
    +
    +sub delete {
    +	my $self = shift;
    +	my $id     = $self->param('id');
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}	
    +	my $name = $self->db->resultset('Tenant')->search( { id => $id } )->get_column('name')->single();
    +	
    +	my $existing_child = $self->db->resultset('Tenant')->search( { parent_id => $id } )->get_column('name')->first();
    +	if ($existing_child) {
    +		return $self->alert("Tenant '$name' has children tenant(s): e.g '$existing_child'. Please update these tenants and retry.");
    +	}
    +
    +	#The order of the below tests is intentional - allowing UT to cover all cases - TODO(nirs) remove this comment when a full "tenancy" UT is added, including permissions and such (no use in putting effort into it yet)
    --- End diff --
    
    No Problem. 
    Note that you'll need to be careful on the merge when the next 3 PRs are added - as the order of the tests is important for the UT, I hope to fix it later on.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371#discussion_r106613842
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,247 @@
    +package API::Tenant;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +use JSON;
    +use MojoPlugins::Response;
    +
    +my $finfo = __FILE__ . ":";
    +
    +sub getTenantName {
    +	my $self 		= shift;
    +	my $tenant_id		= shift;
    +	return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('name')->single() : "n/a";
    +}
    +
    +sub isRootTenant {
    +	my $self 	= shift;
    +	my $tenant_id	= shift;
    +	return !defined($self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('parent_id')->single());
    +}
    +
    +sub index {
    +	my $self 	= shift;
    +
    +	my @data;
    +	my $orderby = $self->param('orderby') || "name";
    +	my $rs_data = $self->db->resultset("Tenant")->search( undef, {order_by => 'me.' . $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub show {
    +	my $self = shift;
    +	my $id   = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("Tenant")->search( { 'me.id' => $id });
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub update {
    +	my $self   = shift;
    +	my $id     = $self->param('id');
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	if ( !defined( $params->{name} ) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +	
    +	if ( $params->{name} ne $self->getTenantName($id) ) {
    +	        my $name = $params->{name};
    +		my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +		if ($existing) {
    +			return $self->alert("A tenant with name \"$name\" already exists.");
    +		}	
    +	}	
    +
    +	if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +	
    +	my $is_active = $params->{active};
    +	
    +	if ( !$params->{active} && $self->isRootTenant($id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +
    +	if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
    +		return $self->alert("Only the \"root\" tenant can have no parent.");
    +	}
    +	
    +	my $values = {
    +		name      => $params->{name},
    +		active    => $params->{active},
    +		parent_id => $params->{parentId}
    +	};
    +
    +	my $rs = $tenant->update($values);
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          = $rs->id;
    +		$response->{name}        = $rs->name;
    +		$response->{active}      = \$rs->active;
    +		$response->{parentId}    = $rs->parent_id;
    +		$response->{lastUpdated} = $rs->last_updated;
    +		&log( $self, "Updated Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +		return $self->success( $response, "Tenant update was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant update failed.");
    +	}
    +
    +}
    +
    +
    +sub create {
    +	my $self   = shift;
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{name};
    +	if ( !defined($name) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +
    +	my $parent_id = $params->{parentId};
    +	if ( !defined($parent_id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +
    +	my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +	if ($existing) {
    +		return $self->alert("A tenant with name \"$name\" already exists.");
    +	}
    +
    +	my $is_active = exists($params->{active})? $params->{active} : 0; #optional, if not set use default
    +	
    +	if ( !$is_active && !defined($parent_id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +	my $values = {
    +		name 		=> $params->{name} ,
    +		active		=> $is_active,
    +		parent_id 	=> $params->{parentId}
    +	};
    +
    +	my $insert = $self->db->resultset('Tenant')->create($values);
    +	my $rs = $insert->insert();
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          	= $rs->id;
    +		$response->{name}        	= $rs->name;
    +		$response->{active}        	= \$rs->active;
    +		$response->{parentId}           = $rs->parent_id;
    +		$response->{lastUpdated} 	= $rs->last_updated;
    +
    +		&log( $self, "Created Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +
    +		return $self->success( $response, "Tenant create was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant create failed.");
    +	}
    +
    +}
    +
    +
    +sub delete {
    +	my $self = shift;
    +	my $id     = $self->param('id');
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}	
    +	my $name = $self->db->resultset('Tenant')->search( { id => $id } )->get_column('name')->single();
    +	
    +	my $existing_child = $self->db->resultset('Tenant')->search( { parent_id => $id } )->get_column('name')->first();
    +	if ($existing_child) {
    +		return $self->alert("Tenant '$name' has children tenant(s): e.g '$existing_child'. Please update these tenants and retry.");
    +	}
    +
    +	#The order of the below tests is intentional - allowing UT to cover all cases - TODO(nirs) remove this comment when a full "tenancy" UT is added, including permissions and such (no use in putting effort into it yet)
    +	#TODO(nirs) - add back when making available: my $existing_ds = $self->db->resultset('Deliveryservice')->search( { tenant_id => $id })->get_column('xml_id')->first();
    +	#TODO(nirs) - add back when making available: if ($existing_ds) {
    +	#TODO(nirs) - add back when making available: 	return $self->alert("Tenant '$name' is assign with delivery-services(s): e.g. '$existing_ds'. Please update/delete these delivery-services and retry.");
    +	#TODO(nirs) - add back when making available: }
    +
    +	#TODO(nirs) - add back when making available: my $existing_cdn = $self->db->resultset('Cdn')->search( { tenant_id => $id })->get_column('name')->first();
    +	#TODO(nirs) - add back when making available: if ($existing_cdn) {
    +	#TODO(nirs) - add back when making available: 	return $self->alert("Tenant '$name' is assign with CDNs(s): e.g. '$existing_cdn'. Please update/delete these CDNs and retry.");
    +	#TODO(nirs) - add back when making available: }
    +
    +	#TODO(nirs) - add back when making available: my $existing_user = $self->db->resultset('TmUser')->search( { tenant_id => $id })->get_column('username')->first();
    --- End diff --
    
    No Problem. 
    Note that you'll need to be careful on the merge when the next 3 PRs are added - as the order of the tests is important for the UT, I hope to fix it later on.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #371: [TC-184] Tenancy table creation

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/371#discussion_r106541500
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,247 @@
    +package API::Tenant;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +use JSON;
    +use MojoPlugins::Response;
    +
    +my $finfo = __FILE__ . ":";
    +
    +sub getTenantName {
    +	my $self 		= shift;
    +	my $tenant_id		= shift;
    +	return defined($tenant_id) ? $self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('name')->single() : "n/a";
    +}
    +
    +sub isRootTenant {
    +	my $self 	= shift;
    +	my $tenant_id	= shift;
    +	return !defined($self->db->resultset('Tenant')->search( { id => $tenant_id } )->get_column('parent_id')->single());
    +}
    +
    +sub index {
    +	my $self 	= shift;
    +
    +	my @data;
    +	my $orderby = $self->param('orderby') || "name";
    +	my $rs_data = $self->db->resultset("Tenant")->search( undef, {order_by => 'me.' . $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub show {
    +	my $self = shift;
    +	my $id   = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("Tenant")->search( { 'me.id' => $id });
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"           => $row->id,
    +				"name"         => $row->name,
    +				"active"       => \$row->active,
    +				"parentId"     => $row->parent_id,
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub update {
    +	my $self   = shift;
    +	my $id     = $self->param('id');
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	if ( !defined( $params->{name} ) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +	
    +	if ( $params->{name} ne $self->getTenantName($id) ) {
    +	        my $name = $params->{name};
    +		my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +		if ($existing) {
    +			return $self->alert("A tenant with name \"$name\" already exists.");
    +		}	
    +	}	
    +
    +	if ( !defined( $params->{parentId}) && !$self->isRootTenant($id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +	
    +	my $is_active = $params->{active};
    +	
    +	if ( !$params->{active} && $self->isRootTenant($id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +
    +	if ( !defined($params->{parentId}) && !isRootTenant($id) ) {
    +		return $self->alert("Only the \"root\" tenant can have no parent.");
    +	}
    +	
    +	my $values = {
    +		name      => $params->{name},
    +		active    => $params->{active},
    +		parent_id => $params->{parentId}
    +	};
    +
    +	my $rs = $tenant->update($values);
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          = $rs->id;
    +		$response->{name}        = $rs->name;
    +		$response->{active}      = \$rs->active;
    +		$response->{parentId}    = $rs->parent_id;
    +		$response->{lastUpdated} = $rs->last_updated;
    +		&log( $self, "Updated Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +		return $self->success( $response, "Tenant update was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant update failed.");
    +	}
    +
    +}
    +
    +
    +sub create {
    +	my $self   = shift;
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{name};
    +	if ( !defined($name) ) {
    +		return $self->alert("Tenant name is required.");
    +	}
    +
    +	my $parent_id = $params->{parentId};
    +	if ( !defined($parent_id) ) {
    +		return $self->alert("Parent Id is required.");
    +	}
    +
    +	my $existing = $self->db->resultset('Tenant')->search( { name => $name } )->get_column('name')->single();
    +	if ($existing) {
    +		return $self->alert("A tenant with name \"$name\" already exists.");
    +	}
    +
    +	my $is_active = exists($params->{active})? $params->{active} : 0; #optional, if not set use default
    +	
    +	if ( !$is_active && !defined($parent_id)) {
    +		return $self->alert("Root user cannot be in-active.");
    +	}
    +	
    +	my $values = {
    +		name 		=> $params->{name} ,
    +		active		=> $is_active,
    +		parent_id 	=> $params->{parentId}
    +	};
    +
    +	my $insert = $self->db->resultset('Tenant')->create($values);
    +	my $rs = $insert->insert();
    +	if ($rs) {
    +		my $response;
    +		$response->{id}          	= $rs->id;
    +		$response->{name}        	= $rs->name;
    +		$response->{active}        	= \$rs->active;
    +		$response->{parentId}           = $rs->parent_id;
    +		$response->{lastUpdated} 	= $rs->last_updated;
    +
    +		&log( $self, "Created Tenant name '" . $rs->name . "' for id: " . $rs->id, "APICHANGE" );
    +
    +		return $self->success( $response, "Tenant create was successful." );
    +	}
    +	else {
    +		return $self->alert("Tenant create failed.");
    +	}
    +
    +}
    +
    +
    +sub delete {
    +	my $self = shift;
    +	my $id     = $self->param('id');
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $tenant = $self->db->resultset('Tenant')->find( { id => $id } );
    +	if ( !defined($tenant) ) {
    +		return $self->not_found();
    +	}	
    +	my $name = $self->db->resultset('Tenant')->search( { id => $id } )->get_column('name')->single();
    +	
    +	my $existing_child = $self->db->resultset('Tenant')->search( { parent_id => $id } )->get_column('name')->first();
    +	if ($existing_child) {
    +		return $self->alert("Tenant '$name' has children tenant(s): e.g '$existing_child'. Please update these tenants and retry.");
    +	}
    +
    +	#The order of the below tests is intentional - allowing UT to cover all cases - TODO(nirs) remove this comment when a full "tenancy" UT is added, including permissions and such (no use in putting effort into it yet)
    --- End diff --
    
    Can you just remove this part and then it will come in with your PR that adds tenant to ds?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #371: [TC-184] Tenancy table creation

Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/371
  
    Thanks Jeremy for your comments and help with documentation.
    Most AIs are in.
    Still need to further discuss "Active" input as required comment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---