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/02/28 19:14:35 UTC

[GitHub] incubator-trafficcontrol pull request #322: Adding tenancy to the database

GitHub user nir-sopher opened a pull request:

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

    Adding tenancy to the database

    WIP
    On this PR the tenancy is only added to the relavant tables. The already existing APIs were not modified.
    Default tenancy of items is "NULL".
    API for tenancy was added, including UT.
    Additionally, an "upgrade" script was added to create the root tenant and assign it to the specified admin user.
    
    LAST - a "add user" API was added, as well as a simple test - this is the last commit in the PR

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

    $ git pull https://github.com/nir-sopher/incubator-trafficcontrol org-tenancy-active

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

    https://github.com/apache/incubator-trafficcontrol/pull/322.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 #322
    
----

----


---
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 #322: Adding tenancy to the database

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/322#discussion_r103535276
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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;
    --- End diff --
    
    ensure this returns a true|false in the api. you might have to cast it like \$row->active,


---
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 #322: Adding tenancy to the database

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/322#discussion_r103532294
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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;
    --- End diff --
    
    ensure this returns true|false in the 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 #322: Adding tenancy to the database

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/322#discussion_r103788980
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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 {
    --- End diff --
    
    Ack, will add verifications


---
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 #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322#discussion_r103572789
  
    --- Diff: traffic_ops/app/t/api/1.2/tenant.t ---
    @@ -0,0 +1,156 @@
    +package main;
    +#
    +#
    +# 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 Mojo::Base -strict;
    +use Test::More;
    +use Test::Mojo;
    +use DBI;
    +use strict;
    +use warnings;
    +no warnings 'once';
    +use warnings 'all';
    +use Test::TestHelper;
    +
    +#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
    +#no_transactions=>0 ==> delete fixtures after every execution
    +
    +BEGIN { $ENV{MOJO_MODE} = "test" }
    +
    +my $schema = Schema->connect_to_database;
    +my $dbh    = Schema->database_handle;
    +my $t      = Test::Mojo->new('TrafficOps');
    +
    +my $false = 0;
    --- End diff --
    
    However,  it's considered bad style in Perl because there are several values for a "false" value (and many more for "true"...).  Just one example:
    
    ```
    my $false = 0;
    if ($self->active == $false) {
    }
    ```
    The above will be incorrect if `$self->active` is undef.


---
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 #322: Adding tenancy to the database

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/322#discussion_r104487532
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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,
    --- End diff --
    
    need to cast boolean


---
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 #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322
  
    These columns are not relevant after downgrade (until next upgrade).
    What is the common practice in such cases in TC?
    
    On Mar 6, 2017 8:25 PM, "Jeremy Mitchell" <no...@github.com> wrote:
    
    > *@mitchell852* commented on this pull request.
    > ------------------------------
    >
    > In traffic_ops/app/db/migrations/20170221000001_initial_tenancy.sql
    > <https://github.com/apache/incubator-trafficcontrol/pull/322#discussion_r104485020>
    > :
    >
    > > +
    > +-- +goose Down
    > +-- SQL section 'Down' is executed when this migration is rolled back
    > +
    > +ALTER TABLE deliveryservice
    > +DROP COLUMN tenant_id;
    > +
    > +ALTER TABLE cdn
    > +DROP COLUMN tenant_id;
    > +
    > +ALTER TABLE tm_user
    > +DROP COLUMN tenant_id;
    > +
    > +DROP TRIGGER on_update_current_timestamp ON tenant;
    > +
    > +DROP TABLE tenant;
    >
    > do you need to drop the 3 indexes you created as well? in the goose down?
    >
    > \u2014
    > You are receiving this because you authored the thread.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/incubator-trafficcontrol/pull/322#pullrequestreview-25325057>,
    > or mute the thread
    > <https://github.com/notifications/unsubscribe-auth/AXGkYIkLacy8AbJWaRpq04DThqNm7P-Nks5rjE-fgaJpZM4MOyR9>
    > .
    >



---
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 #322: Adding tenancy to the database

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/322#discussion_r103547967
  
    --- Diff: traffic_ops/app/lib/API/User.pm ---
    @@ -218,6 +218,118 @@ sub update {
     
     }
     
    +# Create
    +sub create {
    +	my $self = shift;
    +	my $params = $self->req->json;
    +	
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{username};
    +	if ( !defined($name) ) {
    +		return $self->alert("Username is required.");
    +	}
    +	
    +	my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
    +	if ($existing) {
    +		return $self->alert("A user with username \"$name\" already exists.");
    +	}
    +
    +
    +	if ( !defined($params->{fullName}) ) {
    +		return $self->alert("full-name is required.");
    +	}
    +
    +	if ( !defined($params->{email}) ) {
    +		return $self->alert("email is required.");
    +	}
    +	
    +	if ( !defined($params->{localPassword}) ) {
    +		return $self->alert("local-password is required.");
    +	}
    +
    +	if ( !defined($params->{confirmLocalPassword}) ) {
    +		return $self->alert("confirm-local-password is required.");
    +	}
    +
    +	if ($params->{localPassword} ne $params->{confirmLocalPassword}){
    +		return $self->alert("local-password and confirmed-local-password mismatch.");
    +	}
    +	
    +	if ( !defined($params->{role}) ) {
    +		return $self->alert("role is required.");
    +	}
    +	
    +	
    +
    +	my $values = {
    +		address_line1 			=> defined_or_default($params->{addressLine1}, ""),
    +		address_line2 			=> defined_or_default($params->{addressLine2}, ""),
    +		city 				=> defined_or_default($params->{city}, ""),
    +		company 			=> defined_or_default($params->{company}, ""),
    +		country 			=> defined_or_default($params->{country}, ""),
    +		email 				=> $params->{email},
    +		full_name 			=> $params->{fullName},
    +		new_user 			=> ( $params->{newUser} ) ? 1 : 0,
    +		phone_number 			=> defined_or_default($params->{phoneNumber}, ""),
    +		postal_code 			=> defined_or_default($params->{postalCode}, ""),
    +		public_ssh_key 			=> defined_or_default($params->{publicSshKey}, ""),
    +		registration_sent 		=> defined_or_default( $params->{registrationSent}, undef),
    +		role 				=> $params->{role},
    +		state_or_province 		=> defined_or_default($params->{stateOrProvince}, ""),
    +		username 			=> $params->{username},
    +		new_user            		=> defined_or_default($params->{newUser}, 0),		
    +		uid                  		=> defined_or_default($params->{uid}, 0),		
    --- End diff --
    
    OK. Will remove it when I update this PR in the future


---
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 #322: Adding tenancy to the database

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/322#discussion_r103570488
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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 {
    --- End diff --
    
    i think deleting a tenant is an important feature. helps with cleanup.


---
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 #322: Adding tenancy to the database

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/322#discussion_r104485020
  
    --- Diff: traffic_ops/app/db/migrations/20170221000001_initial_tenancy.sql ---
    @@ -0,0 +1,65 @@
    +/*
    +
    +    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.
    +*/
    +
    +-- +goose Up
    +-- SQL in section 'Up' is executed when this migration is applied
    +
    +
    +-- tenant
    +CREATE TABLE tenant (
    +    id BIGSERIAL primary key NOT NULL,
    +    name text UNIQUE NOT NULL,
    +    active boolean NOT NULL DEFAULT false,
    +    parent_id bigint DEFAULT 1 CHECK (id != parent_id),
    +    CONSTRAINT fk_parentid FOREIGN KEY (parent_id) REFERENCES tenant(id),    
    +    last_updated timestamp with time zone DEFAULT now()
    +); 
    +CREATE INDEX idx_k_tenant_parent_tenant_idx ON tenant USING btree (parent_id);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON tenant FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +ALTER TABLE tm_user
    +    ADD tenant_id BIGINT,
    +    ADD CONSTRAINT fk_tenantid FOREIGN KEY (tenant_id) REFERENCES tenant (id) MATCH FULL,
    +    ALTER COLUMN tenant_id SET DEFAULT NULL;
    +CREATE INDEX idx_k_tm_user_tenant_idx ON tm_user USING btree (tenant_id);
    +
    +ALTER TABLE cdn
    +    ADD tenant_id BIGINT,
    +    ADD CONSTRAINT fk_tenantid FOREIGN KEY (tenant_id) REFERENCES tenant (id) MATCH FULL,
    +    ALTER COLUMN tenant_id SET DEFAULT NULL;
    +CREATE INDEX idx_k_cdn_tenant_idx ON cdn USING btree (tenant_id);
    +
    +ALTER TABLE deliveryservice
    +    ADD tenant_id BIGINT,
    +    ADD CONSTRAINT fk_tenantid FOREIGN KEY (tenant_id) REFERENCES tenant (id) MATCH FULL,
    +    ALTER COLUMN tenant_id SET DEFAULT NULL;
    +CREATE INDEX idx_k_deliveryservice_tenant_idx ON deliveryservice USING btree (tenant_id);
    +
    +-- +goose Down
    +-- SQL section 'Down' is executed when this migration is rolled back
    +
    +ALTER TABLE deliveryservice
    +DROP COLUMN tenant_id;
    +
    +ALTER TABLE cdn
    +DROP COLUMN tenant_id;
    +
    +ALTER TABLE tm_user
    +DROP COLUMN tenant_id;
    +
    +DROP TRIGGER on_update_current_timestamp ON tenant;
    +
    +DROP TABLE tenant;
    --- End diff --
    
    do you need to drop the 3 indexes you created as well? in the goose down?



---
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 #322: Adding tenancy to the database

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/322#discussion_r103531928
  
    --- Diff: traffic_ops/app/t/api/1.2/tenant.t ---
    @@ -0,0 +1,156 @@
    +package main;
    +#
    +#
    +# 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 Mojo::Base -strict;
    +use Test::More;
    +use Test::Mojo;
    +use DBI;
    +use strict;
    +use warnings;
    +no warnings 'once';
    +use warnings 'all';
    +use Test::TestHelper;
    +
    +#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
    +#no_transactions=>0 ==> delete fixtures after every execution
    +
    +BEGIN { $ENV{MOJO_MODE} = "test" }
    +
    +my $schema = Schema->connect_to_database;
    +my $dbh    = Schema->database_handle;
    +my $t      = Test::Mojo->new('TrafficOps');
    +
    +my $false = 0;
    --- End diff --
    
    I wouldn't use variables. 0 and 1 should be fine.


---
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 #322: Adding tenancy to the database

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/322#discussion_r103531473
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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,
    --- End diff --
    
    I think you have to "cast" the active flag to ensure that true|false shows in the api as opposed to 0|1


---
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 #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322
  
    can this PR be closed?


---
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 #322: Adding tenancy to the database

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/322#discussion_r103531624
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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,
    --- End diff --
    
    so...."active"       => \\$row->active,


---
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 #322: Adding tenancy to the database

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/322#discussion_r103535340
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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;
    --- End diff --
    
    ensure this returns a true|false in the api. you might have to cast it like \$row->active,


---
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 #322: Adding tenancy to the database

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/322#discussion_r103532650
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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 {
    --- End diff --
    
    do you want to check here that you can't delete a tenant that has a child tenant? or check that you can't delete a tenant that is assigned to a ds or a cdn? and return a 400. rather than letting the db return a 500?


---
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 #322: Adding tenancy to the database

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/322#discussion_r103548410
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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 {
    --- End diff --
    
    Added to my list - in the future will further test relations instead of counting on the DB


---
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 #322: Adding tenancy to the database

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/322#discussion_r103548305
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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;
    --- End diff --
    
    \ again


---
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 #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322
  
    Almost all changes are already added by other PRs.
    Closing


---
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 #322: Adding tenancy to the database

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/322#discussion_r103548618
  
    --- Diff: traffic_ops/app/t/api/1.2/tenant.t ---
    @@ -0,0 +1,156 @@
    +package main;
    +#
    +#
    +# 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 Mojo::Base -strict;
    +use Test::More;
    +use Test::Mojo;
    +use DBI;
    +use strict;
    +use warnings;
    +no warnings 'once';
    +use warnings 'all';
    +use Test::TestHelper;
    +
    +#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
    +#no_transactions=>0 ==> delete fixtures after every execution
    +
    +BEGIN { $ENV{MOJO_MODE} = "test" }
    +
    +my $schema = Schema->connect_to_database;
    +my $dbh    = Schema->database_handle;
    +my $t      = Test::Mojo->new('TrafficOps');
    +
    +my $false = 0;
    --- End diff --
    
    I prefer it like this for readability


---
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 #322: Adding tenancy to the database

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/322#discussion_r103535234
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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,
    --- End diff --
    
    ensure this returns a true|false in the api. you might have to cast it like \$row->active,


---
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 #322: Adding tenancy to the database

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/322#discussion_r103535521
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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 {
    --- End diff --
    
    can you delete a tenant that:
    - has child tenants
    - is assigned to a user
    - is assigned to a ds
    - is assigned to a cdn
    
    if not, you want to throw 400's for that?


---
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 #322: Adding tenancy to the database

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/322#discussion_r103548284
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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;
    --- End diff --
    
    \ again


---
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 #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322
  
    @nir-sopher - let me know when you get this done and i'll review again - 
    
    0. Changes following your remarks done
    1. Delivery service API - add tenancy


---
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 #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322
  
    TO DO list (10x Jeremy!)
    remove uid & gid from the add: will do in a future commit
    true\false in the API
    Verify no dependancy for a deleted tenant


---
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 #322: Adding tenancy to the database

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/322#discussion_r103531735
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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,
    --- End diff --
    
    same thing with active casting 


---
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 #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322
  
    Current status:
    All UTs passed.
    First batch of PR remarks was fixed.
    The API supports tenancy for CDn / Delivery Service / Users
    UTs are added to cover this functionality.
    
    Next, I will try to break this PR into smaller PRs.
    
    Main gap: tenancy verification upon operation. This is for another 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 pull request #322: Adding tenancy to the database

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/322#discussion_r104491526
  
    --- Diff: traffic_ops/app/lib/API/User.pm ---
    @@ -218,6 +226,121 @@ sub update {
     
     }
     
    +# Create
    +sub create {
    +	my $self = shift;
    +	my $params = $self->req->json;
    +	
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{username};
    +	if ( !defined($name) ) {
    +		return $self->alert("Username is required.");
    +	}
    +	
    +	my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
    +	if ($existing) {
    +		return $self->alert("A user with username \"$name\" already exists.");
    +	}
    +
    +
    +	if ( !defined($params->{fullName}) ) {
    --- End diff --
    
    this is probably not required in my opinion


---
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 #322: Adding tenancy to the database

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/322#discussion_r103534895
  
    --- Diff: traffic_ops/app/t/api/1.2/tenant.t ---
    @@ -0,0 +1,156 @@
    +package main;
    +#
    +#
    +# 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 Mojo::Base -strict;
    +use Test::More;
    +use Test::Mojo;
    +use DBI;
    +use strict;
    +use warnings;
    +no warnings 'once';
    +use warnings 'all';
    +use Test::TestHelper;
    +
    +#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
    +#no_transactions=>0 ==> delete fixtures after every execution
    +
    +BEGIN { $ENV{MOJO_MODE} = "test" }
    +
    +my $schema = Schema->connect_to_database;
    +my $dbh    = Schema->database_handle;
    +my $t      = Test::Mojo->new('TrafficOps');
    +
    +my $false = 0;
    +my $true = 1;
    +
    +Test::TestHelper->unload_core_data($schema);
    +Test::TestHelper->load_core_data($schema);
    +
    +ok $t->post_ok( '/login', => form => { u => Test::TestHelper::ADMIN_USER, p => Test::TestHelper::ADMIN_USER_PASSWORD } )->status_is(302)
    +	->or( sub { diag $t->tx->res->content->asset->{content}; } ), 'Should login?';
    +
    +#verifying the basic cfg
    +$t->get_ok("/api/1.2/tenants")->status_is(200)->json_is( "/response/0/name", "root" )->or( sub { diag $t->tx->res->content->asset->{content}; } );;
    +
    +my $root_tenant_id = &get_tenant_id('root');
    +
    +#setting with no "active" field which is optional
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantA", "parentId" => $root_tenant_id })->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +	->json_is( "/response/name" => "tenantA" )
    +	->json_is( "/response/active" =>  $false)
    +	->json_is( "/response/parentId" =>  $root_tenant_id)
    +            , 'Does the tenant details return?';
    +
    +#same name - would not accept
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantA", "active" => $true, "parentId" => $root_tenant_id })->status_is(400);
    +
    +#no name - would not accept
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "parentId" => $root_tenant_id })->status_is(400);
    +
    +#no parent - would not accept
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantB" })->status_is(400);
    +
    +my $tenantA_id = &get_tenant_id('tenantA');
    +#rename, and move to active
    +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id  => {Accept => 'application/json'} => json => {
    +			"name" => "tenantA2", "active" => $true, "parentId" => $root_tenant_id 
    +		})
    +		->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +		->json_is( "/response/name" => "tenantA2" )
    +		->json_is( "/response/id" => $tenantA_id )
    +		->json_is( "/response/active" => $true )
    +		->json_is( "/response/parentId" => $root_tenant_id )
    +		->json_is( "/alerts/0/level" => "success" )
    +	, 'Does the tenantA2 details return?';
    +
    +#change "active"
    +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id  => {Accept => 'application/json'} => json => {
    +			"name" => "tenantA2", "active" => $false, "parentId" => $root_tenant_id 
    +		})
    +		->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +		->json_is( "/response/name" => "tenantA2" )
    +		->json_is( "/response/id" => $tenantA_id )
    +		->json_is( "/response/active" => $false )
    +		->json_is( "/response/parentId" => $root_tenant_id )
    +		->json_is( "/alerts/0/level" => "success" )
    +	, 'Did we moved to non active?';
    +
    +#change "active" back
    +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id  => {Accept => 'application/json'} => json => {
    +			"name" => "tenantA2", "active" => $true, "parentId" => $root_tenant_id 
    +		})
    +		->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +		->json_is( "/response/name" => "tenantA2" )
    +		->json_is( "/response/id" => $tenantA_id )
    +		->json_is( "/response/active" => $true )
    +		->json_is( "/response/parentId" => $root_tenant_id )
    +		->json_is( "/alerts/0/level" => "success" )
    +	, 'Did we moved back to active?';
    +
    +#cannot change tenant parent to undef
    +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id  => {Accept => 'application/json'} => json => {
    +			"name" => "tenantC", 
    +		})->status_is(400);
    +
    +#cannot change root-tenant to inactive
    +ok $t->put_ok('/api/1.2/tenants/' . $root_tenant_id  => {Accept => 'application/json'} => json => {
    +			"name" => "root", "active" => $false, "parentId" => undef  
    +		})->status_is(400);
    +
    +#adding a child tenant
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantD", "active" => $true, "parentId" => $tenantA_id })->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +	->json_is( "/response/name" => "tenantD" )
    +	->json_is( "/response/active" => $true )
    +	->json_is( "/response/parentId" =>  $tenantA_id)
    +            , 'Does the tenant details return?';
    +
    +#adding a child inactive tenant
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantE", "active" => $false, "parentId" => $tenantA_id })->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +	->json_is( "/response/name" => "tenantE" )
    +	->json_is( "/response/active" => $false )
    +	->json_is( "/response/parentId" =>  $tenantA_id)
    +            , 'Does the tenant details return?';
    +
    +#cannot delete a tenant that have children
    +ok $t->delete_ok('/api/1.2/tenants/' . $tenantA_id)->status_is(500);
    --- End diff --
    
    how about returning a 400 instead in the code rather than letting the db throw a 500?


---
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 #322: Adding tenancy to the database

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/322#discussion_r103548093
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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,
    --- End diff --
    
    Will fix 


---
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 #322: Adding tenancy to the database

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/322#discussion_r103548224
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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,
    --- End diff --
    
    \$raw-active again


---
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 #322: Adding tenancy to the database

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/322#discussion_r103793015
  
    --- Diff: traffic_ops/app/t/api/1.2/tenant.t ---
    @@ -0,0 +1,156 @@
    +package main;
    +#
    +#
    +# 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 Mojo::Base -strict;
    +use Test::More;
    +use Test::Mojo;
    +use DBI;
    +use strict;
    +use warnings;
    +no warnings 'once';
    +use warnings 'all';
    +use Test::TestHelper;
    +
    +#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
    +#no_transactions=>0 ==> delete fixtures after every execution
    +
    +BEGIN { $ENV{MOJO_MODE} = "test" }
    +
    +my $schema = Schema->connect_to_database;
    +my $dbh    = Schema->database_handle;
    +my $t      = Test::Mojo->new('TrafficOps');
    +
    +my $false = 0;
    --- End diff --
    
    ACK


---
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 #322: Adding tenancy to the database

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/322#discussion_r103534303
  
    --- Diff: traffic_ops/app/lib/API/User.pm ---
    @@ -218,6 +218,118 @@ sub update {
     
     }
     
    +# Create
    +sub create {
    +	my $self = shift;
    +	my $params = $self->req->json;
    +	
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{username};
    +	if ( !defined($name) ) {
    +		return $self->alert("Username is required.");
    +	}
    +	
    +	my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
    +	if ($existing) {
    +		return $self->alert("A user with username \"$name\" already exists.");
    +	}
    +
    +
    +	if ( !defined($params->{fullName}) ) {
    +		return $self->alert("full-name is required.");
    +	}
    +
    +	if ( !defined($params->{email}) ) {
    +		return $self->alert("email is required.");
    +	}
    +	
    +	if ( !defined($params->{localPassword}) ) {
    +		return $self->alert("local-password is required.");
    +	}
    +
    +	if ( !defined($params->{confirmLocalPassword}) ) {
    +		return $self->alert("confirm-local-password is required.");
    +	}
    +
    +	if ($params->{localPassword} ne $params->{confirmLocalPassword}){
    +		return $self->alert("local-password and confirmed-local-password mismatch.");
    +	}
    +	
    +	if ( !defined($params->{role}) ) {
    +		return $self->alert("role is required.");
    +	}
    +	
    +	
    +
    +	my $values = {
    +		address_line1 			=> defined_or_default($params->{addressLine1}, ""),
    +		address_line2 			=> defined_or_default($params->{addressLine2}, ""),
    +		city 				=> defined_or_default($params->{city}, ""),
    +		company 			=> defined_or_default($params->{company}, ""),
    +		country 			=> defined_or_default($params->{country}, ""),
    +		email 				=> $params->{email},
    +		full_name 			=> $params->{fullName},
    +		new_user 			=> ( $params->{newUser} ) ? 1 : 0,
    +		phone_number 			=> defined_or_default($params->{phoneNumber}, ""),
    +		postal_code 			=> defined_or_default($params->{postalCode}, ""),
    +		public_ssh_key 			=> defined_or_default($params->{publicSshKey}, ""),
    +		registration_sent 		=> defined_or_default( $params->{registrationSent}, undef),
    +		role 				=> $params->{role},
    +		state_or_province 		=> defined_or_default($params->{stateOrProvince}, ""),
    +		username 			=> $params->{username},
    +		new_user            		=> defined_or_default($params->{newUser}, 0),		
    +		uid                  		=> defined_or_default($params->{uid}, 0),		
    --- End diff --
    
    I don't think you'd ever want the api consumer to define the uid or gid of the user...so i would just leave it out. if they send it, it gets ignored.


---
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 #322: Adding tenancy to the database

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/322#discussion_r104487370
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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,
    --- End diff --
    
    looks like this casting isnt done yet


---
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 #322: Adding tenancy to the database

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/322#discussion_r103535963
  
    --- Diff: traffic_ops/app/t/api/1.2/tenant.t ---
    @@ -0,0 +1,156 @@
    +package main;
    +#
    +#
    +# 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 Mojo::Base -strict;
    +use Test::More;
    +use Test::Mojo;
    +use DBI;
    +use strict;
    +use warnings;
    +no warnings 'once';
    +use warnings 'all';
    +use Test::TestHelper;
    +
    +#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
    +#no_transactions=>0 ==> delete fixtures after every execution
    +
    +BEGIN { $ENV{MOJO_MODE} = "test" }
    +
    +my $schema = Schema->connect_to_database;
    +my $dbh    = Schema->database_handle;
    +my $t      = Test::Mojo->new('TrafficOps');
    +
    +my $false = 0;
    --- End diff --
    
    i would get rid of these variables and just use 0 or 1 instead.


---
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 #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322
  
    The \ is still in my local repo. Will be pushed on next update
    
    On Mar 6, 2017 8:35 PM, "Jeremy Mitchell" <no...@github.com> wrote:
    
    *@mitchell852* commented on this pull request.
    ------------------------------
    
    In traffic_ops/app/lib/API/Tenant.pm
    <https://github.com/apache/incubator-trafficcontrol/pull/322#discussion_r104487370>
    :
    
    > +	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,
    
    looks like this casting isnt done yet
    
    \u2014
    You are receiving this because you authored the thread.
    Reply to this email directly, view it on GitHub
    <https://github.com/apache/incubator-trafficcontrol/pull/322#discussion_r104487370>,
    or mute the thread
    <https://github.com/notifications/unsubscribe-auth/AXGkYILQTeuy8bkyLkxZkc5-79YPL9Rrks5rjFHtgaJpZM4MOyR9>
    .



---
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 #322: Adding tenancy to the database

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

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


---
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 #322: Adding tenancy to the database

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/322#discussion_r103547240
  
    --- Diff: traffic_ops/app/t/api/1.2/tenant.t ---
    @@ -0,0 +1,156 @@
    +package main;
    +#
    +#
    +# 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 Mojo::Base -strict;
    +use Test::More;
    +use Test::Mojo;
    +use DBI;
    +use strict;
    +use warnings;
    +no warnings 'once';
    +use warnings 'all';
    +use Test::TestHelper;
    +
    +#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
    +#no_transactions=>0 ==> delete fixtures after every execution
    +
    +BEGIN { $ENV{MOJO_MODE} = "test" }
    +
    +my $schema = Schema->connect_to_database;
    +my $dbh    = Schema->database_handle;
    +my $t      = Test::Mojo->new('TrafficOps');
    +
    +my $false = 0;
    +my $true = 1;
    +
    +Test::TestHelper->unload_core_data($schema);
    +Test::TestHelper->load_core_data($schema);
    +
    +ok $t->post_ok( '/login', => form => { u => Test::TestHelper::ADMIN_USER, p => Test::TestHelper::ADMIN_USER_PASSWORD } )->status_is(302)
    +	->or( sub { diag $t->tx->res->content->asset->{content}; } ), 'Should login?';
    +
    +#verifying the basic cfg
    +$t->get_ok("/api/1.2/tenants")->status_is(200)->json_is( "/response/0/name", "root" )->or( sub { diag $t->tx->res->content->asset->{content}; } );;
    +
    +my $root_tenant_id = &get_tenant_id('root');
    +
    +#setting with no "active" field which is optional
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantA", "parentId" => $root_tenant_id })->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +	->json_is( "/response/name" => "tenantA" )
    +	->json_is( "/response/active" =>  $false)
    +	->json_is( "/response/parentId" =>  $root_tenant_id)
    +            , 'Does the tenant details return?';
    +
    +#same name - would not accept
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantA", "active" => $true, "parentId" => $root_tenant_id })->status_is(400);
    +
    +#no name - would not accept
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "parentId" => $root_tenant_id })->status_is(400);
    +
    +#no parent - would not accept
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantB" })->status_is(400);
    +
    +my $tenantA_id = &get_tenant_id('tenantA');
    +#rename, and move to active
    +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id  => {Accept => 'application/json'} => json => {
    +			"name" => "tenantA2", "active" => $true, "parentId" => $root_tenant_id 
    +		})
    +		->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +		->json_is( "/response/name" => "tenantA2" )
    +		->json_is( "/response/id" => $tenantA_id )
    +		->json_is( "/response/active" => $true )
    +		->json_is( "/response/parentId" => $root_tenant_id )
    +		->json_is( "/alerts/0/level" => "success" )
    +	, 'Does the tenantA2 details return?';
    +
    +#change "active"
    +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id  => {Accept => 'application/json'} => json => {
    +			"name" => "tenantA2", "active" => $false, "parentId" => $root_tenant_id 
    +		})
    +		->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +		->json_is( "/response/name" => "tenantA2" )
    +		->json_is( "/response/id" => $tenantA_id )
    +		->json_is( "/response/active" => $false )
    +		->json_is( "/response/parentId" => $root_tenant_id )
    +		->json_is( "/alerts/0/level" => "success" )
    +	, 'Did we moved to non active?';
    +
    +#change "active" back
    +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id  => {Accept => 'application/json'} => json => {
    +			"name" => "tenantA2", "active" => $true, "parentId" => $root_tenant_id 
    +		})
    +		->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +		->json_is( "/response/name" => "tenantA2" )
    +		->json_is( "/response/id" => $tenantA_id )
    +		->json_is( "/response/active" => $true )
    +		->json_is( "/response/parentId" => $root_tenant_id )
    +		->json_is( "/alerts/0/level" => "success" )
    +	, 'Did we moved back to active?';
    +
    +#cannot change tenant parent to undef
    +ok $t->put_ok('/api/1.2/tenants/' . $tenantA_id  => {Accept => 'application/json'} => json => {
    +			"name" => "tenantC", 
    +		})->status_is(400);
    +
    +#cannot change root-tenant to inactive
    +ok $t->put_ok('/api/1.2/tenants/' . $root_tenant_id  => {Accept => 'application/json'} => json => {
    +			"name" => "root", "active" => $false, "parentId" => undef  
    +		})->status_is(400);
    +
    +#adding a child tenant
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantD", "active" => $true, "parentId" => $tenantA_id })->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +	->json_is( "/response/name" => "tenantD" )
    +	->json_is( "/response/active" => $true )
    +	->json_is( "/response/parentId" =>  $tenantA_id)
    +            , 'Does the tenant details return?';
    +
    +#adding a child inactive tenant
    +ok $t->post_ok('/api/1.2/tenants' => {Accept => 'application/json'} => json => {
    +        "name" => "tenantE", "active" => $false, "parentId" => $tenantA_id })->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
    +	->json_is( "/response/name" => "tenantE" )
    +	->json_is( "/response/active" => $false )
    +	->json_is( "/response/parentId" =>  $tenantA_id)
    +            , 'Does the tenant details return?';
    +
    +#cannot delete a tenant that have children
    +ok $t->delete_ok('/api/1.2/tenants/' . $tenantA_id)->status_is(500);
    --- End diff --
    
    Added to my to do list - improve 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 issue #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322
  
    Is this still a WIP? Or can it be merged when approved?


