Merge/patch additionalProperties

[koenfaro90]

Hi,

Trying to use ajv-merge-patch in a scenario where my definitions are split up over multiple JSON files - lets say I have the following 4 files;

/system.json (all objects have these properties - however not always all of them required)

{
	"id": "/system.json",
	"type": "object",
	"properties": {
		"id": {
			"type": "number"
		}
	}
}

/object/user.json (a specific object type - includes all from system.json)

{
	"id": "/object/user.json",
	"$merge": {
		"source": {
			"type": "object",
			"properties": {
				"firstname": {
					"type": ["string", "null"]
				}
			}
		},
		"with": {
			"$ref": "/system.json#"
		}
	}
}

/general/crud/create/request.json (template for incoming record create requests)

{
	"id": "/general/crud/create/request.json",
	"type": "object",
	"properties": {
		"userId": {
			"type": "number"
		},
		"values": {
			"type": "object"
		}
	},
	"required": ["userId", "values"],
	"additionalProperties": false
}

user/action/create/request.json (specific user create template - extends /general/crud/create/request.json above - making the values node specific with the user model above - adds one additional property)

{
	"id": "/user/action/create/request.json",
	"type": "object",
	"$patch": {
		"source": {
			"$ref": "/general/crud/create/request.json#"
		},
		"with": [{
			"op": "add",
			"path": "/properties/values",
			"value": {
				"$merge": {
					"source": {
						"$ref": "/object/user.json#"
					},
					"with": {
						"properties": {
							"unencrypted_password": {
								"type": "string"
							}
						},
						"required": ["unencrypted_password"]
					}
				}
			}
		}]
	}
}

Now what I would like to do is put an "additionalProperties: false" on the user model as other use cases besides create should not specify any additional properties - however if I do this currently my "unencrypted_password" will be rejected - injecting additionalProperties: true anywhere in the last schema does not seem to have any effect.

Am I just trying to do something stupid here, or is it just not possible to override additionalProperties when using patch/merge in its current state? Is this something on the roadmap to be done or something which would not even be considered if I wrote a PR for it?

[epoberezkin]

@koenfaro90 I am not sure if I understand the issue and what you want to achieve correctly... It would help if you try to simplify the example and provide a working code sample in runkit.com that generates some output and write here how this output is different from your expectations. Issue template in ajv repo provides a decent structure for this information. Thank you!

[epoberezkin]

Also I think it can be related to ajv-validator/ajv#468 - nested $merges/$patches may be confusing and not too intuitive.

[koenfaro90]

Thanks for your quick reply @epoberezkin - I have to run now, but made a quick runkit.com example which should have plenty of comments I think:

https://runkit.com/koenfaro90/ajv-merge-patch-11-additionalproperties

Let me know if you still need me to fill out the issue template later today!

[stoked10]

So I've come across what I feel is a similar problem. If I merge two schemas, one of which has an if-then-else in it, schema validation fails if either of them have an 'additionalProperties: false' in them. If I remove the additionalProperties, everything passes. Here's an example that fails, if you remove all of the additionalProperties it passes:

const Ajv = require('ajv');
const ajv = new Ajv({allErrors: true});

require('ajv-merge-patch')(ajv);

const defaultSchema = {
	"$id": "defaultSchema",
	"type": "object",
	"properties": {
		"item1": {
			"type": "string",
		},
	},
    "additionalProperties": false,
};

const testSchema = {
    "$id": "testSchema",
    "type": "object",
    "properties": {
        "item2": {
            "type": "string",
        },
    },
    "if": {
        "properties": {
            "item2": { "enum": ["def"] },
        },
    },
    "then": {
        "properties": {
            "item2": {
                "type": "string",
            },
            "item3": {
                "type": "string",
            },            
        },
        "additionalProperties": false,
    },
    "else": {
        "properties": {
            "item2": {
                "type": "string",
            },
            "item3": {
                "type": "object",
            },            
        },
        "additionalProperties": false,
    },
};

const mergedSchema = {
    "type": "object",
    "$merge": {
        "source": {
            "$ref": "defaultSchema#",
        },
        "with": {
            "$ref": "testSchema#",
        },
    },
};

ajv.addSchema(defaultSchema);
ajv.addSchema(testSchema);

var validate = ajv.compile(mergedSchema);

test({"item1": "abc", "item2": "def", "item3": "ghi"});
test({"item1": "abc", "item2": "xxx", "item3": { "test": "ok"}});

function test(data) {
  var valid = validate(data);
  if (valid) console.log('Valid!');
  else console.log('Invalid: ' + ajv.errorsText(validate.errors));
}

My thought was that each additonalProperties would be "combined" and applied to the new merged schema, making nothing allowed beyond item1, item2, and item3. But what it appears to be doing is enforcing each one separately, defaultSchema won't allow anything other than item1, testSchema won't allow anything other then item2 and item3, thus causing a merged schema to always fail validation. Noteworthy is if I remove the if-then-else and simply define item2 and item3 with additionalProperties: false, it passes:

const testSchema = {
    "$id": "testSchema",
    "type": "object",
    "properties": {
        "item2": {
            "type": "string",
        },
        "item3": {
            "type": "string",
        },
    },
   "additionalProperties": false
};

So it appears to be something with doing a merge and an if-then-else

Clearly I'm wrong in my understanding, is there a way to achieve validation with if-then-else and additonalProperties: false?

[stoked10]

As I've been playing with this all morning, I also tried to add additionalProperties after the merge via a patch. This also seems to not work:

with: [{op: 'add', path: '/additionalProperties', value: false}]

[epoberezkin]

same as #21 - outer $patch is processed before the inner $merge, that's how it is defined.