From 8806a80b38f56bea5145b9c111ec34b7646d4795 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Wed, 2 Dec 2020 16:45:17 -0800 Subject: [PATCH] make hardware_product.specification not nullable ..and make the tests look a little more like reality --- docs/json-schema/request.json | 9 +-- docs/json-schema/response.json | 9 +-- .../Conch::DB::Result::HardwareProduct.md | 3 +- json-schema/request.yaml | 4 +- json-schema/response.yaml | 4 +- lib/Conch/DB/Result/HardwareProduct.pm | 7 +- ...ardware_product-specification-not-null.sql | 8 ++ sql/schema.sql | 2 +- t/integration/crud/hardware-product.t | 75 +++++++++++++++---- 9 files changed, 78 insertions(+), 43 deletions(-) create mode 100644 sql/migrations/0179-hardware_product-specification-not-null.sql diff --git a/docs/json-schema/request.json b/docs/json-schema/request.json index 1a66e09c3..3afe40818 100644 --- a/docs/json-schema/request.json +++ b/docs/json-schema/request.json @@ -640,14 +640,7 @@ "title" : "SKU" }, "specification" : { - "oneOf" : [ - { - "$ref" : "common.json#/$defs/HardwareProductSpecification" - }, - { - "type" : "null" - } - ], + "$ref" : "common.json#/$defs/HardwareProductSpecification", "title" : "Specification" }, "usb_num" : { diff --git a/docs/json-schema/response.json b/docs/json-schema/response.json index d38e61b8e..54959bc44 100644 --- a/docs/json-schema/response.json +++ b/docs/json-schema/response.json @@ -1450,14 +1450,7 @@ "title" : "SKU" }, "specification" : { - "oneOf" : [ - { - "$ref" : "common.json#/$defs/HardwareProductSpecification" - }, - { - "type" : "null" - } - ], + "$ref" : "common.json#/$defs/HardwareProductSpecification", "title" : "Specification" }, "updated" : { diff --git a/docs/modules/Conch::DB::Result::HardwareProduct.md b/docs/modules/Conch::DB::Result::HardwareProduct.md index da8e19a06..117b7c7c7 100644 --- a/docs/modules/Conch::DB::Result::HardwareProduct.md +++ b/docs/modules/Conch::DB::Result::HardwareProduct.md @@ -78,7 +78,8 @@ original: {default_value => \"now()"} ``` data_type: 'jsonb' -is_nullable: 1 +default_value: '{}' +is_nullable: 0 ``` ### sku diff --git a/json-schema/request.yaml b/json-schema/request.yaml index c7711c867..9d73badec 100644 --- a/json-schema/request.yaml +++ b/json-schema/request.yaml @@ -258,9 +258,7 @@ $defs: $ref: common.yaml#/$defs/uuid specification: title: Specification - oneOf: - - $ref: common.yaml#/$defs/HardwareProductSpecification - - type: 'null' + $ref: common.yaml#/$defs/HardwareProductSpecification sku: title: SKU $ref: common.yaml#/$defs/mojo_standard_placeholder diff --git a/json-schema/response.yaml b/json-schema/response.yaml index 58c2106ad..60ece2b46 100644 --- a/json-schema/response.yaml +++ b/json-schema/response.yaml @@ -654,9 +654,7 @@ $defs: $ref: common.yaml#/$defs/mojo_standard_placeholder specification: title: Specification - oneOf: - - $ref: common.yaml#/$defs/HardwareProductSpecification - - type: 'null' + $ref: common.yaml#/$defs/HardwareProductSpecification rack_unit_size: title: Rack Unit Size (RU) $ref: common.yaml#/$defs/positive_integer diff --git a/lib/Conch/DB/Result/HardwareProduct.pm b/lib/Conch/DB/Result/HardwareProduct.pm index 25c333e6d..e7e950fad 100644 --- a/lib/Conch/DB/Result/HardwareProduct.pm +++ b/lib/Conch/DB/Result/HardwareProduct.pm @@ -79,7 +79,8 @@ __PACKAGE__->table("hardware_product"); =head2 specification data_type: 'jsonb' - is_nullable: 1 + default_value: '{}' + is_nullable: 0 =head2 sku @@ -285,7 +286,7 @@ __PACKAGE__->add_columns( original => { default_value => \"now()" }, }, "specification", - { data_type => "jsonb", is_nullable => 1 }, + { data_type => "jsonb", default_value => "{}", is_nullable => 0 }, "sku", { data_type => "text", is_nullable => 0 }, "generation_name", @@ -441,7 +442,7 @@ __PACKAGE__->has_many( # Created by DBIx::Class::Schema::Loader v0.07049 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:9Q3v8Ag2aFFkitFegf0t2g +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:jh81nspu+x8IBhfHU063jQ use experimental 'signatures'; use Mojo::JSON 'from_json'; diff --git a/sql/migrations/0179-hardware_product-specification-not-null.sql b/sql/migrations/0179-hardware_product-specification-not-null.sql new file mode 100644 index 000000000..de903572e --- /dev/null +++ b/sql/migrations/0179-hardware_product-specification-not-null.sql @@ -0,0 +1,8 @@ +SELECT run_migration(179, $$ + + update hardware_product set specification = '{}' where specification is null; + + alter table hardware_product alter column specification set default '{}', + alter column specification set not null; + +$$); diff --git a/sql/schema.sql b/sql/schema.sql index 232e4c753..4878c44e3 100644 --- a/sql/schema.sql +++ b/sql/schema.sql @@ -380,7 +380,7 @@ CREATE TABLE public.hardware_product ( deactivated timestamp with time zone, created timestamp with time zone DEFAULT now() NOT NULL, updated timestamp with time zone DEFAULT now() NOT NULL, - specification jsonb, + specification jsonb DEFAULT '{}'::jsonb NOT NULL, sku text NOT NULL, generation_name text, legacy_product_name text, diff --git a/t/integration/crud/hardware-product.t b/t/integration/crud/hardware-product.t index f4572c162..cff644406 100644 --- a/t/integration/crud/hardware-product.t +++ b/t/integration/crud/hardware-product.t @@ -258,20 +258,24 @@ $t->get_ok('/hardware_product/sungo') ->status_is(409) ->json_is({ error => 'there is more than one match' }); +my $base_specification = $hw_fields{specification} = { + disk_size => { _default => 0, AcmeCorp => 512 }, +}; + subtest 'manipulate hardware_product.specification' => sub { $t->put_ok('/hardware_product/'.$new_hw_id.'/specification', json => {}) ->status_is(400) ->json_schema_is('QueryParamsValidationError') ->json_cmp_deeply('/details', [ superhashof({ error => 'missing property: path' }) ]); - $t->put_ok('/hardware_product/'.$new_hw_id.'/specification?path=', json => {}) + $t->put_ok('/hardware_product/'.$new_hw_id.'/specification?path=', json => $base_specification) ->status_is(204) ->location_is('/hardware_product/'.$new_hw_id); $t->get_ok('/hardware_product/'.$new_hw_id) ->status_is(200) ->json_schema_is('HardwareProduct') - ->json_cmp_deeply(superhashof({ specification => {} })); + ->json_cmp_deeply(superhashof({ specification => $base_specification })); $t->put_ok('/hardware_product/'.$new_hw_id.'/specification?path=', json => 'hello') ->status_is(400) @@ -298,7 +302,10 @@ subtest 'manipulate hardware_product.specification' => sub { ->status_is(200) ->json_schema_is('HardwareProduct') ->json_cmp_deeply(superhashof({ - specification => { disk_size => { _default => 128 } }, + specification => { + $base_specification->%*, + disk_size => { _default => 128 }, + }, })); $t->put_ok('/hardware_product/'.$new_hw_id.'/specification?path=/disk_size/SEAGATE_8000', @@ -319,7 +326,10 @@ subtest 'manipulate hardware_product.specification' => sub { ->status_is(200) ->json_schema_is('HardwareProduct') ->json_cmp_deeply(superhashof({ - specification => { disk_size => { _default => 128, SEAGATE_8000 => 1 } }, + specification => { + $base_specification->%*, + disk_size => { _default => 128, SEAGATE_8000 => 1 }, + }, })); $t->put_ok('/hardware_product/'.$new_hw_id.'/specification?path=/disk_size/_default', json => 64) @@ -330,7 +340,10 @@ subtest 'manipulate hardware_product.specification' => sub { ->status_is(200) ->json_schema_is('HardwareProduct') ->json_cmp_deeply(superhashof({ - specification => { disk_size => { _default => 64, SEAGATE_8000 => 1 } }, + specification => { + $base_specification->%*, + disk_size => { _default => 64, SEAGATE_8000 => 1 }, + }, })); # the path we want to operate on is called .../~1~device/ and encodes as .../~01~0device/... @@ -347,14 +360,18 @@ subtest 'manipulate hardware_product.specification' => sub { ->status_is(200) ->json_schema_is('HardwareProduct') ->json_cmp_deeply(superhashof({ - specification => { disk_size => { - _default => 64, - SEAGATE_8000 => 1, - 'tilde~1~device' => 2, - } }, + specification => { + $base_specification->%*, + disk_size => { + _default => 64, + SEAGATE_8000 => 1, + 'tilde~1~device' => 2, + }, + }, })); - $t->put_ok('/hardware_product/'.$new_hw_id.'/specification?path=/other_prop', json => [ 1,2,3 ]) + $t->put_ok('/hardware_product/'.$new_hw_id.'/specification?path=/disks', + json => { usb_hdd_num => 0 }) ->status_is(204) ->location_is('/hardware_product/'.$new_hw_id); @@ -363,8 +380,25 @@ subtest 'manipulate hardware_product.specification' => sub { ->json_schema_is('HardwareProduct') ->json_cmp_deeply(superhashof({ specification => { + $base_specification->%*, disk_size => { _default => 64, SEAGATE_8000 => 1, 'tilde~1~device' => 2 }, - other_prop => [ 1, 2, 3 ], + disks => { usb_hdd_num => 0 }, + }, + })); + + $t->put_ok('/hardware_product/'.$new_hw_id.'/specification?path=/disks', + json => { usb_hdd_num => 0, sas_ssd_slots => '1,2,3' }) + ->status_is(204) + ->location_is('/hardware_product/'.$new_hw_id); + + $t->get_ok('/hardware_product/'.$new_hw_id) + ->status_is(200) + ->json_schema_is('HardwareProduct') + ->json_cmp_deeply(superhashof({ + specification => { + $base_specification->%*, + disk_size => { _default => 64, SEAGATE_8000 => 1, 'tilde~1~device' => 2 }, + disks => { usb_hdd_num => 0, sas_ssd_slots => '1,2,3' }, }, })); @@ -393,8 +427,9 @@ subtest 'manipulate hardware_product.specification' => sub { ->json_schema_is('HardwareProduct') ->json_cmp_deeply(superhashof({ specification => { + $base_specification->%*, disk_size => { _default => 64, SEAGATE_8000 => 1 }, - other_prop => [ 1, 2, 3 ], + disks => { usb_hdd_num => 0, sas_ssd_slots => '1,2,3' }, }, })); @@ -406,17 +441,25 @@ subtest 'manipulate hardware_product.specification' => sub { ->status_is(200) ->json_schema_is('HardwareProduct') ->json_cmp_deeply(superhashof({ - specification => { other_prop => [ 1, 2, 3 ] }, + specification => { + $base_specification->%{ grep $_ ne 'disk_size', keys $base_specification->%* }, + disks => { usb_hdd_num => 0, sas_ssd_slots => '1,2,3' }, + }, })); - $t->delete_ok('/hardware_product/'.$new_hw_id.'/specification?path=') + $t->delete_ok('/hardware_product/'.$new_hw_id.'/specification?path=/disks/usb_hdd_num') ->status_is(204) ->location_is('/hardware_product/'.$new_hw_id); $t->get_ok('/hardware_product/'.$new_hw_id) ->status_is(200) ->json_schema_is('HardwareProduct') - ->json_cmp_deeply(superhashof({ specification => undef })); + ->json_cmp_deeply(superhashof({ + specification => { + $base_specification->%{ grep $_ ne 'disk_size', keys $base_specification->%* }, + disks => { sas_ssd_slots => '1,2,3' }, + }, + })); }; subtest 'delete a hardware product' => sub {