---
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 #322: Adding tenancy to the database

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/322#discussion_r103534500
  
    --- Diff: traffic_ops/app/lib/API/User.pm ---
    @@ -218,6 +218,118 @@ sub update {
     
     }
     
    +# Create
    +sub create {
    +	my $self = shift;
    +	my $params = $self->req->json;
    +	
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{username};
    +	if ( !defined($name) ) {
    +		return $self->alert("Username is required.");
    +	}
    +	
    +	my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
    +	if ($existing) {
    +		return $self->alert("A user with username \"$name\" already exists.");
    +	}
    +
    +
    +	if ( !defined($params->{fullName}) ) {
    +		return $self->alert("full-name is required.");
    +	}
    +
    +	if ( !defined($params->{email}) ) {
    +		return $self->alert("email is required.");
    +	}
    +	
    +	if ( !defined($params->{localPassword}) ) {
    +		return $self->alert("local-password is required.");
    +	}
    +
    +	if ( !defined($params->{confirmLocalPassword}) ) {
    +		return $self->alert("confirm-local-password is required.");
    +	}
    +
    +	if ($params->{localPassword} ne $params->{confirmLocalPassword}){
    +		return $self->alert("local-password and confirmed-local-password mismatch.");
    +	}
    +	
    +	if ( !defined($params->{role}) ) {
    +		return $self->alert("role is required.");
    +	}
    +	
    +	
    +
    +	my $values = {
    --- End diff --
    
    aren't they supposed to defined the tenantId of the 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 issue #322: Adding tenancy to the database

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

    https://github.com/apache/incubator-trafficcontrol/pull/322
  
    This PR can stand for its own. It passed the unit tests beside
    delivery-service related failures, which as far as I can see are not
    related to my change. Will be able to double check only next week.
    
    Next to come are
    0. Changes following your remarks
    1. Delivery service API - add tenancy
    2. Warning on put requests with no tenancy check.
    3. Delivery enforcement.
    
    So no much changes in already PR code.
    
    On the other hand, it might be best to pull it after step "2" - just before
    we start to enforce stuff.
    
    Nir
    
    On Mar 1, 2017 3:38 PM, "Jeremy Mitchell" <no...@github.com> wrote:
    
    > Is this still a WIP? Or can it be merged when approved?
    >
    > \u2014
    > You are receiving this because you authored the thread.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/incubator-trafficcontrol/pull/322#issuecomment-283494568>,
    > or mute the thread
    > <https://github.com/notifications/unsubscribe-auth/AXGkYDRV6wi-OyLTtvNqKzBofCzSV3A5ks5rhfOCgaJpZM4MOyR9>
    > .
    >



---
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 #322: Adding tenancy to the database

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/322#discussion_r103535173
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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,
    --- End diff --
    
    ensure this returns a true|false in the api.  you might have to cast it like \\$row->active,


---
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 #322: Adding tenancy to the database

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/322#discussion_r103532374
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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;
    --- End diff --
    
    ensure this returns true|false in the 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 #322: Adding tenancy to the database

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/322#discussion_r103535776
  
    --- Diff: traffic_ops/app/lib/Schema/Result/GooseDbVersion.pm ---
    @@ -25,25 +25,24 @@ __PACKAGE__->table("goose_db_version");
     
    --- End diff --
    
    not sure why this file was touched...doesn't seem related to your changes


---
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 #322: Adding tenancy to the database

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/322#discussion_r103547425
  
    --- Diff: traffic_ops/app/lib/API/User.pm ---
    @@ -218,6 +218,118 @@ sub update {
     
     }
     
    +# Create
    +sub create {
    +	my $self = shift;
    +	my $params = $self->req->json;
    +	
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	my $name = $params->{username};
    +	if ( !defined($name) ) {
    +		return $self->alert("Username is required.");
    +	}
    +	
    +	my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
    +	if ($existing) {
    +		return $self->alert("A user with username \"$name\" already exists.");
    +	}
    +
    +
    +	if ( !defined($params->{fullName}) ) {
    +		return $self->alert("full-name is required.");
    +	}
    +
    +	if ( !defined($params->{email}) ) {
    +		return $self->alert("email is required.");
    +	}
    +	
    +	if ( !defined($params->{localPassword}) ) {
    +		return $self->alert("local-password is required.");
    +	}
    +
    +	if ( !defined($params->{confirmLocalPassword}) ) {
    +		return $self->alert("confirm-local-password is required.");
    +	}
    +
    +	if ($params->{localPassword} ne $params->{confirmLocalPassword}){
    +		return $self->alert("local-password and confirmed-local-password mismatch.");
    +	}
    +	
    +	if ( !defined($params->{role}) ) {
    +		return $self->alert("role is required.");
    +	}
    +	
    +	
    +
    +	my $values = {
    --- End diff --
    
    This will be added a separate commit


---
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 #322: Adding tenancy to the database

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/322#discussion_r103532921
  
    --- Diff: traffic_ops/app/lib/Schema/Result/GooseDbVersion.pm ---
    @@ -25,25 +25,24 @@ __PACKAGE__->table("goose_db_version");
     
    --- End diff --
    
    not really sure why this table got touched....


---
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 #322: Adding tenancy to the database

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/322#discussion_r103548560
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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 {
    --- End diff --
    
    Maybe we should avoid deleting a tenant???


---
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 #322: Adding tenancy to the database

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/322#discussion_r103547046
  
    --- Diff: traffic_ops/app/lib/API/Tenant.pm ---
    @@ -0,0 +1,225 @@
    +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 {
    --- End diff --
    
    No. Protection is done via the DB.
    Need to UT that


---
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.
---