From 26a54c66f2068f9b59e803033f653377c707c0f8 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Fri, 3 Dec 2021 19:15:13 +0200 Subject: [PATCH] fix(schemas) add transformations support to subschemas --- kong/db/schema/init.lua | 113 ++++---- kong/db/schema/metaschema.lua | 148 +++++----- .../01-db/01-schema/01-schema_spec.lua | 133 +++++---- .../01-db/01-schema/02-metaschema_spec.lua | 261 ++++++++++++++++++ 4 files changed, 470 insertions(+), 185 deletions(-) diff --git a/kong/db/schema/init.lua b/kong/db/schema/init.lua index a4de61c2c85a..832f54eb5573 100644 --- a/kong/db/schema/init.lua +++ b/kong/db/schema/init.lua @@ -1335,53 +1335,56 @@ do end -local function run_transformation_checks(self, input, original_input, rbw_entity, errors) - if not self.transformations then - return - end - - for _, transformation in ipairs(self.transformations) do - local args = {} - local argc = 0 - local none_set = true - for _, input_field_name in ipairs(transformation.input) do - if is_nonempty(get_field(original_input or input, input_field_name)) then - none_set = false - end - - argc = argc + 1 - args[argc] = input_field_name - end - - local needs_changed = false - if transformation.needs then - for _, input_field_name in ipairs(transformation.needs) do - if rbw_entity and not needs_changed then - local value = get_field(original_input or input, input_field_name) - local rbw_value = get_field(rbw_entity, input_field_name) - if value ~= rbw_value then - needs_changed = true - end +local function run_transformation_checks(schema_or_subschema, input, original_input, rbw_entity, errors) + if schema_or_subschema.transformations then + for _, transformation in ipairs(schema_or_subschema.transformations) do + local args = {} + local argc = 0 + local none_set = true + for _, input_field_name in ipairs(transformation.input) do + if is_nonempty(get_field(original_input or input, input_field_name)) then + none_set = false end argc = argc + 1 args[argc] = input_field_name end - end - if needs_changed or (not none_set) then - local ok, err = mutually_required(needs_changed and original_input or input, args) - if not ok then - insert_entity_error(errors, validation_errors.MUTUALLY_REQUIRED:format(err)) + local needs_changed = false + if transformation.needs then + for _, input_field_name in ipairs(transformation.needs) do + if rbw_entity and not needs_changed then + local value = get_field(original_input or input, input_field_name) + local rbw_value = get_field(rbw_entity, input_field_name) + if value ~= rbw_value then + needs_changed = true + end + end - else - ok, err = mutually_required(original_input or input, transformation.input) + argc = argc + 1 + args[argc] = input_field_name + end + end + + if needs_changed or (not none_set) then + local ok, err = mutually_required(needs_changed and original_input or input, args) if not ok then insert_entity_error(errors, validation_errors.MUTUALLY_REQUIRED:format(err)) + + else + ok, err = mutually_required(original_input or input, transformation.input) + if not ok then + insert_entity_error(errors, validation_errors.MUTUALLY_REQUIRED:format(err)) + end end end end end + + local subschema = get_subschema(schema_or_subschema, input) + if subschema then + run_transformation_checks(subschema, input, original_input, rbw_entity, errors) + end end @@ -2113,19 +2116,10 @@ local function get_transform_args(input, original_input, output, transformation) return args end ---- Run transformations on fields. --- @param input The input table. --- @param original_input The original input for transformation detection. --- @param context a string describing the CRUD context: --- valid values are: "insert", "update", "upsert", "select" --- @return the transformed entity -function Schema:transform(input, original_input, context) - if not self.transformations then - return input - end - local output = nil - for _, transformation in ipairs(self.transformations) do +local function run_transformations(self, transformations, input, original_input, context) + local output + for _, transformation in ipairs(transformations) do local transform if context == "select" then transform = transformation.on_read @@ -2136,7 +2130,6 @@ function Schema:transform(input, original_input, context) if transform then local args = get_transform_args(input, original_input, output, transformation) - if args then local data, err = transform(unpack(args)) if err then @@ -2153,6 +2146,32 @@ function Schema:transform(input, original_input, context) end +--- Run transformations on fields. +-- @param input The input table. +-- @param original_input The original input for transformation detection. +-- @param context a string describing the CRUD context: +-- valid values are: "insert", "update", "upsert", "select" +-- @return the transformed entity +function Schema:transform(input, original_input, context) + local output, err + if self.transformations then + output, err = run_transformations(self, self.transformations, input, original_input, context) + if not output then + return nil, err + end + end + + local subschema = get_subschema(self, input) + if subschema and subschema.transformations then + output, err = run_transformations(subschema, subschema.transformations, output or input, original_input, context) + if not output then + return nil, err + end + end + + return output or input +end + --- Instatiate a new schema from a definition. -- @param definition A table with attributes describing -- fields and other information about a schema. diff --git a/kong/db/schema/metaschema.lua b/kong/db/schema/metaschema.lua index 6ac43d1b8470..798449f3bcff 100644 --- a/kong/db/schema/metaschema.lua +++ b/kong/db/schema/metaschema.lua @@ -360,7 +360,66 @@ local nested_attributes = { local check_field +local function has_schema_field(schema, name) + if schema == nil then + return false + end + + local dot = string.find(name, ".", 1, true) + if not dot then + for _, field in ipairs(schema.fields) do + local k = next(field) + if k == name then + return true + end + end + + return false + end + + local hd, tl = string.sub(name, 1, dot - 1), string.sub(name, dot + 1) + for _, field in ipairs(schema.fields) do + local k = next(field) + if k == hd then + if field[hd] and field[hd].type == "foreign" then + -- metaschema has no access to foreign schemas + -- so we just trust the developer of the schema. + + return true + end + + return has_schema_field(field[hd], tl) + end + end + + return false +end + local check_fields = function(schema, errors) + if schema.transformations then + for i, transformation in ipairs(schema.transformations) do + for j, input in ipairs(transformation.input) do + if not has_schema_field(schema, input) then + errors.transformations = errors.transformations or {} + errors.transformations.input = errors.transformations.input or {} + errors.transformations.input[i] = errors.transformations.input[i] or {} + errors.transformations.input[i][j] = string.format("invalid field name: %s", input) + end + end + + if transformation.needs then + for j, need in ipairs(transformation.needs) do + if not has_schema_field(schema, need) then + errors.transformations = errors.transformations or {} + errors.transformations.needs = errors.transformations.needs or {} + errors.transformations.needs[i] = errors.transformations.needs[i] or {} + errors.transformations.needs[i][j] = string.format("invalid field name: %s", need) + end + end + end + end + end + for _, item in ipairs(schema.fields) do if type(item) ~= "table" then errors["fields"] = meta_errors.FIELDS_ARRAY @@ -420,42 +479,6 @@ check_field = function(k, field, errors) end -local function has_schema_field(schema, name) - if schema == nil then - return false - end - - local dot = string.find(name, ".", 1, true) - if not dot then - for _, field in ipairs(schema.fields) do - local k = next(field) - if k == name then - return true - end - end - - return false - end - - local hd, tl = string.sub(name, 1, dot - 1), string.sub(name, dot + 1) - for _, field in ipairs(schema.fields) do - local k = next(field) - if k == hd then - if field[hd] and field[hd].type == "foreign" then - -- metaschema has no access to foreign schemas - -- so we just trust the developer of the schema. - - return true - end - - return has_schema_field(field[hd], tl) - end - end - - return false -end - - -- Build a variant of the field_schema, adding a 'func' attribute -- and restricting the set of valid types. local function make_shorthand_field_schema() @@ -595,6 +618,9 @@ local MetaSchema = Schema.new({ { shorthand_fields = shorthand_fields_array, }, + { + transformations = transformations_array, + }, { check = { type = "function", @@ -607,9 +633,6 @@ local MetaSchema = Schema.new({ nilable = true }, }, - { - transformations = transformations_array, - }, }, entity_checks = { @@ -689,50 +712,6 @@ local MetaSchema = Schema.new({ end end - if schema.transformations then - for i, transformation in ipairs(schema.transformations) do - for j, input in ipairs(transformation.input) do - if not has_schema_field(schema, input) then - if not errors.transformations then - errors.transformations = {} - end - - if not errors.transformations.input then - errors.transformations.input = {} - end - - - if not errors.transformations.input[i] then - errors.transformations.input[i] = {} - end - - errors.transformations.input[i][j] = string.format("invalid field name: %s", input) - end - end - - if transformation.needs then - for j, need in ipairs(transformation.needs) do - if not has_schema_field(schema, need) then - if not errors.transformations then - errors.transformations = {} - end - - if not errors.transformations.needs then - errors.transformations.needs = {} - end - - - if not errors.transformations.needs[i] then - errors.transformations.needs[i] = {} - end - - errors.transformations.needs[i][j] = string.format("invalid field name: %s", need) - end - end - end - end - end - return check_fields(schema, errors) end, @@ -778,6 +757,9 @@ MetaSchema.MetaSubSchema = Schema.new({ { shorthands = shorthands_array, }, + { + transformations = transformations_array, + }, { check = { type = "function", diff --git a/spec/01-unit/01-db/01-schema/01-schema_spec.lua b/spec/01-unit/01-db/01-schema/01-schema_spec.lua index 4b9aa197c5bf..62ce552d3612 100644 --- a/spec/01-unit/01-db/01-schema/01-schema_spec.lua +++ b/spec/01-unit/01-db/01-schema/01-schema_spec.lua @@ -17,6 +17,21 @@ if luacov_ok then end +local SchemaKind = { + { name = "schema", new = Schema.new, }, + { name = "subschema", new = function(definition) + local schema = assert(Schema.new({ + name = "test", + subschema_key = "name", + fields = definition.fields, + })) + assert(schema:new_subschema("subtest", definition)) + return assert(schema.subschemas["subtest"]) + end + } +} + + describe("schema", function() local uuid_pattern = "^" .. ("%x"):rep(8) .. "%-" .. ("%x"):rep(4) .. "%-" .. ("%x"):rep(4) .. "%-" .. ("%x"):rep(4) .. "%-" @@ -1864,53 +1879,57 @@ describe("schema", function() assert.falsy(err) end) - - it("test mutually required checks specified by transformations", function() - local Test = Schema.new({ - fields = { - { a1 = { type = "string" } }, - { a2 = { type = "string" } }, - { a3 = { type = "string" } }, - }, - transformations = { - { - input = { "a2" }, - on_write = function() return {} end - }, - { - input = { "a1", "a3" }, - on_write = function() return {} end + for i = 1, 2 do + it("test mutually required checks specified by transformations (" .. SchemaKind[i].name .. ")", function() + local Test = SchemaKind[i].new({ + fields = { + { name = { type = "string", required = true, } }, + { a1 = { type = "string" } }, + { a2 = { type = "string" } }, + { a3 = { type = "string" } }, }, - } - }) - - local ok, err = Test:validate_update({ - a1 = "foo" - }) - assert.is_falsy(ok) - assert.match("all or none of these fields must be set: 'a1', 'a3'", err["@entity"][1]) + transformations = { + { + input = { "a2" }, + on_write = function() return {} end + }, + { + input = { "a1", "a3" }, + on_write = function() return {} end + }, + } + }) - ok, err = Test:validate_update({ - a2 = "foo" - }) - assert.truthy(ok) - assert.falsy(err) + local ok, err = Test:validate_update({ + name = "test", + a1 = "foo" + }) + assert.is_falsy(ok) + assert.match("all or none of these fields must be set: 'a1', 'a3'", err["@entity"][1]) - ok, err = Test:validate_update({ - a1 = "aaa", - a2 = "bbb", - a3 = "ccc", - a4 = "ddd", - }, { - a1 = "foo" - }) + ok, err = Test:validate_update({ + a2 = "foo" + }) + assert.truthy(ok) + assert.falsy(err) + + ok, err = Test:validate_update({ + a1 = "aaa", + a2 = "bbb", + a3 = "ccc", + a4 = "ddd", + }, { + a1 = "foo" + }) - assert.is_falsy(ok) - assert.match("all or none of these fields must be set: 'a1', 'a3'", err["@entity"][1]) + assert.is_falsy(ok) + assert.match("all or none of these fields must be set: 'a1', 'a3'", err["@entity"][1]) end) + end - it("test mutually required checks specified by transformations with needs", function() - local Test = Schema.new({ + for i = 1, 2 do + it("test mutually required checks specified by transformations with needs (" .. SchemaKind[i].name .. ")", function() + local Test = SchemaKind[i].new({ fields = { { a1 = { type = "string" } }, { a2 = { type = "string" } }, @@ -1956,9 +1975,10 @@ describe("schema", function() assert.truthy(ok) assert.falsy(err) end) + end - - it("test mutually required checks specified by transformations with needs (combinations)", function() + for i = 1, 2 do + it("test mutually required checks specified by transformations with needs (combinations) (" .. SchemaKind[i].name .. ")", function() -- { -- input = I1, I2 -- needs = N1, N2 @@ -1996,7 +2016,7 @@ describe("schema", function() -- 28. N1 ok, no changes in needs, would not invalidate I1 I2 -- 29. N2 ok, no changes in needs, would not invalidate I1 I2 - local Test = Schema.new({ + local Test = SchemaKind[i].new({ fields = { { i1 = { type = "string" } }, { i2 = { type = "string" } }, @@ -2443,6 +2463,7 @@ describe("schema", function() assert.truthy(ok) assert.falsy(err) end) + end it("test mutually exclusive checks", function() local Test = Schema.new({ @@ -3899,7 +3920,8 @@ describe("schema", function() end) end) - describe("transform", function() + for i = 1, 2 do + describe("transform (" .. SchemaKind[i].name .. ")", function() it("transforms fields", function() local test_schema = { name = "test", @@ -3921,7 +3943,7 @@ describe("schema", function() } local entity = { name = "test1" } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, _ = TestEntities:transform(entity) assert.truthy(transformed_entity) @@ -3952,7 +3974,7 @@ describe("schema", function() } local entity = { name = "TeSt1" } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, _ = TestEntities:transform(entity) assert.truthy(transformed_entity) @@ -3986,7 +4008,7 @@ describe("schema", function() local entity = { name = "test1" } local input = { name = "we have a value" } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, _ = TestEntities:transform(entity, input) assert.truthy(transformed_entity) @@ -4014,7 +4036,7 @@ describe("schema", function() } local entity = { name = "test1" } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, _ = TestEntities:transform(entity) assert.truthy(transformed_entity) @@ -4043,7 +4065,7 @@ describe("schema", function() local entity = { name = "test1" } local input = { name = nil } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, _ = TestEntities:transform(entity, input) assert.truthy(transformed_entity) @@ -4079,7 +4101,7 @@ describe("schema", function() local entity = { name = "Bob" } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, _ = TestEntities:transform(entity) assert.truthy(transformed_entity) @@ -4111,7 +4133,7 @@ describe("schema", function() local entity = { name = "Bob" } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, _ = TestEntities:transform(entity) assert.truthy(transformed_entity) @@ -4140,7 +4162,7 @@ describe("schema", function() } local entity = { name = "test1" } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, err = TestEntities:transform(entity) assert.falsy(transformed_entity) @@ -4172,7 +4194,7 @@ describe("schema", function() } local entity = { name = "test1" } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, _ = TestEntities:transform(entity) assert.truthy(transformed_entity) @@ -4205,11 +4227,12 @@ describe("schema", function() } local entity = { name = "John", age = 13 } - local TestEntities = Schema.new(test_schema) + local TestEntities = SchemaKind[i].new(test_schema) local transformed_entity, _ = TestEntities:transform(entity) assert.truthy(transformed_entity) assert.equal("John 13", transformed_entity.name) end) end) + end end) diff --git a/spec/01-unit/01-db/01-schema/02-metaschema_spec.lua b/spec/01-unit/01-db/01-schema/02-metaschema_spec.lua index 584752cd7b89..57aa6ecf89cc 100644 --- a/spec/01-unit/01-db/01-schema/02-metaschema_spec.lua +++ b/spec/01-unit/01-db/01-schema/02-metaschema_spec.lua @@ -1093,4 +1093,265 @@ describe("metasubschema", function() assert.truthy(MetaSchema.MetaSubSchema:validate(schema)) end) end + + it("validates transformation has transformation function specified (positive)", function() + assert.truthy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { test = { type = "string" } }, + }, + transformations = { + { + input = { "test" }, + on_write = function() return true end, + }, + }, + })) + + assert.truthy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { test = { type = "string" } }, + }, + transformations = { + { + input = { "test" }, + on_read = function() return true end, + }, + }, + })) + + assert.truthy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { test = { type = "string" } }, + }, + transformations = { + { + input = { "test" }, + on_read = function() return true end, + on_write = function() return true end, + }, + }, + })) + end) + + it("validates transformation has transformation function specified (negative)", function() + assert.falsy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { test = { type = "string" } }, + }, + transformations = { + { + input = { "test" }, + }, + }, + })) + end) + + it("validates transformation input fields exists (positive)", function() + assert.truthy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { test = { type = "string" } }, + }, + transformations = { + { + input = { "test" }, + on_write = function() return true end, + }, + }, + })) + end) + + it("validates transformation input fields exists (negative)", function() + assert.falsy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { test = { type = "string" } }, + }, + transformations = { + { + input = { "nonexisting" }, + on_write = function() return true end, + }, + }, + })) + end) + + it("validates nested transformation input fields exists (positive)", function() + assert.truthy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { + test = { + type = "record", + fields = { + { + field = { type = "string" } + }, + } + } + }, + }, + transformations = { + { + input = { "test.field" }, + on_write = function() return true end, + }, + }, + })) + end) + + it("validates nested transformation input fields exists (negative)", function() + assert.falsy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { + test = { + type = "record", + fields = { + { + field = { type = "string" }, + }, + }, + }, + }, + }, + transformations = { + { + input = { "test.nonexisting" }, + on_write = function() return true end, + }, + }, + })) + + assert.falsy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { + test = { + type = "record", + fields = { + { + field = { type = "string" }, + }, + }, + }, + }, + }, + transformations = { + { + input = { "nonexisting.field" }, + on_write = function() return true end, + }, + }, + })) + end) + + it("validates transformation needs fields exists (positive)", function() + assert.truthy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { test = { type = "string" } }, + }, + transformations = { + { + input = { "test" }, + needs = { "test" }, + on_write = function() return true end, + }, + }, + })) + end) + + it("validates transformation needs fields exists (negative)", function() + assert.falsy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { test = { type = "string" } }, + }, + transformations = { + { + input = { "test" }, + needs = { "nonexisting" }, + on_write = function() return true end, + }, + }, + })) + end) + + it("validates nested transformation needs fields exists (positive)", function() + assert.truthy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { + test = { + type = "record", + fields = { + { + field = { type = "string" } + }, + } + } + }, + }, + transformations = { + { + input = { "test.field" }, + needs = { "test.field" }, + on_write = function() return true end, + }, + }, + })) + end) + + it("validates nested transformation needs fields exists (negative)", function() + assert.falsy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { + test = { + type = "record", + fields = { + { + field = { type = "string" }, + }, + }, + }, + }, + }, + transformations = { + { + input = { "test.field" }, + needs = { "test.nonexisting" }, + on_write = function() return true end, + }, + }, + })) + + assert.falsy(MetaSchema.MetaSubSchema:validate({ + name = "test", + fields = { + { + test = { + type = "record", + fields = { + { + field = { type = "string" }, + }, + }, + }, + }, + }, + transformations = { + { + input = { "test.field" }, + needs = { "nonexisting.field" }, + on_write = function() return true end, + }, + }, + })) + end) end)