16263: Assigns nil to select attributes when receiving empty values.

Controller may translate NULL values to "" on certain object string fields.
The same with integers, NULLs are converted to 0.
When controller retrieves objects from railsAPI and uses the data to create
or update objects, some of those fields should get converted back to NULL.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas@di-pentima.com.ar>

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 369043e780863a4f84c4c9667fda9bdff2597afb..fbf177b01f4cf42be2678808fb28327d845b46e8 100644 (file)
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -45,6 +45,7 @@ class ApplicationController < ActionController::Base
   before_action :load_required_parameters
   before_action(:find_object_by_uuid,
                 except: [:index, :create] + ERROR_ACTIONS)
+  before_action(:set_nullable_attrs_to_null, only: [:update, :create])
   before_action :load_limit_offset_order_params, only: [:index, :contents]
   before_action :load_where_param, only: [:index, :contents]
   before_action :load_filters_param, only: [:index, :contents]
@@ -478,6 +479,21 @@ class ApplicationController < ActionController::Base
     @object = @objects.first
   end
 
+  def nullable_attributes
+    []
+  end
+
+  # Go code may send empty values (ie: empty string instead of NULL) that
+  # should be translated to NULL on the database.
+  def set_nullable_attrs_to_null
+    (resource_attrs.keys & nullable_attributes).each do |attr|
+      val = resource_attrs[attr]
+      if (val.class == Integer && val == 0) || (val.class == String && val == "")
+        resource_attrs[attr] = nil
+      end
+    end
+  end
+
   def reload_object_before_update
     # This is necessary to prevent an ActiveRecord::ReadOnlyRecord
     # error when updating an object which was retrieved using a join.

diff --git a/services/api/app/controllers/arvados/v1/users_controller.rb b/services/api/app/controllers/arvados/v1/users_controller.rb
index 1cf3b9d78a6e9d6290e9df1886304ba7124638c5..fecc0620c7c984932f5fefbd5c5468187212b51d 100644 (file)
--- a/services/api/app/controllers/arvados/v1/users_controller.rb
+++ b/services/api/app/controllers/arvados/v1/users_controller.rb
@@ -268,4 +268,8 @@ class Arvados::V1::UsersController < ApplicationController
     end
     super
   end
+
+  def nullable_attributes
+    super + [:email, :first_name, :last_name, :username]
+  end
 end

diff --git a/services/api/test/functional/arvados/v1/users_controller_test.rb b/services/api/test/functional/arvados/v1/users_controller_test.rb
index 753e707b629da2fa59536b9336fcfc404fbb7831..b38f0d52fb44d72e100d02b0d610909fa1ae2f70 100644 (file)
--- a/services/api/test/functional/arvados/v1/users_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/users_controller_test.rb
@@ -88,6 +88,38 @@ class Arvados::V1::UsersControllerTest < ActionController::TestCase
     assert_nil created['identity_url'], 'expected no identity_url'
   end
 
+  test "create new user with empty username" do
+    authorize_with :admin
+    post :create, params: {
+      user: {
+        first_name: "test_first_name",
+        last_name: "test_last_name",
+        username: ""
+      }
+    }
+    assert_response :success
+    created = JSON.parse(@response.body)
+    assert_equal 'test_first_name', created['first_name']
+    assert_not_nil created['uuid'], 'expected uuid for the newly created user'
+    assert_nil created['email'], 'expected no email'
+    assert_nil created['username'], 'expected no username'
+  end
+
+  test "update user with empty username" do
+    authorize_with :admin
+    user = users('spectator')
+    assert_not_nil user['username']
+    put :update, params: {
+      id: users('spectator')['uuid'],
+      user: {
+        username: ""
+      }
+    }
+    assert_response :success
+    updated = JSON.parse(@response.body)
+    assert_nil updated['username'], 'expected no username'
+  end
+
   test "create user with user, vm and repo as input" do
     authorize_with :admin
     repo_name = 'usertestrepo'