diff --git a/lib/descriptor-builder.js b/lib/descriptor-builder.js index a37745a..535b0d8 100644 --- a/lib/descriptor-builder.js +++ b/lib/descriptor-builder.js @@ -135,11 +135,20 @@ DescriptorBuilder.prototype.addNamedProperty = function(p, validate) { propsByName = this.propertiesByName; if (validate) { - this.assertNotDefined(p, ns.name); - this.assertNotDefined(p, ns.localName); + this.assertNotDefined(p); } - propsByName[ns.name] = propsByName[ns.localName] = p; + if (!propsByName[ns.name]) { + propsByName[ns.name] = p; + + if (ns.prefix === this.ns.prefix || !propsByName[ns.localName]) { + + // property defined in local ns, also add with localName + propsByName[ns.localName] = p; + } + } + else if (!propsByName[ns.localName]) + propsByName[ns.localName] = p; }; DescriptorBuilder.prototype.removeNamedProperty = function(p) { @@ -172,15 +181,23 @@ DescriptorBuilder.prototype.setIdProperty = function(p, validate) { this.idProperty = p; }; -DescriptorBuilder.prototype.assertNotDefined = function(p, name) { - var propertyName = p.name, - definedProperty = this.propertiesByName[propertyName]; +DescriptorBuilder.prototype.assertNotDefined = function(p) { + var ns = p.ns, + localName = ns.localName, + nsName = ns.name, + propByName = this.propertiesByName; + + // redefining properties inside the same namespace without 'redefines' or + // 'replaces' is not allowed. + if (propByName[nsName] && propByName[localName]) { + var definedProperty = propByName[localName], + prevDefinedName = definedProperty.definedBy.ns.name + '#' + localName, + curDefinedName = p.definedBy.ns.name + '#' + localName; - if (definedProperty) { throw new Error( - 'property <' + propertyName + '> already defined; ' + - 'override of <' + definedProperty.definedBy.ns.name + '#' + definedProperty.ns.name + '> by ' + - '<' + p.definedBy.ns.name + '#' + p.ns.name + '> not allowed without redefines'); + 'property <' + localName + '> already defined in namespace <' + + ns.prefix + '>; override of <' + prevDefinedName + '> by ' + + '<' + curDefinedName + '> not allowed without redefines or replaces'); } }; @@ -230,4 +247,4 @@ DescriptorBuilder.prototype.addTrait = function(t, inherited) { types.push(t); typesByName[typeName] = t; -}; \ No newline at end of file +}; diff --git a/lib/factory.js b/lib/factory.js index 64330c9..c6de684 100644 --- a/lib/factory.js +++ b/lib/factory.js @@ -27,7 +27,17 @@ Factory.prototype.createType = function(descriptor) { // initialize default values forEach(descriptor.properties, function(p) { if (!p.isMany && p.default !== undefined) { - prototype[p.name] = p.default; + var name; + + if (p.ns.prefix === descriptor.ns.prefix) { + + // use local name if property already in local ns + name = p.name; + } + else { + name = p.ns.name; + } + prototype[name] = p.default; } }); @@ -58,4 +68,4 @@ Factory.prototype.createType = function(descriptor) { props.defineDescriptor(ModdleElement, descriptor); return ModdleElement; -}; \ No newline at end of file +}; diff --git a/lib/properties.js b/lib/properties.js index 4201032..20dad26 100644 --- a/lib/properties.js +++ b/lib/properties.js @@ -19,10 +19,10 @@ export default function Properties(model) { Properties.prototype.set = function(target, name, value) { var property = this.model.getPropertyDescriptor(target, name); - - var propertyName = property && property.name; + var propertyName = getPropertyName(target, name, property); if (isUndefined(value)) { + // unset the property, if the specified value is undefined; // delete from $attrs (for extensions) or the target itself if (property) { @@ -31,13 +31,14 @@ Properties.prototype.set = function(target, name, value) { delete target.$attrs[name]; } } else { - // set the property, defining well defined properties on the fly - // or simply updating them in target.$attrs (for extensions) + + // set the property, defining well defined properties on the fly or simply + // updating them in target.$attrs (for extensions) if (property) { if (propertyName in target) { target[propertyName] = value; } else { - defineProperty(target, property, value); + defineProperty(target, propertyName, property, value); } } else { target.$attrs[name] = value; @@ -45,6 +46,7 @@ Properties.prototype.set = function(target, name, value) { } }; + /** * Returns the named property of the given element * @@ -56,16 +58,15 @@ Properties.prototype.set = function(target, name, value) { Properties.prototype.get = function(target, name) { var property = this.model.getPropertyDescriptor(target, name); + var propertyName = getPropertyName(target, name, property); if (!property) { return target.$attrs[name]; } - var propertyName = property.name; - // check if access to collection property and lazily initialize it if (!target[propertyName] && property.isMany) { - defineProperty(target, property, []); + defineProperty(target, propertyName, property, []); } return target[propertyName]; @@ -103,11 +104,28 @@ function isUndefined(val) { return typeof val === 'undefined'; } -function defineProperty(target, property, value) { - Object.defineProperty(target, property.name, { + +function getPropertyName(target, name, descriptor) { + if (!target || !name || !descriptor) + return undefined; + + var propertyName = descriptor.name; + + if (name.includes(':')) { + var parts = name.split(':'); + + if (parts[0] !== target.$descriptor.ns.prefix) + propertyName = descriptor.ns.name; + } + + return propertyName; +} + +function defineProperty(target, name, property, value) { + Object.defineProperty(target, name, { enumerable: !property.isReference, writable: true, value: value, configurable: true }); -} \ No newline at end of file +} diff --git a/test/fixtures/model/datatype.json b/test/fixtures/model/datatype.json index 95b20e9..e3546c4 100644 --- a/test/fixtures/model/datatype.json +++ b/test/fixtures/model/datatype.json @@ -7,7 +7,10 @@ "name": "Root", "properties": [ { "name": "bounds", "serialize": "xsi:type", "type": "Rect" }, - { "name": "otherBounds", "type": "do:Rect", "serialize": "xsi:type", "isMany": true } + { "name": "otherBounds", "type": "do:Rect", "serialize": "xsi:type", "isMany": true }, + { + "name": "defaultSingle", "type": "Boolean", "default": true + } ] }, { @@ -17,4 +20,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/test/fixtures/model/multiple-inherited-properties.json b/test/fixtures/model/multiple-inherited-properties.json new file mode 100644 index 0000000..8a3770a --- /dev/null +++ b/test/fixtures/model/multiple-inherited-properties.json @@ -0,0 +1,53 @@ +{ + "name": "MultipleInheritance", + "uri": "http://multipleinheritance", + "prefix": "mh", + "types": [ + { + "name": "AnotherRoot", + "properties": [ + { + "name": "any", + "type": "AnotherRoot", + "isMany": true + }, + { + "name": "many", + "type": "String", + "isMany": true + }, + { + "name": "single", + "type": "String" + }, + { + "name": "defaultSingle", + "type": "String", + "default": "mh-default-string" + } + ] + }, + { + "name": "MultipleInherited", + "superClass": [ "AnotherRoot", "props:Root", "dt:Root" ], + "properties": [ + { + "name": "anotherDefaultSingle", + "type": "Boolean", + "default": false + } + ] + }, + { + "name": "InvalidMultipleInherited", + "superClass": [ "AnotherRoot", "props:Root" ], + "properties": [ + { + "name": "defaultSingle", + "type": "Boolean", + "default": false + } + ] + } + ] +} diff --git a/test/fixtures/model/properties.json b/test/fixtures/model/properties.json index b8ead1d..d8a51f7 100644 --- a/test/fixtures/model/properties.json +++ b/test/fixtures/model/properties.json @@ -84,6 +84,13 @@ { "name": "idNumeric", "type": "Integer", "isAttr": true, "redefines": "BaseWithId#id", "isId": true } ] }, + { + "name": "BaseWithAlreadyDefinedId", + "superClass": [ "BaseWithId" ], + "properties": [ + { "name": "id", "type": "Integer", "isAttr": true, "isId": true } + ] + }, { "name": "Attributes", "superClass": [ "BaseWithId" ], @@ -122,6 +129,25 @@ "name": "any", "type": "Base", "isMany": true + }, + { + "name": "many", + "type": "Integer", + "isMany": true + }, + { + "name": "single", + "type": "Integer" + }, + { + "name": "defaultSingle", + "type": "Integer", + "default": 42 + }, + { + "name": "anotherDefaultSingle", + "type": "Integer", + "default": 23 } ] }, @@ -171,4 +197,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/test/spec/moddle.js b/test/spec/moddle.js index 2150cae..206149f 100644 --- a/test/spec/moddle.js +++ b/test/spec/moddle.js @@ -101,7 +101,10 @@ describe('moddle', function() { expect(descriptor.ns).to.jsonEqual(expectedDescriptorNs); expect(descriptor.properties).to.jsonEqual(expectedDescriptorProperties); - expect(descriptor.propertiesByName).to.jsonEqual(expectedDescriptorPropertiesByName); + expect(descriptor.propertiesByName.id).to + .equal(descriptor.propertiesByName['props:id']); + expect(descriptor.propertiesByName['props:id']).to + .jsonEqual(expectedDescriptorPropertiesByName['props:id']); }); diff --git a/test/spec/multiple-inherited-properties.js b/test/spec/multiple-inherited-properties.js new file mode 100644 index 0000000..4d5db8f --- /dev/null +++ b/test/spec/multiple-inherited-properties.js @@ -0,0 +1,400 @@ +import expect from '../expect'; +import { createModelBuilder } from '../helper'; + + +describe('multiple inherited properties', function() { + + // given + + // Moddle instance with multiple superClasses partly from different + // namespaces. + var createModel = createModelBuilder('test/fixtures/model/'); + var mhModel = createModel([ + 'properties', + 'multiple-inherited-properties', + 'datatype' + ]); + + + describe('Type', function() { + + it('should provide Type', function() { + + // when + var getType = function() { + mhModel.getType('mh:MultipleInherited'); + }; + + // then + expect(getType).to.not.throw(); + + var Type = eval(getType); + expect(Type).to.exist; + }); + + it('should provide descriptor', function() { + + // given + var Type = mhModel.getType('mh:MultipleInherited'); + + // when + var getDescriptor = function() { + mhModel.getElementDescriptor(Type); + }; + + // then + expect(getDescriptor).to.not.throw(); + + var descriptor = mhModel.getElementDescriptor(Type); + expect(descriptor).to.exist; + }); + + it('should provide descriptor', function() { + + // given + var Type = mhModel.getType('mh:MultipleInherited'); + + // when + var getDescriptor = function() { + mhModel.getElementDescriptor(Type); + }; + + // then + expect(getDescriptor).to.not.throw(); + + var descriptor = mhModel.getElementDescriptor(Type); + expect(descriptor).to.exist; + }); + + describe('invalid', function() { + + it('should NOT provide Type', function() { + + // when + var getType = function() { + mhModel.getType('mh:InvalidMultipleInherited'); + }; + + // then + expect(getType).to.throw( + 'property already defined in namespace ; ' + + 'override of by ' + + ' not allowed ' + + 'without redefines or replaces'); + }); + + }); // describe(multiple inherited/Type/invalid) + + describe('descriptor', function() { + + // given + var Type = mhModel.getType('mh:MultipleInherited'); + var descriptor = mhModel.getElementDescriptor(Type); + var propsByName = descriptor.propertiesByName; + + it('should map properties by name', function() { + + // then + expect(propsByName.any).to.exist; + expect(propsByName['mh:any']).to.exist; + expect(propsByName['props:any']).to.exist; + expect(propsByName.any).to.eql(propsByName['mh:any']); + + expect(propsByName.many).to.exist; + expect(propsByName['mh:many']).to.exist; + expect(propsByName['props:many']).to.exist; + expect(propsByName.many).to.eql(propsByName['mh:many']); + + expect(propsByName.single).to.exist; + expect(propsByName['mh:single']).to.exist; + expect(propsByName['props:single']).to.exist; + expect(propsByName.single).to.equal(propsByName['mh:single']); + + expect(propsByName.defaultSingle).to.exist; + expect(propsByName['mh:defaultSingle']).to.exist; + expect(propsByName['props:defaultSingle']).to.exist; + expect(propsByName['dt:defaultSingle']).to.exist; + expect(propsByName.defaultSingle).to + .equal(propsByName['mh:defaultSingle']); + + expect(propsByName.anotherDefaultSingle).to.exist; + expect(propsByName['mh:anotherDefaultSingle']).to.exist; + expect(propsByName['props:anotherDefaultSingle']).to.exist; + expect(propsByName.anotherDefaultSingle).to + .equal(propsByName['mh:anotherDefaultSingle']); + }); + + it('should describe defined property type', function() { + + // then + expect(propsByName.any.type).to.equal('mh:AnotherRoot'); + + // Note: no need to test 'mh:any' as we test equality with 'any' in + // 'should map properties by name' + + expect(propsByName['props:any'].type).to.equal('props:Base'); + + expect(propsByName.many.type).to.equal('String'); + expect(propsByName['props:many'].type).to.equal('Integer'); + + expect(propsByName.single.type).to.equal('String'); + expect(propsByName['props:single'].type).to.equal('Integer'); + + expect(propsByName.defaultSingle.type).to.equal('String'); + expect(propsByName['props:defaultSingle'].type).to.equal('Integer'); + expect(propsByName['dt:defaultSingle'].type).to.equal('Boolean'); + + expect(propsByName.anotherDefaultSingle.type).to.equal('Boolean'); + expect(propsByName['props:anotherDefaultSingle'].type).to + .equal('Integer'); + }); + + }); // describe(multiple inherited/Type/descriptor) + + }); // describe(multiple inherited/Type) + + describe('instance', function() { + + it('should be created', function() { + + // when + var instance = mhModel.create('mh:MultipleInherited'); + + // then + expect(instance).to.exist; + }); + + describe('non-many', function() { + + describe('default', function() { + + // given + var instance = mhModel.create('mh:MultipleInherited'); + + // when + var originalProperty = instance.defaultSingle; + + // Note: access to local name ONLY with 'get()'. The local name is NOT + // mapped into the object! + + var localProperty = instance.get('mh:defaultSingle'); + var otherProperty = instance['props:defaultSingle']; + var anotherProperty = instance['dt:defaultSingle']; + + it('should exist', function() { + + // then + expect(originalProperty).to.exist; + expect(localProperty).to.exist; + expect(otherProperty).to.exist; + expect(anotherProperty).to.exist; + }); + + it('should hold different values', function() { + + // then + expect(originalProperty).to.equal('mh-default-string'); + expect(otherProperty).to.equal(42); + expect(anotherProperty).to.equal(true); + expect(originalProperty).to.eql(localProperty); + }); + + }); // describe(multiple inherited/instance/non-many/default) + + describe('initialize with create', function() { + + it('should initialize values via original name', function() { + + // given + var instance = mhModel.create('mh:MultipleInherited', { + single: 'non-default-string', + 'props:single': 23 + }); + + // when + var originalProperty = instance.single; + + // Note: access to local name ONLY with 'get()'. The local name is NOT + // mapped into the object! + + var localProperty = instance.get('mh:single'); + var otherProperty = instance['props:single']; + + // then + expect(originalProperty).to.equal('non-default-string'); + expect(otherProperty).to.equal(23); + expect(originalProperty).to.eql(localProperty); + }); + + it('should initialize values via local name', function() { + + // given + var instance = mhModel.create('mh:MultipleInherited', { + 'mh:single': 'mh-non-default-string', + 'props:single': 23 + }); + + // when + var originalProperty = instance.single; + var localProperty = instance.get('mh:single'); + var otherProperty = instance['props:single']; + + // then + expect(originalProperty).to.equal('mh-non-default-string'); + expect(otherProperty).to.equal(23); + expect(originalProperty).to.eql(localProperty); + }); + + }); // describe(multiple inherited/instance/non-many/initialize with create) + + }); // describe(multiple inherited/instance/non-many) + + describe('many', function() { + + describe('initialize with create', function() { + + it('should initialize values via original name', function() { + + // given + var instance = mhModel.create('mh:MultipleInherited', { + many: [ 'mh-original-string' ], + 'props:many': [ 23 ] + }); + + // when + var originalProperty = instance.many; + var localProperty = instance.get('mh:many'); + var otherProperty = instance['props:many']; + + // then + expect(originalProperty).to.exist; + expect(localProperty).to.exist; + expect(otherProperty).to.exist; + + expect(originalProperty.length).to.equal(1); + expect(originalProperty).to.eql([ 'mh-original-string' ]); + + expect(otherProperty.length).to.equal(1); + expect(otherProperty).to.eql([ 23 ]); + + expect(originalProperty).to.eql(localProperty); + }); + + it('should initialize values via local name', function() { + + // given + var instance = mhModel.create('mh:MultipleInherited', { + 'mh:many': [ 'mh-local-string' ], + 'props:many': [ 23 ] + }); + + // when + var originalProperty = instance.many; + var localProperty = instance.get('mh:many'); + var otherProperty = instance['props:many']; + + // then + expect(localProperty.length).to.equal(1); + expect(localProperty).to.eql([ 'mh-local-string' ]); + + expect(otherProperty.length).to.equal(1); + expect(otherProperty).to.eql([ 23 ]); + + expect(originalProperty).to.eql(localProperty); + }); + + }); // describe(multiple inherited/instance/many/initialize with create) + + describe('get', function() { + + // given + var instance = mhModel.create('mh:MultipleInherited', { + many: [ 'mh-original-string' ], + 'props:many': [ 23 ] + }); + + // when + var originalProperty = instance.get('many'); + var localProperty = instance.get('mh:many'); + var otherProperty = instance.get('props:many'); + + + it('should access via original name', function() { + + // then + expect(originalProperty).to.eql([ 'mh-original-string' ]); + expect(originalProperty).to.eql(localProperty); + }); + + + it('should access via local name', function() { + + // then + expect(localProperty).to.eql([ 'mh-original-string' ]); + expect(localProperty).to.eql(localProperty); + }); + + + it('should access via other name', function() { + + // then + expect(otherProperty).to.eql([ 23 ]); + }); + + }); // describe(multiple inherited properties/instance/many/get) + + describe('set', function() { + + it('should modify via original name', function() { + + // given + var instance = mhModel.create('mh:MultipleInherited'); + + // when + instance.set('many', [ 'test-many' ]); + instance.set('props:many', [ 23 ]); + + var originalProperty = instance.get('many'); + var localProperty = instance.get('mh:many'); + var otherProperty = instance.get('props:many'); + + // then + expect(originalProperty.length).to.equal(1); + expect(originalProperty).to.eql([ 'test-many' ]); + + expect(otherProperty.length).to.equal(1); + expect(otherProperty).to.eql([ 23 ]); + + expect(originalProperty).to.eql(localProperty); + }); + + it('should modify via local name', function() { + + // given + var instance = mhModel.create('mh:MultipleInherited'); + + // when + instance.set('mh:many', [ 'test-many' ]); + instance.set('props:many', [ 23 ]); + + var originalProperty = instance.get('many'); + var localProperty = instance.get('mh:many'); + var otherProperty = instance.get('props:many'); + + // then + expect(originalProperty.length).to.equal(1); + expect(originalProperty).to.eql([ 'test-many' ]); + + expect(otherProperty.length).to.equal(1); + expect(otherProperty).to.eql([ 23 ]); + + expect(originalProperty).to.eql(localProperty); + }); + + }); // describe(multiple inherited properties/instance/many/set) + + }); // describe(multiple inherited/instance/instance/many) + + }); // describe(multiple inherited properties/instance) + +}); // describe(multiple inherited properties) diff --git a/test/spec/properties.js b/test/spec/properties.js index fb75103..636b17b 100644 --- a/test/spec/properties.js +++ b/test/spec/properties.js @@ -4,7 +4,6 @@ import { createModelBuilder } from '../helper'; - describe('properties', function() { var createModel = createModelBuilder('test/fixtures/model/'); @@ -73,6 +72,22 @@ describe('properties', function() { expect(inheritedAnyProperty).to.exist; }); + + it('should NOT add already defined property without redefine', function() { + + // when + var getType = function() { + model.getType('props:BaseWithAlreadyDefinedId'); + }; + + // then + expect(getType).to.throw( + 'property already defined in namespace ; ' + + 'override of by ' + + ' not allowed ' + + 'without redefines or replaces'); + }); + });