[ARVADOS] updated: 050277d9f11f275fb8c2750e5e267e40da36d76a
git at public.curoverse.com
git at public.curoverse.com
Mon Dec 8 16:47:58 EST 2014
Summary of changes:
.../controllers/arvados/v1/collections_controller.rb | 2 +-
services/api/app/models/arvados_model.rb | 4 ++++
services/api/app/models/job.rb | 6 +++---
.../test/functional/arvados/v1/links_controller_test.rb | 17 ++++++++++-------
.../test/functional/arvados/v1/logs_controller_test.rb | 4 ++--
5 files changed, 20 insertions(+), 13 deletions(-)
via 050277d9f11f275fb8c2750e5e267e40da36d76a (commit)
from 780b29bc2d8cc7cc990303a62c23e8a629170f67 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 050277d9f11f275fb8c2750e5e267e40da36d76a
Author: Tim Pierce <twp at curoverse.com>
Date: Mon Dec 8 16:44:17 2014 -0500
4269: clean up uuid regex matching
Code review feedback:
* Improved name for validation "no_collection_uuids" to
"ensure_no_collection_uuids_in_script_params"
* Added ArvadosModel.uuid_regex (along the lines of uuid_like_pattern)
and substituted it for hardcoded uuid regexes throughout the code.
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 7bc207f..54f0982 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -54,7 +54,7 @@ class Arvados::V1::CollectionsController < ApplicationController
when String
if m = /[a-f0-9]{32}\+\d+/.match(sp)
yield m[0], nil
- elsif m = /[0-9a-z]{5}-4zz18-[0-9a-z]{15}/.match(sp)
+ elsif m = Collection.uuid_regex.match(sp)
yield nil, m[0]
end
end
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 13ccd70..a170fb9 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -445,6 +445,10 @@ class ArvadosModel < ActiveRecord::Base
"_____-#{uuid_prefix}-_______________"
end
+ def self.uuid_regex
+ %r/[a-z0-9]{5}-#{uuid_prefix}-[a-z0-9]{15}/
+ end
+
def ensure_valid_uuids
specials = [system_user_uuid]
diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index bc64cf5..90f67d1 100644
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -15,7 +15,7 @@ class Job < ArvadosModel
validate :find_docker_image_locator
validate :validate_status
validate :validate_state_change
- validate :no_collection_uuids
+ validate :ensure_no_collection_uuids_in_script_params
before_save :update_timestamps_when_state_changes
has_many :commit_ancestors, :foreign_key => :descendant, :primary_key => :script_version
@@ -376,7 +376,7 @@ class Job < ArvadosModel
ok
end
- def no_collection_uuids
+ def ensure_no_collection_uuids_in_script_params
# recursive_hash_search searches recursively through hashes and
# arrays in 'thing' for string fields matching regular expression
# 'pattern'. Returns true if pattern is found, false otherwise.
@@ -398,7 +398,7 @@ class Job < ArvadosModel
# Fail validation if any script_parameters field includes a string containing a
# collection uuid pattern.
if self.script_parameters_changed?
- if recursive_hash_search(self.script_parameters, /[a-z0-9]{5}-4zz18-[a-z0-9]{15}/)
+ if recursive_hash_search(self.script_parameters, Collection.uuid_regex)
self.errors.add :script_parameters, "must use portable_data_hash instead of collection uuid"
return false
end
diff --git a/services/api/test/functional/arvados/v1/links_controller_test.rb b/services/api/test/functional/arvados/v1/links_controller_test.rb
index 7515e49..9bf1b0b 100644
--- a/services/api/test/functional/arvados/v1/links_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/links_controller_test.rb
@@ -137,7 +137,7 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
assert_response :success
found = assigns(:objects)
assert_not_equal 0, found.count
- assert_equal found.count, (found.select { |f| f.tail_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ assert_equal found.count, (found.select { |f| f.tail_uuid.match User.uuid_regex }).count
end
test "filter links with 'is_a' operator with more than one" do
@@ -148,7 +148,10 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
assert_response :success
found = assigns(:objects)
assert_not_equal 0, found.count
- assert_equal found.count, (found.select { |f| f.tail_uuid.match /[a-z0-9]{5}-(tpzed|j7d0g)-[a-z0-9]{15}/}).count
+ assert_equal found.count, (found.select { |f|
+ f.tail_uuid.match User.uuid_regex or
+ f.tail_uuid.match Group.uuid_regex
+ }).count
end
test "filter links with 'is_a' operator with bogus type" do
@@ -169,7 +172,7 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
assert_response :success
found = assigns(:objects)
assert_not_equal 0, found.count
- assert_equal found.count, (found.select { |f| f.head_uuid.match /.....-4zz18-.............../}).count
+ assert_equal found.count, (found.select { |f| f.head_uuid.match Collection.uuid_regex}).count
end
test "test can still use where tail_kind" do
@@ -180,7 +183,7 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
assert_response :success
found = assigns(:objects)
assert_not_equal 0, found.count
- assert_equal found.count, (found.select { |f| f.tail_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ assert_equal found.count, (found.select { |f| f.tail_uuid.match User.uuid_regex }).count
end
test "test can still use where head_kind" do
@@ -191,7 +194,7 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
assert_response :success
found = assigns(:objects)
assert_not_equal 0, found.count
- assert_equal found.count, (found.select { |f| f.head_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ assert_equal found.count, (found.select { |f| f.head_uuid.match User.uuid_regex }).count
end
test "test can still use filter tail_kind" do
@@ -202,7 +205,7 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
assert_response :success
found = assigns(:objects)
assert_not_equal 0, found.count
- assert_equal found.count, (found.select { |f| f.tail_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ assert_equal found.count, (found.select { |f| f.tail_uuid.match User.uuid_regex }).count
end
test "test can still use filter head_kind" do
@@ -213,7 +216,7 @@ class Arvados::V1::LinksControllerTest < ActionController::TestCase
assert_response :success
found = assigns(:objects)
assert_not_equal 0, found.count
- assert_equal found.count, (found.select { |f| f.head_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ assert_equal found.count, (found.select { |f| f.head_uuid.match User.uuid_regex }).count
end
test "head_kind matches head_uuid" do
diff --git a/services/api/test/functional/arvados/v1/logs_controller_test.rb b/services/api/test/functional/arvados/v1/logs_controller_test.rb
index f3826ca..475e7d6 100644
--- a/services/api/test/functional/arvados/v1/logs_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/logs_controller_test.rb
@@ -29,7 +29,7 @@ class Arvados::V1::LogsControllerTest < ActionController::TestCase
assert_response :success
found = assigns(:objects)
assert_not_equal 0, found.count
- assert_equal found.count, (found.select { |f| f.object_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ assert_equal found.count, (found.select { |f| f.object_uuid.match User.uuid_regex }).count
l = JSON.parse(@response.body)
assert_equal 'arvados#user', l['items'][0]['object_kind']
end
@@ -42,7 +42,7 @@ class Arvados::V1::LogsControllerTest < ActionController::TestCase
assert_response :success
found = assigns(:objects)
assert_not_equal 0, found.count
- assert_equal found.count, (found.select { |f| f.object_uuid.match /[a-z0-9]{5}-tpzed-[a-z0-9]{15}/}).count
+ assert_equal found.count, (found.select { |f| f.object_uuid.match User.uuid_regex }).count
end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list