Bug #4651

[API] Do not evaluate "false" value as true for a parameter whose type is advertised as boolean.

Added by Tom Clegg about 6 years ago. Updated about 6 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Target version:
Start date:
11/22/2014
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Description

Example: arv create job should not invoke find_or_create behavior.

Currently, it sends "find_or_create"=>"false" because that's the default advertised in the discovery document. In the controller, we test params[:find_or_create], which is "false", which is truthy, and we do the wrong thing.


Subtasks

Task #4654: Review 4651-boolean-paramsResolvedTom Clegg

Task #4653: Add testsResolvedTom Clegg

Task #4652: FixResolvedTom Clegg

Associated revisions

Revision 257ecfec
Added by Tom Clegg about 6 years ago

Merge branch '4651-boolean-params' closes #4651

History

#1 Updated by Tom Clegg about 6 years ago

  • Description updated (diff)
  • Category set to API
  • Status changed from New to In Progress
  • Assigned To set to Tom Clegg
4651-boolean-params @ ac4cdfc
  • Fixes the specific bug: params[:find_or_create]==false if supplied by the client / router as "false"
  • Does this for all params advertised via _*_requires_parameters
  • Moves the information about find_or_create's existence out of Arvados::V1::SchemaController#index into ApplicationController._create_requires_parameters so it can be introspected
  • Adds tests

#2 Updated by Brett Smith about 6 years ago

At ac4cdfc2, I'm getting a number of API server test failures:

[187/306] Arvados::V1::KeepDisksControllerTest#test_add_keep_disk_with_admin_token = 0.04 s
  1) Failure:
Arvados::V1::KeepDisksControllerTest#test_add_keep_disk_with_admin_token [/home/brett/repos/arvados/services/api/test/functional/arvados/v1/keep_disks_controller_test.rb:11]:
Expected response to be a <:success>, but was <422>

[188/306] Arvados::V1::KeepDisksControllerTest#test_add_keep_disk_with_no_filesystem_uuid_{:ping_secret=>"",_:filesystem_uuid=>""} =
  2) Failure:
Arvados::V1::KeepDisksControllerTest#test_add_keep_disk_with_no_filesystem_uuid_{:ping_secret=>"",_:filesystem_uuid=>""} [/home/brett/repos/arvados/services/api/test/functional/arvados/v1/keep_disks_controller_test.rb:26]:
Expected response to be a <:success>, but was <422>

[189/306] Arvados::V1::KeepDisksControllerTest#test_add_keep_disk_with_no_filesystem_uuid_{:ping_secret=>""} = 0.04 s
  3) Failure:
Arvados::V1::KeepDisksControllerTest#test_add_keep_disk_with_no_filesystem_uuid_{:ping_secret=>""} [/home/brett/repos/arvados/services/api/test/functional/arvados/v1/keep_disks_controller_test.rb:26]:
Expected response to be a <:success>, but was <422>

[192/306] Arvados::V1::KeepDisksControllerTest#test_ping_keep_disk = 0.01 s
  4) Failure:
Arvados::V1::KeepDisksControllerTest#test_ping_keep_disk [/home/brett/repos/arvados/services/api/test/functional/arvados/v1/keep_disks_controller_test.rb:44]:
Expected response to be a <:success>, but was <422>

[193/306] Arvados::V1::KeepDisksControllerTest#test_refuse_to_add_keep_disk_without_admin_token = 0.00 s
  5) Failure:
Arvados::V1::KeepDisksControllerTest#test_refuse_to_add_keep_disk_without_admin_token [/home/brett/repos/arvados/services/api/test/functional/arvados/v1/keep_disks_controller_test.rb:35]:
Expected response to be a <404>, but was <422>

[243/306] Arvados::V1::NodesControllerTest#test_node_should_fail_ping_with_invalid_ping_secret = 0.00 s
  7) Failure:
Arvados::V1::NodesControllerTest#test_node_should_fail_ping_with_invalid_ping_secret [/home/brett/repos/arvados/services/api/test/functional/arvados/v1/nodes_controller_test.rb:66]:
Expected response to be a <401>, but was <422>

[244/306] Arvados::V1::NodesControllerTest#test_node_should_ping_with_ping_secret_and_no_token = 0.01 s
  8) Failure:
Arvados::V1::NodesControllerTest#test_node_should_ping_with_ping_secret_and_no_token [/home/brett/repos/arvados/services/api/test/functional/arvados/v1/nodes_controller_test.rb:50]:
Expected response to be a <:success>, but was <422>

[247/306] Arvados::V1::NodesControllerTest#test_ping_adds_node_stats_to_info = 0.02 s
  9) Failure:
Arvados::V1::NodesControllerTest#test_ping_adds_node_stats_to_info [/home/brett/repos/arvados/services/api/test/functional/arvados/v1/nodes_controller_test.rb:88]:
Expected response to be a <:success>, but was <422>

The traceback consistently looks like this:

#<NoMethodError: undefined method `[]' for false:FalseClass>
/home/brett/repos/arvados/services/api/app/controllers/application_controller.rb:453:in `block in load_required_parameters'
/home/brett/repos/arvados/services/api/app/controllers/application_controller.rb:452:in `each'
/home/brett/repos/arvados/services/api/app/controllers/application_controller.rb:452:in `load_required_parameters'
/home/brett/.rvm/gems/ruby-2.1.1/gems/activesupport-3.2.17/lib/active_support/callbacks.rb:484:in `_run__2342243818766961439__process_action__1050683344284048655__callbacks'
…

#3 Updated by Tom Clegg about 6 years ago

My bad, apparently I forgot to run all tests. Fixed @ f3d43ab

#4 Updated by Tom Clegg about 6 years ago

Aaaaand fixed again at 3334021, now so fixed that even SDKs and Workbench pass! Sorry.

#5 Updated by Brett Smith about 6 years ago

3334021 looks great, thanks.

#6 Updated by Anonymous about 6 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 67 to 100

Applied in changeset arvados|commit:257ecfece0f6941011c85e735459d86b9f850d25.

Also available in: Atom PDF