[ARVADOS] created: 1.3.0-1019-g0ecff7295
Git user
git at public.curoverse.com
Thu Jun 6 21:01:51 UTC 2019
at 0ecff72955e194a1060e4b1dd847ef9900b0fbbb (commit)
commit 0ecff72955e194a1060e4b1dd847ef9900b0fbbb
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Thu Jun 6 16:04:54 2019 -0400
15311: Remove convert_serialized_symbols_to_strings hook
Add "rake legacy:symbols" to warn you if there are any serialized
columns that may have legacy symbols.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 339bc9e23..efef7b812 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -27,7 +27,6 @@ class ArvadosModel < ApplicationRecord
after_create :log_create
after_update :log_update
after_destroy :log_destroy
- after_find :convert_serialized_symbols_to_strings
before_validation :normalize_collection_uuids
before_validation :set_default_owner
validate :ensure_valid_uuids
@@ -602,41 +601,6 @@ class ArvadosModel < ApplicationRecord
false
end
- def self.has_symbols? x
- if x.is_a? Hash
- x.each do |k,v|
- return true if has_symbols?(k) or has_symbols?(v)
- end
- elsif x.is_a? Array
- x.each do |k|
- return true if has_symbols?(k)
- end
- elsif x.is_a? Symbol
- return true
- elsif x.is_a? String
- return true if x.start_with?(':') && !x.start_with?('::')
- end
- false
- end
-
- def self.recursive_stringify x
- if x.is_a? Hash
- Hash[x.collect do |k,v|
- [recursive_stringify(k), recursive_stringify(v)]
- end]
- elsif x.is_a? Array
- x.collect do |k|
- recursive_stringify k
- end
- elsif x.is_a? Symbol
- x.to_s
- elsif x.is_a? String and x.start_with?(':') and !x.start_with?('::')
- x[1..-1]
- else
- x
- end
- end
-
def self.where_serialized(colname, value, md5: false)
colsql = colname.to_s
if md5
@@ -677,23 +641,6 @@ class ArvadosModel < ApplicationRecord
self.class.serialized_attributes
end
- def convert_serialized_symbols_to_strings
- # ensure_serialized_attribute_type should prevent symbols from
- # getting into the database in the first place. If someone managed
- # to get them into the database (perhaps using an older version)
- # we'll convert symbols to strings when loading from the
- # database. (Otherwise, loading and saving an object with existing
- # symbols in a serialized field will crash.)
- jsonb_cols = self.class.columns.select{|c| c.type == :jsonb}.collect{|j| j.name}
- (jsonb_cols + self.class.serialized_attributes.keys).uniq.each do |colname|
- if self.class.has_symbols? attributes[colname]
- attributes[colname] = self.class.recursive_stringify attributes[colname]
- send(colname + '=',
- self.class.recursive_stringify(attributes[colname]))
- end
- end
- end
-
def foreign_key_attributes
attributes.keys.select { |a| a.match(/_uuid$/) }
end
diff --git a/services/api/lib/tasks/symbols.rake b/services/api/lib/tasks/symbols.rake
new file mode 100644
index 000000000..5b4eab47e
--- /dev/null
+++ b/services/api/lib/tasks/symbols.rake
@@ -0,0 +1,48 @@
+def has_symbols? x
+ if x.is_a? Hash
+ x.each do |k,v|
+ return true if has_symbols?(k) or has_symbols?(v)
+ end
+ elsif x.is_a? Array
+ x.each do |k|
+ return true if has_symbols?(k)
+ end
+ elsif x.is_a? Symbol
+ return true
+ elsif x.is_a? String
+ return true if x.start_with?(':') && !x.start_with?('::')
+ end
+ false
+end
+
+def check_for_serialized_symbols rec
+ # ensure_serialized_attribute_type should prevent symbols from
+ # getting into the database in the first place. If someone managed
+ # to get them into the database (perhaps using an older version)
+ # we'll convert symbols to strings when loading from the
+ # database. (Otherwise, loading and saving an object with existing
+ # symbols in a serialized field will crash.)
+ jsonb_cols = rec.class.columns.select{|c| c.type == :jsonb}.collect{|j| j.name}
+ (jsonb_cols + rec.class.serialized_attributes.keys).uniq.each do |colname|
+ if has_symbols? rec.attributes[colname]
+ puts "Found string with leading ':' in attribute '#{colname}' of #{rec.uuid}:\n#{rec.attributes[colname].to_s[0..1024]}\n\n"
+ end
+ end
+end
+
+namespace :legacy do
+ desc 'Warn about serialized values starting with ":" that may be symbols'
+ task symbols: :environment do
+ [ApiClientAuthorization, ApiClient,
+ AuthorizedKey, Collection,
+ Container, ContainerRequest, Group,
+ Human, Job, JobTask, KeepDisk, KeepService, Link,
+ Node, PipelineInstance, PipelineTemplate,
+ Repository, Specimen, Trait, User, VirtualMachine,
+ Workflow].each do |klass|
+ klass.all.each do |c|
+ check_for_serialized_symbols c
+ end
+ end
+ end
+end
diff --git a/services/api/test/integration/container_request_test.rb b/services/api/test/integration/container_request_test.rb
index 170af6def..26cc081a6 100644
--- a/services/api/test/integration/container_request_test.rb
+++ b/services/api/test/integration/container_request_test.rb
@@ -4,9 +4,11 @@
require 'test_helper'
-class ContainerRequestTest < ActionDispatch::IntegrationTest
+class ContainerRequestIntegrationTest < ActionDispatch::IntegrationTest
test "test colon in input" do
+ # Tests for bug #15311 where strings with leading colons get
+ # corrupted when the leading ":" is stripped.
val = {"itemSeparator" => ":"}
post "/arvados/v1/container_requests",
params: {
diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index 0fcdad704..7d9da1e56 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -82,17 +82,6 @@ class ArvadosModelTest < ActiveSupport::TestCase
end
end
- test "Stringify symbols coming from serialized attribute in database" do
- set_user_from_auth :admin_trustedclient
- fixed = Link.find_by_uuid(links(:has_symbol_keys_in_database_somehow).uuid)
- assert_equal(["baz", "foo"], fixed.properties.keys.sort,
- "Hash symbol keys from DB did not get stringified.")
- assert_equal(['waz', 'waz', 'waz', 1, nil, false, true],
- fixed.properties['baz'],
- "Array symbol values from DB did not get stringified.")
- assert_equal true, fixed.save, "Failed to save fixed model back to db."
- end
-
test "No HashWithIndifferentAccess in database" do
set_user_from_auth :admin_trustedclient
link = Link.create!(link_class: 'test',
commit 52e4567ac230e552cc69ccd17acfaa1673f29dc3
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Thu Jun 6 11:36:34 2019 -0400
15311: Add test case demonstrating bug
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/test/integration/container_request_test.rb b/services/api/test/integration/container_request_test.rb
new file mode 100644
index 000000000..170af6def
--- /dev/null
+++ b/services/api/test/integration/container_request_test.rb
@@ -0,0 +1,37 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'test_helper'
+
+class ContainerRequestTest < ActionDispatch::IntegrationTest
+
+ test "test colon in input" do
+ val = {"itemSeparator" => ":"}
+ post "/arvados/v1/container_requests",
+ params: {
+ :container_request => {
+ :name => "workflow",
+ :state => "Committed",
+ :command => ["echo"],
+ :container_image => "fa3c1a9cb6783f85f2ecda037e07b8c3+167",
+ :output_path => "/",
+ :priority => 1,
+ :runtime_constraints => {"vcpus" => 1, "ram" => 1},
+ :mounts => {
+ :foo => {
+ :kind => "json",
+ :content => JSON.parse(SafeJSON.dump(val)),
+ }
+ }
+ }
+ }.to_json,
+ headers: {
+ 'HTTP_AUTHORIZATION' => "OAuth2 #{api_client_authorizations(:active).api_token}",
+ 'CONTENT_TYPE' => 'application/json'
+ }
+ assert_response :success
+ assert_equal "arvados#containerRequest", json_response['kind']
+ assert_equal val, json_response['mounts']['foo']['content']
+ end
+end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list