From 1813ec93946bb934a9ab2b64deeb191855072eb2 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Tue, 5 Jan 2021 12:44:01 -0800 Subject: [PATCH 1/6] fix documentation --- docs/modules/Conch::Route::HardwareProduct.md | 4 ++-- docs/modules/Conch::Route::JSONSchema.md | 14 +++-------- lib/Conch/Route/HardwareProduct.pm | 4 ++-- lib/Conch/Route/JSONSchema.pm | 23 +++---------------- 4 files changed, 10 insertions(+), 35 deletions(-) diff --git a/docs/modules/Conch::Route::HardwareProduct.md b/docs/modules/Conch::Route::HardwareProduct.md index bd7671937..43b09b9a0 100644 --- a/docs/modules/Conch::Route::HardwareProduct.md +++ b/docs/modules/Conch::Route::HardwareProduct.md @@ -83,7 +83,7 @@ Results in this data in `specification`, changing the data type at node `/foo/ba - Requires system admin authorization - Controller/Action: ["set\_specification" in Conch::Controller::HardwareProduct](../modules/Conch%3A%3AController%3A%3AHardwareProduct#set_specification) - Request: after the update operation, the `specification` property must validate against -[common.json#/$defs/HardwareProductSpecification](../json-schema/common.json#/$defs/HardwareProductSpecification). +the schema available from `GET /json_schema/hardware_product/specification/latest`. - Response: `204 No Content` ### `DELETE /hardware_product/:hardware_product_id_or_other/specification?path=:path_to_data` @@ -93,7 +93,7 @@ parameter `path` as the JSON pointer to the data to be removed. All other proper blob are left untouched. After the delete operation, the `specification` property must validate against -[common.json#/$defs/HardwareProductSpecification](../json-schema/common.json#/$defs/HardwareProductSpecification). +the schema available from `GET /json_schema/hardware_product/specification/latest`. - Requires system admin authorization - Controller/Action: ["delete\_specification" in Conch::Controller::HardwareProduct](../modules/Conch%3A%3AController%3A%3AHardwareProduct#delete_specification) diff --git a/docs/modules/Conch::Route::JSONSchema.md b/docs/modules/Conch::Route::JSONSchema.md index f0268dc79..9cac3660d 100644 --- a/docs/modules/Conch::Route::JSONSchema.md +++ b/docs/modules/Conch::Route::JSONSchema.md @@ -56,6 +56,8 @@ Fetches the referenced JSON Schema document. ### `DELETE /json_schema/:json_schema_id` +### `DELETE /json_schema/:json_schema_type/:json_schema_name/:json_schema_version` + Deactivates the database entry for a single JSON Schema, rendering it unusable. This operation is not permitted until all references from other documents have been removed, exception of references using `.../latest` which will now resolve to a different document @@ -70,19 +72,9 @@ If this JSON Schema was the latest of its series (`/json_schema/foo/bar/latest`) ### `GET /json_schema/:json_schema_type` -Gets meta information about all JSON Schemas in a particular type series. - -Optionally accepts the following query parameter: - -- `active_only` (default `0`): set to `1` to only query for JSON Schemas that have not been -deactivated. - -- Controller/Action: ["get\_metadata" in Conch::Controller::JSONSchema](../modules/Conch%3A%3AController%3A%3AJSONSchema#get_metadata) -- Response: [response.json#/$defs/JSONSchemaDescriptions](../json-schema/response.json#/$defs/JSONSchemaDescriptions) - ### `GET /json_schema/:json_schema_type/:json_schema_name` -Gets meta information about all JSON Schemas in a particular type and name series. +Gets meta information about all JSON Schemas in a particular type series, or a type and name series. Optionally accepts the following query parameter: diff --git a/lib/Conch/Route/HardwareProduct.pm b/lib/Conch/Route/HardwareProduct.pm index 2997bbdb2..dc70ab475 100644 --- a/lib/Conch/Route/HardwareProduct.pm +++ b/lib/Conch/Route/HardwareProduct.pm @@ -194,7 +194,7 @@ Results in this data in C, changing the data type at node C =item * Request: after the update operation, the C property must validate against -F. +the schema available from C. =item * Response: C<204 No Content> @@ -207,7 +207,7 @@ parameter C as the JSON pointer to the data to be removed. All other prope blob are left untouched. After the delete operation, the C property must validate against -F. +the schema available from C. =over 4 diff --git a/lib/Conch/Route/JSONSchema.pm b/lib/Conch/Route/JSONSchema.pm index 505593f78..591c8b8f9 100644 --- a/lib/Conch/Route/JSONSchema.pm +++ b/lib/Conch/Route/JSONSchema.pm @@ -149,6 +149,8 @@ Fetches the referenced JSON Schema document. =head2 C +=head2 C + Deactivates the database entry for a single JSON Schema, rendering it unusable. This operation is not permitted until all references from other documents have been removed, exception of references using C<.../latest> which will now resolve to a different document @@ -169,28 +171,9 @@ C<.../latest> link will now resolve to an earlier version in the series. =head2 C -Gets meta information about all JSON Schemas in a particular type series. - -Optionally accepts the following query parameter: - -=over 4 - -=item * C (default C<0>): set to C<1> to only query for JSON Schemas that have not been -deactivated. - -=back - -=over 4 - -=item * Controller/Action: L - -=item * Response: F - -=back - =head2 C -Gets meta information about all JSON Schemas in a particular type and name series. +Gets meta information about all JSON Schemas in a particular type series, or a type and name series. Optionally accepts the following query parameter: From 1a080ece70475503df04d765d3328a785200e80b Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Wed, 6 Jan 2021 15:34:15 -0800 Subject: [PATCH 2/6] use url_for() consistently when setting Location header --- docs/modules/Conch.md | 4 ++++ lib/Conch.pm | 13 ++++++++++++- lib/Conch/Controller/Build.pm | 2 +- lib/Conch/Controller/Datacenter.pm | 2 +- lib/Conch/Controller/DatacenterRoom.pm | 6 +++--- lib/Conch/Controller/Device.pm | 4 ++-- lib/Conch/Controller/DeviceReport.pm | 2 +- lib/Conch/Controller/HardwareProduct.pm | 8 ++++---- lib/Conch/Controller/HardwareVendor.pm | 2 +- lib/Conch/Controller/JSONSchema.pm | 2 +- lib/Conch/Controller/Login.pm | 4 ++-- lib/Conch/Controller/Organization.pm | 2 +- lib/Conch/Controller/Rack.pm | 10 +++++----- lib/Conch/Controller/RackLayout.pm | 6 +++--- lib/Conch/Controller/RackRole.pm | 4 ++-- lib/Conch/Controller/Relay.pm | 2 +- lib/Conch/Controller/User.pm | 6 ++---- 17 files changed, 46 insertions(+), 33 deletions(-) diff --git a/docs/modules/Conch.md b/docs/modules/Conch.md index d4a7e8871..31486340b 100644 --- a/docs/modules/Conch.md +++ b/docs/modules/Conch.md @@ -27,6 +27,10 @@ and therefore other helpers). Helper method for setting the response status code and json content. Calls `$c->render` as a side-effect. +### res\_location + +Simple helper for setting the `Location` header in the response. + ### startup\_time Stores a [Conch::Time](../modules/Conch%3A%3ATime) instance representing the time the server started accepting requests. diff --git a/lib/Conch.pm b/lib/Conch.pm index 5f08eeae0..a4b0e0858 100644 --- a/lib/Conch.pm +++ b/lib/Conch.pm @@ -146,7 +146,7 @@ C<< $c->render >> as a side-effect. } if ($code == 204) { - $c->res->headers->location($payload) if defined $payload; + $c->res_location($payload) if defined $payload; $c->rendered; } elsif (any { $code == $_ } 301, 302, 303, 305, 307, 308) { @@ -163,6 +163,17 @@ C<< $c->render >> as a side-effect. }); +=head2 res_location + +Simple helper for setting the C header in the response. + +=cut + + $self->helper(res_location => sub ($c, @args) { + $c->res->headers->location($c->url_for(@args)); + }); + + $self->hook(before_routes => sub ($c) { my $headers = $c->req->headers; diff --git a/lib/Conch/Controller/Build.pm b/lib/Conch/Controller/Build.pm index e6500fc70..8593294c6 100644 --- a/lib/Conch/Controller/Build.pm +++ b/lib/Conch/Controller/Build.pm @@ -91,7 +91,7 @@ sub create ($c) { user_build_roles => [ map +{ user_id => $_->[2], role => 'admin' }, @admins ], }); $c->log->info('created build '.$build->id.' ('.$build->name.')'); - $c->res->headers->location('/build/'.$build->id); + $c->res_location('/build/'.$build->id); $c->status(201); } diff --git a/lib/Conch/Controller/Datacenter.pm b/lib/Conch/Controller/Datacenter.pm index fa0a7c782..bfade7656 100644 --- a/lib/Conch/Controller/Datacenter.pm +++ b/lib/Conch/Controller/Datacenter.pm @@ -95,7 +95,7 @@ sub create ($c) { my $dc = $c->db_datacenters->create($input); $c->log->debug('Created datacenter '.$dc->id); - $c->res->headers->location('/dc/'.$dc->id); + $c->res_location('/dc/'.$dc->id); $c->status(201); } diff --git a/lib/Conch/Controller/DatacenterRoom.pm b/lib/Conch/Controller/DatacenterRoom.pm index ba5201ad1..2ba1c8725 100644 --- a/lib/Conch/Controller/DatacenterRoom.pm +++ b/lib/Conch/Controller/DatacenterRoom.pm @@ -73,7 +73,7 @@ Response uses the DatacenterRoomDetailed json schema. sub get_one ($c) { my $room = $c->stash('datacenter_room_rs')->single; - $c->res->headers->location('/room/'.$room->id); + $c->res_location('/room/'.$room->id); $c->status(200, $room); } @@ -97,7 +97,7 @@ sub create ($c) { my $room = $c->db_datacenter_rooms->create($input); $c->log->debug('Created datacenter room '.$room->id); - $c->res->headers->location('/room/'.$room->id); + $c->res_location('/room/'.$room->id); $c->status(201); } @@ -124,7 +124,7 @@ sub update ($c) { if $input->{vendor_name} and $input->{vendor_name} ne $room->vendor_name and $c->db_datacenter_rooms->search({ vendor_name => $input->{vendor_name} })->exists; - $c->res->headers->location('/room/'.$room->id); + $c->res_location('/room/'.$room->id); $room->set_columns($input); return $c->status(204) if not $room->is_changed; diff --git a/lib/Conch/Controller/Device.pm b/lib/Conch/Controller/Device.pm index 12ce0947a..d5122146b 100644 --- a/lib/Conch/Controller/Device.pm +++ b/lib/Conch/Controller/Device.pm @@ -113,12 +113,12 @@ sub find_device ($c) { if (my $bad_phase = $params->{phase_earlier_than} // $c->stash('phase_earlier_than')) { my $phase = $c->stash('device_rs')->get_column('phase')->single; if (Conch::DB::Result::Device->phase_cmp($phase, $bad_phase) >= 0) { - $c->res->headers->location($c->url_for('/device/'.$device_id.'/links')); + $c->res_location('/device/'.$device_id.'/links'); return $c->status(409, { error => 'device is in the '.$phase.' phase' }); } } - $c->res->headers->location('/device/'.$device_id); + $c->res_location('/device/'.$device_id); return 1; } diff --git a/lib/Conch/Controller/DeviceReport.pm b/lib/Conch/Controller/DeviceReport.pm index 22f82cbf2..8dcd180a1 100644 --- a/lib/Conch/Controller/DeviceReport.pm +++ b/lib/Conch/Controller/DeviceReport.pm @@ -181,7 +181,7 @@ sub process ($c) { } } - $c->res->headers->location($c->url_for('/validation_state/'.$validation_state->id)); + $c->res_location('/validation_state/'.$validation_state->id); $c->status(201); } diff --git a/lib/Conch/Controller/HardwareProduct.pm b/lib/Conch/Controller/HardwareProduct.pm index c0df6f273..4f9cb5e94 100644 --- a/lib/Conch/Controller/HardwareProduct.pm +++ b/lib/Conch/Controller/HardwareProduct.pm @@ -85,7 +85,7 @@ Response uses the HardwareProduct json schema. sub get ($c) { my $hardware_product = $c->stash('hardware_product_rs')->single; - $c->res->headers->location('/hardware_product/'.$hardware_product->id); + $c->res_location('/hardware_product/'.$hardware_product->id); $c->status(200, $hardware_product); } @@ -122,7 +122,7 @@ sub create ($c) { return $c->status(400) if not $hardware_product; $c->log->debug('Created hardware product id '.$hardware_product->id); - $c->res->headers->location('/hardware_product/'.$hardware_product->id); + $c->res_location('/hardware_product/'.$hardware_product->id); $c->status(201); } @@ -232,7 +232,7 @@ sub set_specification ($c) { return $rendered ? () : $c->status(400) if not $result; - $c->res->headers->location('/hardware_product/'.$hardware_product_id); + $c->res_location('/hardware_product/'.$hardware_product_id); $c->status(204); } @@ -284,7 +284,7 @@ sub delete_specification ($c) { return $rendered ? () : $c->status(400) if not $result; - $c->res->headers->location('/hardware_product/'.$hardware_product_id); + $c->res_location('/hardware_product/'.$hardware_product_id); $c->status(204); } diff --git a/lib/Conch/Controller/HardwareVendor.pm b/lib/Conch/Controller/HardwareVendor.pm index b538906dc..82c602935 100644 --- a/lib/Conch/Controller/HardwareVendor.pm +++ b/lib/Conch/Controller/HardwareVendor.pm @@ -83,7 +83,7 @@ sub create ($c) { my $hardware_vendor = $c->db_hardware_vendors->create({ name => $c->stash('hardware_vendor_name') }); $c->log->debug('Created hardware vendor '.$c->stash('hardware_vendor_name')); - $c->res->headers->location('/hardware_vendor/'.$hardware_vendor->id); + $c->res_location('/hardware_vendor/'.$hardware_vendor->id); $c->status(201); } diff --git a/lib/Conch/Controller/JSONSchema.pm b/lib/Conch/Controller/JSONSchema.pm index bc97da99d..f521164b3 100644 --- a/lib/Conch/Controller/JSONSchema.pm +++ b/lib/Conch/Controller/JSONSchema.pm @@ -183,7 +183,7 @@ sub create ($c) { my $path = $row->canonical_path; $c->log->info('created json schema '.$row->id.' at '.$path); - $c->res->headers->location($c->url_for('/json_schema/'.$row->id)); + $c->res_location('/json_schema/'.$row->id); $c->res->headers->content_location($c->url_for($path)); $c->status(201); } diff --git a/lib/Conch/Controller/Login.pm b/lib/Conch/Controller/Login.pm index b66a4da98..3d4fb5038 100644 --- a/lib/Conch/Controller/Login.pm +++ b/lib/Conch/Controller/Login.pm @@ -138,7 +138,7 @@ sub _check_authentication ($c) { $user->user_session_tokens->login_only ->update({ expires => \'least(expires, now() + interval \'10 minutes\')' }) if $session_token; - $c->res->headers->location($c->url_for('/user/me/password?clear_tokens=none')); + $c->res_location('/user/me/password?clear_tokens=none'); return 0; } @@ -203,7 +203,7 @@ sub login ($c) { $c->_update_session($user->id, $input->{set_session} ? time + 10 * 60 : 0); # we logged the user in, but he must now change his password (within 10 minutes) - $c->res->headers->location($c->url_for('/user/me/password?clear_tokens=none')); + $c->res_location('/user/me/password?clear_tokens=none'); return $c->_respond_with_jwt($user->id, time + 10 * 60); } diff --git a/lib/Conch/Controller/Organization.pm b/lib/Conch/Controller/Organization.pm index d4057cfe3..420dfad01 100644 --- a/lib/Conch/Controller/Organization.pm +++ b/lib/Conch/Controller/Organization.pm @@ -82,7 +82,7 @@ sub create ($c) { user_organization_roles => [ map +{ user_id => $_->[2], role => 'admin' }, @admins ], }); $c->log->info('created organization '.$organization->id.' ('.$organization->name.')'); - $c->res->headers->location('/organization/'.$organization->id); + $c->res_location('/organization/'.$organization->id); $c->status(201); } diff --git a/lib/Conch/Controller/Rack.pm b/lib/Conch/Controller/Rack.pm index 02c2625a5..1a053b847 100644 --- a/lib/Conch/Controller/Rack.pm +++ b/lib/Conch/Controller/Rack.pm @@ -118,7 +118,7 @@ sub create ($c) { my $rack = $c->db_racks->create($input); $c->log->debug('Created rack '.$rack->id); - $c->res->headers->location('/rack/'.$rack->id); + $c->res_location('/rack/'.$rack->id); $c->status(201); } @@ -131,7 +131,7 @@ Response uses the Rack json schema. =cut sub get ($c) { - $c->res->headers->location('/rack/'.$c->stash('rack_id')); + $c->res_location('/rack/'.$c->stash('rack_id')); my $rs = $c->stash('rack_rs') ->with_build_name ->with_full_rack_name @@ -160,7 +160,7 @@ sub get_layouts ($c) { ->all; $c->log->debug('Found '.scalar(@layouts).' rack layouts'); - $c->res->headers->location('/rack/'.$c->stash('rack_id').'/layout'); + $c->res_location('/rack/'.$c->stash('rack_id').'/layout'); $c->status(200, \@layouts); } @@ -296,7 +296,7 @@ sub update ($c) { } } - $c->res->headers->location('/rack/'.$rack->id); + $c->res_location('/rack/'.$rack->id); $rack->set_columns($input); return $c->status(204) if not $rack->is_changed; @@ -348,7 +348,7 @@ sub get_assignment ($c) { ->all; $c->log->debug('Found '.scalar(@assignments).' device-rack assignments'); - $c->res->headers->location('/rack/'.$c->stash('rack_id').'/assignment'); + $c->res_location('/rack/'.$c->stash('rack_id').'/assignment'); $c->status(200, \@assignments); } diff --git a/lib/Conch/Controller/RackLayout.pm b/lib/Conch/Controller/RackLayout.pm index 9d043c883..72bafebbe 100644 --- a/lib/Conch/Controller/RackLayout.pm +++ b/lib/Conch/Controller/RackLayout.pm @@ -111,7 +111,7 @@ sub create ($c) { my $layout = $c->db_rack_layouts->create($input); $c->log->debug('Created rack layout '.$layout->id); - $c->res->headers->location('/layout/'.$layout->id); + $c->res_location('/layout/'.$layout->id); $c->status(201); } @@ -130,7 +130,7 @@ sub get ($c) { ->with_sku ->single; - $c->res->headers->location('/layout/'.$layout->id); + $c->res_location('/layout/'.$layout->id); $c->status(200, $layout); } @@ -237,7 +237,7 @@ sub update ($c) { return $c->status(409, { error => 'rack_unit_start conflict' }); } - $c->res->headers->location('/layout/'.$layout->id); + $c->res_location('/layout/'.$layout->id); $layout->set_columns($input); return $c->status(204) if not $layout->is_changed; diff --git a/lib/Conch/Controller/RackRole.pm b/lib/Conch/Controller/RackRole.pm index a3376aa04..8b72e0e4a 100644 --- a/lib/Conch/Controller/RackRole.pm +++ b/lib/Conch/Controller/RackRole.pm @@ -58,7 +58,7 @@ sub create ($c) { my $rack_role = $c->db_rack_roles->create($input); $c->log->debug('Created rack role '.$rack_role->id); - $c->res->headers->location('/rack_role/'.$rack_role->id); + $c->res_location('/rack_role/'.$rack_role->id); $c->status(201); } @@ -125,7 +125,7 @@ sub update ($c) { } } - $c->res->headers->location('/rack_role/'.$rack_role->id); + $c->res_location('/rack_role/'.$rack_role->id); $rack_role->set_columns($input); return $c->status(204) if not $rack_role->is_changed; diff --git a/lib/Conch/Controller/Relay.pm b/lib/Conch/Controller/Relay.pm index 9dd613ea7..bc9ceb68e 100644 --- a/lib/Conch/Controller/Relay.pm +++ b/lib/Conch/Controller/Relay.pm @@ -42,7 +42,7 @@ sub register ($c) { $c->status(204); } - $c->res->headers->location($c->url_for('/relay/'.$relay->id)); + $c->res_location('/relay/'.$relay->id); return; } diff --git a/lib/Conch/Controller/User.pm b/lib/Conch/Controller/User.pm index c9468bda9..dfaa742c9 100644 --- a/lib/Conch/Controller/User.pm +++ b/lib/Conch/Controller/User.pm @@ -473,7 +473,7 @@ sub create ($c) { ); } - $c->res->headers->location($c->url_for('/user/'.$user->id)); + $c->res_location('/user/'.$user->id); return $c->status(201, { map +($_ => $user->$_), qw(id email name) }); } @@ -583,9 +583,7 @@ sub create_api_token ($c) { $c->res->headers->last_modified(Mojo::Date->new($token->created->epoch)); $c->res->headers->expires(Mojo::Date->new($token->expires->epoch)); - $c->res->headers->location($c->url_for('/user/' - .($user->id eq $c->stash('user_id') ? 'me' : $user->id) - .'/token/'.$input->{name})); + $c->res_location('/user/'.($user->id eq $c->stash('user_id') ? 'me' : $user->id).'/token/'.$input->{name}); my $token_data = $token->TO_JSON; delete $token_data->{last_ipaddr}; From f590e4a6397dde93f5e8446e422fc3e46dd7839c Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Tue, 5 Jan 2021 16:21:44 -0800 Subject: [PATCH 3/6] assert that json booleans are real booleans, not integer 0, 1 --- t/integration/crud/devices.t | 4 ++-- t/integration/json_schema-unauthed.t | 22 +++++++++++----------- t/integration/users.t | 6 +++--- t/json-validation.t | 8 ++++---- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/t/integration/crud/devices.t b/t/integration/crud/devices.t index f19fc5f4c..f67416d33 100644 --- a/t/integration/crud/devices.t +++ b/t/integration/crud/devices.t @@ -206,7 +206,7 @@ subtest 'unlocated device with a registered relay' => sub { }, 'response.yaml#/$defs/DetailedDevice')->TO_JSON, { - valid => bool(0), + valid => JSON::PP::false, errors => [ superhashof({ error => 'missing property: location' }) ], }, 'lack of location data is rejected by the response schema when phase=integration', @@ -217,7 +217,7 @@ subtest 'unlocated device with a registered relay' => sub { { $test_device_data->%*, phase => 'production' }, 'response.yaml#/$defs/DetailedDevice')->TO_JSON, { - valid => bool(0), + valid => JSON::PP::false, errors => [ map +{ instanceLocation => '/'.$_, diff --git a/t/integration/json_schema-unauthed.t b/t/integration/json_schema-unauthed.t index a35ec182f..772140a91 100644 --- a/t/integration/json_schema-unauthed.t +++ b/t/integration/json_schema-unauthed.t @@ -47,7 +47,7 @@ $t->get_ok('/json_schema/response/Ping' => { 'If-Modified-Since' => 'Sun, 01 Jan '$schema' => SPEC_URL, '$id' => $base_uri.'json_schema/response/Ping', type => 'object', - additionalProperties => bool(0), + additionalProperties => JSON::PP::false, required => ['status'], properties => { status => { const => 'ok' } }, }); @@ -69,7 +69,7 @@ $t->get_ok('/schema/response/login_token') '$schema' => SPEC_URL, '$id' => $base_uri.'json_schema/response/LoginToken', type => 'object', - additionalProperties => bool(0), + additionalProperties => JSON::PP::false, required => ['jwt_token'], properties => { jwt_token => { type => 'string', @@ -93,7 +93,7 @@ $t->get_ok('/json_schema/request/Login') '$schema' => SPEC_URL, '$id' => $base_uri.'json_schema/request/Login', type => 'object', - unevaluatedProperties => bool(0), + unevaluatedProperties => JSON::PP::false, required => [ 'password' ], properties => { password => { '$ref' => '/json_schema/common/non_empty_string' }, @@ -117,7 +117,7 @@ $t->get_ok('/json_schema/request/Login') UserIdOrEmail => { '$id' => '/json_schema/request/UserIdOrEmail', type => 'object', - additionalProperties => bool(1), + additionalProperties => JSON::PP::true, oneOf => [ { required => [ 'user_id' ] }, { required => [ 'email' ] } ], properties => { user_id => { '$ref' => '/json_schema/common/uuid' }, @@ -125,7 +125,7 @@ $t->get_ok('/json_schema/request/Login') }, }, }, - default => { set_session => bool(0) }, + default => { set_session => JSON::PP::false }, }); $t->get_ok('/json_schema/query_params/ResetUserPassword') @@ -142,7 +142,7 @@ $t->get_ok('/json_schema/query_params/ResetUserPassword') }, }, type => 'object', - additionalProperties => bool(0), + additionalProperties => JSON::PP::false, properties => { clear_tokens => { enum => [ qw(none login_only all) ] }, send_mail => { '$ref' => '/json_schema/query_params/boolean_string' }, @@ -202,12 +202,12 @@ $t->get_ok('/json_schema/response/JSONSchemaOnDisk') '$comment' => ignore, '$id' => ignore, # we just need there to be something here '$ref' => SPEC_URL, - '$recursiveAnchor' => bool(1), + '$recursiveAnchor' => JSON::PP::true, '$ref' => SPEC_URL, properties => { '$schema' => { const => SPEC_URL }, }, - unevaluatedProperties => bool(0), + unevaluatedProperties => JSON::PP::false, }, { '$comment' => ignore, @@ -232,13 +232,13 @@ $t->get_ok('/json_schema/response/JSONSchema') '$comment' => ignore, '$id' => ignore, # we just need there to be something here '$ref' => SPEC_URL, - '$recursiveAnchor' => bool(1), + '$recursiveAnchor' => JSON::PP::true, '$ref' => SPEC_URL, properties => { '$schema' => { const => SPEC_URL }, 'x-json_schema_id' => { '$ref' => '/json_schema/common/uuid' }, }, - unevaluatedProperties => bool(0), + unevaluatedProperties => JSON::PP::false, }, { '$comment' => ignore, @@ -301,7 +301,7 @@ $t->get_ok('/json_schema/device_report/DeviceReport_v3_0_0') '$schema' => SPEC_URL, '$comment' => ignore, type => 'object', - additionalProperties => bool(1), + additionalProperties => JSON::PP::true, required => ignore, properties => superhashof({}), '$defs' => { diff --git a/t/integration/users.t b/t/integration/users.t index 50681dcb9..8bbaf253c 100644 --- a/t/integration/users.t +++ b/t/integration/users.t @@ -329,7 +329,7 @@ subtest 'User' => sub { ->json_schema_is('UserDetailed') ->json_cmp_deeply({ $user_detailed->%*, - refuse_session_auth => bool(1), + refuse_session_auth => JSON::PP::true, last_login => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/), last_seen => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/), }); @@ -342,7 +342,7 @@ subtest 'User' => sub { ->json_schema_is('UserDetailed') ->json_cmp_deeply({ $user_detailed->%*, - refuse_session_auth => bool(1), + refuse_session_auth => JSON::PP::true, last_login => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/), last_seen => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/), }); @@ -425,7 +425,7 @@ subtest 'User' => sub { ->json_schema_is('UserDetailed') ->json_cmp_deeply({ $user_detailed->%*, - refuse_session_auth => bool(1), + refuse_session_auth => JSON::PP::true, last_login => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/), last_seen => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/), }); diff --git a/t/json-validation.t b/t/json-validation.t index 38500a5cf..cae2dcbf1 100644 --- a/t/json-validation.t +++ b/t/json-validation.t @@ -86,7 +86,7 @@ subtest '/device/:id/interface/:iface_name/:field validation' => sub { cmp_deeply( $validator->evaluate({ device_id => create_uuid_str() }, $schema)->TO_JSON, { - valid => bool(0), + valid => JSON::PP::false, errors => [ { instanceLocation => '/device_id', @@ -102,7 +102,7 @@ subtest '/device/:id/interface/:iface_name/:field validation' => sub { cmp_deeply( $validator->evaluate({ created => '2018-01-02T00:00:00.000+00:20' }, $schema)->TO_JSON, { - valid => bool(0), + valid => JSON::PP::false, errors => [ { instanceLocation => '/created', @@ -128,7 +128,7 @@ subtest 'device report validation' => sub { $validator->evaluate('00000000-0000-0000-0000-000000000000', 'device_report.yaml#/$defs/DeviceReport_v3_0_0/properties/system_uuid')->TO_JSON, { - valid => bool(0), + valid => JSON::PP::false, errors => [ { instanceLocation => '', @@ -145,7 +145,7 @@ subtest 'device report validation' => sub { $validator->evaluate({ '' => {} }, 'device_report.yaml#/$defs/DeviceReport_v3_0_0/properties/disks')->TO_JSON, { - valid => bool(0), + valid => JSON::PP::false, errors => [ { instanceLocation => '/', From 48931307567115f9738f28927691a7031a858e40 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Mon, 4 Jan 2021 12:13:15 -0800 Subject: [PATCH 4/6] add more annotations to assist clients when generating forms and results --- docs/json-schema/request.json | 12 +++++++++--- json-schema/request.yaml | 6 ++++++ t/integration/json_schema-unauthed.t | 2 +- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/json-schema/request.json b/docs/json-schema/request.json index 04ad6e329..06d2a97cf 100644 --- a/docs/json-schema/request.json +++ b/docs/json-schema/request.json @@ -720,7 +720,9 @@ }, "properties" : { "password" : { - "$ref" : "common.json#/$defs/non_empty_string" + "$ref" : "common.json#/$defs/non_empty_string", + "title" : "Password", + "writeOnly" : true }, "set_session" : { "type" : "boolean" @@ -745,7 +747,9 @@ "$ref" : "common.json#/$defs/non_empty_string" }, "password" : { - "$ref" : "common.json#/$defs/non_empty_string" + "$ref" : "common.json#/$defs/non_empty_string", + "title" : "Password", + "writeOnly" : true } }, "required" : [ @@ -1168,7 +1172,9 @@ "additionalProperties" : false, "properties" : { "password" : { - "$ref" : "common.json#/$defs/non_empty_string" + "$ref" : "common.json#/$defs/non_empty_string", + "title" : "Password", + "writeOnly" : true } }, "required" : [ diff --git a/json-schema/request.yaml b/json-schema/request.yaml index 381a3dd6d..7f9a5474b 100644 --- a/json-schema/request.yaml +++ b/json-schema/request.yaml @@ -411,6 +411,8 @@ $defs: required: [ password ] properties: password: + title: Password + writeOnly: true $ref: common.yaml#/$defs/non_empty_string set_session: type: boolean @@ -433,6 +435,8 @@ $defs: required: [ password ] properties: password: + title: Password + writeOnly: true $ref: common.yaml#/$defs/non_empty_string NewUser: type: object @@ -444,6 +448,8 @@ $defs: email: $ref: common.yaml#/$defs/email_address password: + title: Password + writeOnly: true $ref: common.yaml#/$defs/non_empty_string is_admin: type: boolean diff --git a/t/integration/json_schema-unauthed.t b/t/integration/json_schema-unauthed.t index 772140a91..cb3d2446e 100644 --- a/t/integration/json_schema-unauthed.t +++ b/t/integration/json_schema-unauthed.t @@ -96,7 +96,7 @@ $t->get_ok('/json_schema/request/Login') unevaluatedProperties => JSON::PP::false, required => [ 'password' ], properties => { - password => { '$ref' => '/json_schema/common/non_empty_string' }, + password => { title => 'Password', writeOnly => JSON::PP::true, '$ref' => '/json_schema/common/non_empty_string' }, set_session => { type => 'boolean' }, }, '$ref' => '/json_schema/request/UserIdOrEmail', From f0d52d802bb2b06bf9bbec82e56774e66389edf5 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Mon, 14 Dec 2020 13:51:42 -0800 Subject: [PATCH 5/6] try a bit harder to surface unexpected exceptions --- lib/Test/Conch.pm | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/Test/Conch.pm b/lib/Test/Conch.pm index d292f5fc0..f45fd7f6e 100644 --- a/lib/Test/Conch.pm +++ b/lib/Test/Conch.pm @@ -611,12 +611,11 @@ subtest and is invoked with the test object as well as any additional provided a =cut sub txn_local ($self, $test_name, $subref, @args) { - $self->app->schema->txn_begin; - + $self->app->txn_wrapper(sub { local $Test::Builder::Level = $Test::Builder::Level + 1; Test::More::subtest($test_name => $subref, $self, @args); - - $self->app->schema->txn_rollback; + die 'rollback'; + }); } =head2 email_cmp_deeply @@ -807,6 +806,7 @@ sub reset_log ($self) { sub _request_ok ($self, @args) { undef $self->{_mail_composed}; $self->reset_log; + $self->stash(undef); my $result = $self->next::method(@args); my $dump_log; @@ -821,6 +821,9 @@ sub _request_ok ($self, @args) { Data::Dumper->new([ $log_history ])->Sortkeys(1)->Indent(1)->Terse(1)->Dump if $dump_log || $self->tx->res->code == 500 && $self->tx->req->url->path !~ qr{^/_die}; + my $stash = $self->stash; + $self->test('note', 'request died: '.$stash->{exception}->message) if exists $stash->{exception}; + return $result; } From e7ec448b95a9b1c8cc51102bc9bbf4657a9f7568 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Mon, 14 Dec 2020 13:52:37 -0800 Subject: [PATCH 6/6] also test POST /device_report?no_save_db=1 with existing devices This fixes a longstanding unnoticed bug where existing devices could not be used with POST /device_report?no_save_db=1 --- lib/Conch/Controller/DeviceReport.pm | 5 ++--- t/integration/device-reports.t | 23 ++++++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/Conch/Controller/DeviceReport.pm b/lib/Conch/Controller/DeviceReport.pm index 8dcd180a1..d20f0355b 100644 --- a/lib/Conch/Controller/DeviceReport.pm +++ b/lib/Conch/Controller/DeviceReport.pm @@ -425,14 +425,13 @@ sub validate_report ($c) { my ($status, @validation_results); $c->txn_wrapper(sub ($c) { if ($device) { - $c->db_devices->update({ + $device->update({ serial_number => $unserialized_report->{serial_number}, system_uuid => $unserialized_report->{system_uuid}, uptime_since => $unserialized_report->{uptime_since}, hostname => $unserialized_report->{os}{hostname}, updated => \'now()', - }, - { key => 'device_serial_number_key' }); + }); } else { $device = $c->db_devices->create({ diff --git a/t/integration/device-reports.t b/t/integration/device-reports.t index f34ef47f4..4c56c0090 100644 --- a/t/integration/device-reports.t +++ b/t/integration/device-reports.t @@ -24,6 +24,8 @@ my $report = path('t/integration/resource/passing-device-report.json')->slurp_ut # matches report's product_name = Joyent-G1 (compute) my $hardware_product; +my $device2 = $t->generate_fixtures('device'); + subtest preliminaries => sub { my $report_data = from_json($report); @@ -75,10 +77,15 @@ my ($full_validation_plan) = $t->load_validation_plans([{ validations => [ map $_->module, @validations ], }]); -subtest 'run report without an existing device and without making updates' => sub { +subtest 'run report without making updates' => sub { my $report_data = from_json($report); - $report_data->{serial_number} = 'different_device'; - $report_data->{system_uuid} = create_uuid_str(); + + my %system_uuid = ( + TEST => $report_data->{system_uuid}, + DEADBEEF => create_uuid_str(), + ); + + $report_data->@{qw(serial_number system_uuid)} = %system_uuid{DEADBEEF}; $t->txn_local('hardware_product must not be deactivated', sub { $hardware_product->update({ deactivated => \'now()' }); @@ -96,11 +103,16 @@ subtest 'run report without an existing device and without making updates' => su ->json_is({ error => 'legacy_validation_plan (id '.$hardware_product->legacy_validation_plan_id.') is deactivated and cannot be used' }); }); - $t->post_ok('/device_report?no_save_db=1', json => $report_data) + foreach my $serial (qw(DEADBEEF TEST)) { + $t->post_ok('/device_report?no_save_db=1', json => { + %$report_data, + serial_number => $serial, + system_uuid => $system_uuid{$serial}, + }) ->status_is(200) ->json_schema_is('ReportValidationResults') ->json_cmp_deeply({ - device_serial_number => 'different_device', + device_serial_number => $serial, hardware_product_id => $hardware_product->id, sku => $hardware_product->sku, status => any(qw(error fail pass)), # likely some validations will hate this report. @@ -116,6 +128,7 @@ subtest 'run report without an existing device and without making updates' => su do { my $v = $_; map +($_ => $v->$_), qw(name version description) }, }, @validations)), }); + } ok(!$t->app->db_devices->search({ serial_number => 'different_device' })->exists, 'the device was not inserted into the database');