Skip to content
This repository has been archived by the owner on Jul 24, 2021. It is now read-only.

Commit

Permalink
Merge pull request #1072 from joyent/ether/return-201-not-303
Browse files Browse the repository at this point in the history
return 201 not 303
  • Loading branch information
karenetheridge authored Dec 7, 2020
2 parents 11b76ee + 6f0d9a6 commit ede58be
Show file tree
Hide file tree
Showing 48 changed files with 199 additions and 190 deletions.
8 changes: 4 additions & 4 deletions docs/modules/Conch::Route::Build.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Supports the following optional query parameters:
- Requires system admin authorization
- Controller/Action: ["create" in Conch::Controller::Build](../modules/Conch%3A%3AController%3A%3ABuild#create)
- Request: [request.json#/$defs/BuildCreate](../json-schema/request.json#/$defs/BuildCreate)
- Response: Redirect to the build
- Response: `201 Created`, plus Location header

### `GET /build/:build_id_or_name`

Expand All @@ -48,20 +48,20 @@ Supports the following optional query parameters:
- Requires system admin authorization or the admin role on the build
- Controller/Action: ["update" in Conch::Controller::Build](../modules/Conch%3A%3AController%3A%3ABuild#update)
- Request: [request.json#/$defs/BuildUpdate](../json-schema/request.json#/$defs/BuildUpdate)
- Response: Redirect to the build
- Response: `204 No Content`, plus Location header

#### `POST /build/:build_id_or_name/links`

- Requires system admin authorization or the admin role on the build
- Controller/Action: ["add\_links" in Conch::Controller::Build](../modules/Conch%3A%3AController%3A%3ABuild#add_links)
- Request: [request.json#/$defs/BuildLinks](../json-schema/request.json#/$defs/BuildLinks)
- Response: Redirect to the updated build
- Response: `204 No Content`, plus Location header

#### `DELETE /build/:build_id_or_name/links`

- Requires system admin authorization or the admin role on the build
- Request: [request.json#/$defs/BuildLinksOrNull](../json-schema/request.json#/$defs/BuildLinksOrNull)
- Response: 204 NO CONTENT
- Response: `204 No Content`, plus Location header

### `GET /build/:build_id_or_name/user`

Expand Down
2 changes: 1 addition & 1 deletion docs/modules/Conch::Route::Datacenter.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ All routes require authentication.
- Requires system admin authorization
- Controller/Action: ["update" in Conch::Controller::Datacenter](../modules/Conch%3A%3AController%3A%3ADatacenter#update)
- Request: [request.json#/$defs/DatacenterUpdate](../json-schema/request.json#/$defs/DatacenterUpdate)
- Response: Redirect to the updated datacenter
- Response: `204 No Content`, plus Location header

### `DELETE /dc/:datacenter_id`

Expand Down
4 changes: 2 additions & 2 deletions docs/modules/Conch::Route::DatacenterRoom.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ All routes require authentication.
- Requires system admin authorization
- Controller/Action: ["create" in Conch::Controller::DatacenterRoom](../modules/Conch%3A%3AController%3A%3ADatacenterRoom#create)
- Request: [request.json#/$defs/DatacenterRoomCreate](../json-schema/request.json#/$defs/DatacenterRoomCreate)
- Response: Redirect to the created room
- Response: `201 Created`, plus Location header

### `GET /room/:datacenter_room_id_or_alias`

Expand All @@ -39,7 +39,7 @@ the room
- Requires system admin authorization
- Controller/Action: ["update" in Conch::Controller::DatacenterRoom](../modules/Conch%3A%3AController%3A%3ADatacenterRoom#update)
- Request: [request.json#/$defs/DatacenterRoomUpdate](../json-schema/request.json#/$defs/DatacenterRoomUpdate)
- Response: Redirect to the updated room
- Response: `204 No Content`, plus Location header

### `DELETE /room/:datacenter_room_id_or_alias`

Expand Down
14 changes: 7 additions & 7 deletions docs/modules/Conch::Route::Device.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,28 @@ below.
- User requires the read/write role
- Controller/Action: ["set\_asset\_tag" in Conch::Controller::Device](../modules/Conch%3A%3AController%3A%3ADevice#set_asset_tag)
- Request: [request.json#/$defs/DeviceAssetTag](../json-schema/request.json#/$defs/DeviceAssetTag)
- Response: Redirect to the updated device
- Response: `204 No Content`, plus Location header

### `POST /device/:device_id_or_serial_number/validated`

- User requires the read/write role
- Controller/Action: ["set\_validated" in Conch::Controller::Device](../modules/Conch%3A%3AController%3A%3ADevice#set_validated)
- Request: [request.json#/$defs/Null](../json-schema/request.json#/$defs/Null)
- Response: Redirect to the updated device
- Response: `204 No Content`, plus Location header

### `POST /device/:device_id_or_serial_number/phase`

- User requires the read/write role
- Controller/Action: ["set\_phase" in Conch::Controller::Device](../modules/Conch%3A%3AController%3A%3ADevice#set_phase)
- Request: [request.json#/$defs/DevicePhase](../json-schema/request.json#/$defs/DevicePhase)
- Response: Redirect to the updated device
- Response: `204 No Content`, plus Location header

### `POST /device/:device_id_or_serial_number/links`

- User requires the read/write role
- Controller/Action: ["add\_links" in Conch::Controller::Device](../modules/Conch%3A%3AController%3A%3ADevice#add_links)
- Request: [request.json#/$defs/DeviceLinks](../json-schema/request.json#/$defs/DeviceLinks)
- Response: Redirect to the updated device
- Response: `204 No Content`, plus Location header

### `DELETE /device/:device_id_or_serial_number/links`

Expand All @@ -102,7 +102,7 @@ below.
- User requires the read/write role for the device, as well as the old and new builds
- Controller/Action: ["set\_build" in Conch::Controller::Device](../modules/Conch%3A%3AController%3A%3ADevice#set_build)
- Request: [request.json#/$defs/DeviceBuild](../json-schema/request.json#/$defs/DeviceBuild)
- Response: Redirect to the updated device
- Response: `204 No Content`, plus Location header

### `POST /device/:device_id_or_serial_number/hardware_product`

Expand All @@ -111,7 +111,7 @@ below.
- User requires the admin role for the device
- Controller/Action: ["set\_hardware\_product" in Conch::Controller::Device](../modules/Conch%3A%3AController%3A%3ADevice#set_hardware_product)
- Request: [request.json#/$defs/DeviceHardware](../json-schema/request.json#/$defs/DeviceHardware)
- Response: Redirect to the updated device
- Response: `204 No Content`, plus Location header

### `GET /device/:device_id_or_serial_number/location`

Expand All @@ -124,7 +124,7 @@ below.
- User requires the read/write role
- Controller/Action: ["set" in Conch::Controller::DeviceLocation](../modules/Conch%3A%3AController%3A%3ADeviceLocation#set)
- Request: [request.json#/$defs/DeviceLocationUpdate](../json-schema/request.json#/$defs/DeviceLocationUpdate)
- Response: Redirect to the updated device
- Response: `204 No Content`, plus Location header

### `DELETE /device/:device_id_or_serial_number/location`

Expand Down
4 changes: 2 additions & 2 deletions docs/modules/Conch::Route::HardwareProduct.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ All routes require authentication.
- Requires system admin authorization
- Controller/Action: ["create" in Conch::Controller::HardwareProduct](../modules/Conch%3A%3AController%3A%3AHardwareProduct#create)
- Request: [request.json#/$defs/HardwareProductCreate](../json-schema/request.json#/$defs/HardwareProductCreate)
- Response: Redirect to the created hardware product
- Response: `201 Created`, plus Location header

### `GET /hardware_product/:hardware_product_id_or_other`

Expand All @@ -42,7 +42,7 @@ Identifiers accepted: `id`, `sku`, `name` and `alias`.
- Requires system admin authorization
- Controller/Action: ["update" in Conch::Controller::HardwareProduct](../modules/Conch%3A%3AController%3A%3AHardwareProduct#update)
- Request: [request.json#/$defs/HardwareProductUpdate](../json-schema/request.json#/$defs/HardwareProductUpdate)
- Response: Redirect to the updated hardware product
- Response: `204 No Content`, plus Location header

### `DELETE /hardware_product/:hardware_product_id_or_other`

Expand Down
2 changes: 1 addition & 1 deletion docs/modules/Conch::Route::HardwareVendor.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ All routes require authentication.
- Requires system admin authorization
- Controller/Action: ["create" in Conch::Controller::HardwareVendor](../modules/Conch%3A%3AController%3A%3AHardwareVendor#create)
- Request: [request.json#/$defs/Null](../json-schema/request.json#/$defs/Null)
- Response: Redirect to the created hardware vendor
- Response: `201 Created`, plus Location header

## LICENSING

Expand Down
4 changes: 2 additions & 2 deletions docs/modules/Conch::Route::Organization.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ All routes require authentication.
- Requires system admin authorization
- Controller/Action: ["create" in Conch::Controller::Organization](../modules/Conch%3A%3AController%3A%3AOrganization#create)
- Request: [request.json#/$defs/OrganizationCreate](../json-schema/request.json#/$defs/OrganizationCreate)
- Response: Redirect to the organization
- Response: `201 Created`, plus Location header

### `GET /organization/:organization_id_or_name`

Expand All @@ -37,7 +37,7 @@ All routes require authentication.
- Requires system admin authorization or the admin role on the organization
- Controller/Action: ["update" in Conch::Controller::Organization](../modules/Conch%3A%3AController%3A%3AOrganization#update)
- Request: [request.json#/$defs/OrganizationUpdate](../json-schema/request.json#/$defs/OrganizationUpdate)
- Response: Redirect to the organization
- Response: `204 No Content`, plus Location header

### `DELETE /organization/:organization_id_or_name`

Expand Down
14 changes: 7 additions & 7 deletions docs/modules/Conch::Route::Rack.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ available under `/rack/:rack_id_or_long_name` as well as
- Requires system admin authorization
- Controller/Action: ["create" in Conch::Controller::Rack](../modules/Conch%3A%3AController%3A%3ARack#create)
- Request: [request.json#/$defs/RackCreate](../json-schema/request.json#/$defs/RackCreate)
- Response: Redirect to the created rack
- Response: `201 Created`, plus Location header

### `GET /rack/:rack_id_or_name`

Expand All @@ -40,7 +40,7 @@ available under `/rack/:rack_id_or_long_name` as well as
- User requires the read/write role on the rack
- Controller/Action: ["update" in Conch::Controller::Rack](../modules/Conch%3A%3AController%3A%3ARack#update)
- Request: [request.json#/$defs/RackUpdate](../json-schema/request.json#/$defs/RackUpdate)
- Response: Redirect to the updated rack
- Response: `204 No Content`, plus Location header

### `DELETE /rack/:rack_id_or_name`

Expand All @@ -59,7 +59,7 @@ available under `/rack/:rack_id_or_long_name` as well as
- User requires the read/write role on the rack
- Controller/Action: ["overwrite\_layouts" in Conch::Controller::Rack](../modules/Conch%3A%3AController%3A%3ARack#overwrite_layouts)
- Request: [request.json#/$defs/RackLayouts](../json-schema/request.json#/$defs/RackLayouts)
- Response: Redirect to the rack's layouts
- Response: `204 No Content`, plus Location header

### `GET /rack/:rack_id_or_name/assignment`

Expand All @@ -72,7 +72,7 @@ available under `/rack/:rack_id_or_long_name` as well as
- User requires the read/write role on the rack
- Controller/Action: ["set\_assignment" in Conch::Controller::Rack](../modules/Conch%3A%3AController%3A%3ARack#set_assignment)
- Request: [request.json#/$defs/RackAssignmentUpdates](../json-schema/request.json#/$defs/RackAssignmentUpdates)
- Response: Redirect to the updated rack assignment
- Response: `204 No Content`, plus Location header

### `DELETE /rack/:rack_id_or_name/assignment`

Expand All @@ -91,20 +91,20 @@ only the rack's phase, or all the rack's devices' phases as well.
- User requires the read/write role on the rack
- Controller/Action: ["set\_phase" in Conch::Controller::Rack](../modules/Conch%3A%3AController%3A%3ARack#set_phase)
- Request: [request.json#/$defs/RackPhase](../json-schema/request.json#/$defs/RackPhase)
- Response: Redirect to the updated rack
- Response: `204 No Content`, plus Location header

#### `POST /rack/:rack_id_or_name/links`

- User requires the read/write role on the rack
- Controller/Action: ["add\_links" in Conch::Controller::Rack](../modules/Conch%3A%3AController%3A%3ARack#add_links)
- Request: [request.json#/$defs/RackLinks](../json-schema/request.json#/$defs/RackLinks)
- Response: Redirect to the updated rack
- Response: `204 No Content`, plus Location header

#### `DELETE /rack/:rack_id_or_name/links`

- User requires the read/write role on the rack
- Request: [request.json#/$defs/RackLinksOrNull](../json-schema/request.json#/$defs/RackLinksOrNull)
- Response: 204 NO CONTENT
- Response: `204 No Content`, plus Location header

### `GET /rack/:rack_id_or_name/layout/:layout_id_or_rack_unit_start`

Expand Down
4 changes: 2 additions & 2 deletions docs/modules/Conch::Route::RackLayout.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ well as
- Requires system admin authorization
- Controller/Action: ["create" in Conch::Controller::RackLayout](../modules/Conch%3A%3AController%3A%3ARackLayout#create)
- Request: [request.json#/$defs/RackLayoutCreate](../json-schema/request.json#/$defs/RackLayoutCreate)
- Response: Redirect to the created rack layout
- Response: `201 Created`, plus Location header

### `GET /layout/:layout_id`

Expand All @@ -47,7 +47,7 @@ well as
- Requires system admin authorization
- Controller/Action: ["update" in Conch::Controller::RackLayout](../modules/Conch%3A%3AController%3A%3ARackLayout#update)
- Request: [request.json#/$defs/RackLayoutUpdate](../json-schema/request.json#/$defs/RackLayoutUpdate)
- Response: Redirect to the update rack layout
- Response: `204 No Content`, plus Location header

### `DELETE /layout/:layout_id`

Expand Down
4 changes: 2 additions & 2 deletions docs/modules/Conch::Route::RackRole.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ All routes require authentication.
- Requires system admin authorization
- Controller/Action: ["create" in Conch::Controller::RackRole](../modules/Conch%3A%3AController%3A%3ARackRole#create)
- Request: [request.json#/$defs/RackRoleCreate](../json-schema/request.json#/$defs/RackRoleCreate)
- Response: Redirect to the created rack role
- Response: `201 Created`, plus Location header

### `GET /rack_role/:rack_role_id_or_name`

Expand All @@ -36,7 +36,7 @@ All routes require authentication.
- Requires system admin authorization
- Controller/Action: ["update" in Conch::Controller::RackRole](../modules/Conch%3A%3AController%3A%3ARackRole#update)
- Request: [request.json#/$defs/RackRoleUpdate](../json-schema/request.json#/$defs/RackRoleUpdate)
- Response: Redirect to the updated rack role
- Response: `204 No Content`, plus Location header

### `DELETE /rack_role/:rack_role_id_or_name`

Expand Down
4 changes: 2 additions & 2 deletions docs/modules/Conch::Route::User.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ an email telling the user their account was updated

- Controller/Action: ["update" in Conch::Controller::User](../modules/Conch%3A%3AController%3A%3AUser#update)
- Request: [request.json#/$defs/UpdateUser](../json-schema/request.json#/$defs/UpdateUser)
- Success Response: Redirect to the user that was updated
- Response: `204 No Content`, plus Location header
- Error response on duplicate user: [response.json#/$defs/UserError](../json-schema/response.json#/$defs/UserError) (only if the
calling user is a system admin)

Expand Down Expand Up @@ -123,7 +123,7 @@ an email telling the user their account was updated
- Requires system admin authorization
- Controller/Action: ["update" in Conch::Controller::User](../modules/Conch%3A%3AController%3A%3AUser#update)
- Request: [request.json#/$defs/UpdateUser](../json-schema/request.json#/$defs/UpdateUser)
- Success Response: Redirect to the user that was updated
- Response: `204 No Content`, plus Location header
- Error response on duplicate user: [response.json#/$defs/UserError](../json-schema/response.json#/$defs/UserError) (only if the
calling user is a system admin)

Expand Down
2 changes: 1 addition & 1 deletion docs/modules/Test::Conch.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Wrapper around ["status\_is" in Test::Mojo](https://metacpan.org/pod/Test%3A%3AM
3.1. 2xx and 4xx JSON responses should have a Link header
4. HEAD requests should not have body content
5. 200, 203, 206, 207 and most 4xx responses should have body content
6. 201, 204, 205 and most 3xx responses should not have body content
6. 204, 205 and most 3xx responses should not have body content
7. 302 should not be used at all
8. 401, 403 responses should have a WWW-Authenticate header
```
Expand Down
6 changes: 5 additions & 1 deletion lib/Conch.pm
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,11 @@ C<< $c->render >> as a side-effect.
return 0;
}

if (any { $code == $_ } 301, 302, 303, 305, 307, 308) {
if ($code == 204) {
$c->res->headers->location($payload) if defined $payload;
$c->rendered;
}
elsif (any { $code == $_ } 301, 302, 303, 305, 307, 308) {
$c->redirect_to($payload);
}
else {
Expand Down
10 changes: 6 additions & 4 deletions lib/Conch/Controller/Build.pm
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ sub create ($c) {
user_build_roles => [ map +{ user_id => $_->[2], role => 'admin' }, @admins ],
});
$c->log->info('created build '.$build->id.' ('.$build->name.')');
$c->status(303, '/build/'.$build->id);
$c->res->headers->location('/build/'.$build->id);
$c->status(201);
}

=head2 find_build
Expand Down Expand Up @@ -234,7 +235,7 @@ sub update ($c) {

$build->update if $build->is_changed;

$c->status(303, '/build/'.$build->id);
$c->status(204, '/build/'.$build->id);
}

=head2 add_links
Expand All @@ -253,7 +254,7 @@ sub add_links ($c) {
->update({ links => \[ 'array_cat_distinct(links,?)', [{},$input->{links}] ] });

my $build_id = $c->stash('build_id') // $c->stash('build_rs')->get_column('id')->single;
$c->status(303, '/build/'.$build_id);
$c->status(204, '/build/'.$build_id);
}

=head2 remove_links
Expand All @@ -279,7 +280,8 @@ sub remove_links ($c) {
->update({ links => '{}' });
}

$c->status(204);
my $build_id = $c->stash('build_id') // $c->stash('build_rs')->get_column('id')->single;
$c->status(204, '/build/'.$build_id);
}

=head2 get_users
Expand Down
7 changes: 3 additions & 4 deletions lib/Conch/Controller/Datacenter.pm
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ sub create ($c) {
return $c->status(409, { error => 'a datacenter already exists with that vendor-region-location' });
}
else {
$c->res->headers->location('/dc/'.$dc->id);
return $c->status(204);
return $c->status(204, '/dc/'.$dc->id);
}
}

Expand All @@ -110,11 +109,11 @@ sub update ($c) {
my $input = $c->stash('request_data');
my $datacenter = $c->stash('datacenter');
$datacenter->set_columns($input);
return $c->status(204) if not $datacenter->is_changed;
return $c->status(204, '/dc/'.$datacenter->id) if not $datacenter->is_changed;

$datacenter->update({ updated => \'now()' });
$c->log->debug('Updated datacenter '.$datacenter->id);
$c->status(303, '/dc/'.$datacenter->id);
$c->status(204, '/dc/'.$datacenter->id);
}

=head2 delete
Expand Down
5 changes: 3 additions & 2 deletions lib/Conch/Controller/DatacenterRoom.pm
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ sub create ($c) {

my $room = $c->db_datacenter_rooms->create($input);
$c->log->debug('Created datacenter room '.$room->id);
$c->status(303, '/room/'.$room->id);
$c->res->headers->location('/room/'.$room->id);
$c->status(201);
}

=head2 update
Expand Down Expand Up @@ -130,7 +131,7 @@ sub update ($c) {

$room->update({ updated => \'now()' });
$c->log->debug('Updated datacenter room '.$c->stash('datacenter_room_id_or_alias'));
$c->status(303);
$c->status(204);
}

=head2 delete
Expand Down
Loading

0 comments on commit ede58be

Please sign in to comment.