Upgrade to last data model release (hasvariant)

[RachL]

The current changelog is on the next branch :
https://github.com/datafoodconsortium/data-model-uml/blob/next/CHANGELOG.md

[jgaehring]

So I'm building the TS connector w/ the updates to data-model-uml for isVariantOf and hasVariant properties, but I ran into an error when compiling from TypeScript that I'm unsure how to address.

Basically, the variants property is an array of DefinedProduct objects, not just a single object, so I'm not sure the intended behavior of setVariants: should it replace everything, or just add whichever products aren't already included? or is setVariants unnecessary if there are a addVariant and removeVariant methods already? I wasn't sure if this was just a mistake on my part with the code generator and it was generating those setters by mistake, but they are in fact part of the updated UML and in CHANGELOG as well.

And regarding the CHANGELOG, I don't think I realized this wasn't pegged to any release for the data model. I was planning to point to the data model's release tag in the TS release and commit history, and to link to the data model's CHANGELOG from the TS CHANGELOG. That way there would be a definite set of properties, interfaces, etc that can be identified with and pegged to the TypeScript release. Would it make sense to give data-model-uml some kind of provisional release tag in git, something like v2.2.0-RC-1 or something to that effect?

[mkllnk]

And regarding the CHANGELOG, I don't think I realized this wasn't pegged to any release for the data model.

For the Ruby version we link to the used data-model commit:

• https://github.com/datafoodconsortium/connector-ruby/blob/main/CHANGELOG.md

I'm wondering if it would make sense to declare the data model as Git submodule. Then the data-model commit would be tracked in the Git history as well.

[jgaehring]

For the Ruby version we link to the used data-model commit:

* https://github.com/datafoodconsortium/connector-ruby/blob/main/CHANGELOG.md

Ah yes, that would work too.

I'm wondering if it would make sense to declare the data model as Git submodule. Then the data-model commit would be tracked in the Git history as well.

I was having the same thought. It would make sense and save a couple steps since it's already part of the normal procedure to clone the repo into the project anyways.

[lecoqlibre]

I'm not sure the intended behavior of setVariants

Yes setVariants is intended to replace everything while addVariant adds one variant and removeVariant removes one variant.

Normally the generator should not produce errors when new methods are introduced. Can I see the definition of the methods producing errors? @jgaehring

To link the data-model to a generation I also just write into the changelog. I'm open to other solutions and a Git submodule can be a good idea.

[jgaehring]

Yes setVariants is intended to replace everything while addVariant adds one variant and removeVariant removes one variant.

Ah, okay, then there it is a bug in the setter generation. Here's the setter's definition:

[template public generateSetterBody(aClass: Class, operation: Operation)]
[let parameter: Parameter = operation.getInputParameter()]
[let property: Property = operation.getProperty(aClass)]
[let map: String = property.getMapping()][if (parameter.type.isPrimitive())]
this.setSemanticPropertyLiteral("[map/]", [parameter.name/]);[else]
this.setSemanticProperty[if (parameter.type.isBlankNode())]Anonymous[else]Reference[/if]("[map/]", [parameter.name/]);
[if (not parameter.type.isBlankNode())]['\n'/]this.connector.store([parameter.name/]);[/if][/if][/let][/let][/let]
[/template]

(with syntax highlighting:)

Seems clear to me now that it's missing a condition for parameter.upper = -1 (or maybe parameter.isMultiValued()?). I guess it's surprising it hasn't caused trouble before.

[jgaehring]

@lecoqlibre, does this look like a correct implementation for the DefinedProduct::setVariants method?

	public setVariants(variants: IDefinedProduct[]): void {
		const store = new ConnectorStoreMap();
		variants.forEach(variant => {
			this.addSemanticPropertyReference('dfc-b:hasVariant', variant, true);
			store.set(variant);
		});
		this.connector.setDefaultStore(store);
	}

If so, we should add setDefaultStore to the connector's interface; it's implemented as a public method on the Connector class, but not explicitly part of the IConnector interface.

I can update this from the code generator if that looks right to you. Hopefully I will have more time tomorrow, but if not, by next week.

[lecoqlibre]

@jgaehring I'm wondering why you reassign a store to the connector in the method body? Setters are supposed to use the store of the object settled in the constructor.

[lecoqlibre]

Seems clear to me now that it's missing a condition for parameter.upper = -1 (or maybe parameter.isMultiValued()?). I guess it's surprising it hasn't caused trouble before.

@jgaehring We may have not a multiple setter yet. You should check that the isMultiValued method does what you want or otherwise write a query in the the queries.mtl file.

[jgaehring]

So I rebuilt the TS libary based on our discussions, and it looks pretty good to me, passing no fewer tests than usual. Here's the latest commit that fixes the setter issue in the connector-codegen repo:
datafoodconsortium/connector-codegen@3819297

But I only just now noticed there's a bunch of src files that are missing since the previous alpha-9 build of the TS library:

jamie@xps-13-9300:~/Code/datafoodconsortium/connector-typescript$ git status src/ | grep deleted:
        deleted:    src/Contactable.ts
        deleted:    src/Emailable.ts
        deleted:    src/IAllergenDimension.ts
        deleted:    src/ICertification.ts
        deleted:    src/ICharacteristicDimension.ts
        deleted:    src/IClaim.ts
        deleted:    src/IGeographicalOrigin.ts
        deleted:    src/INatureOrigin.ts
        deleted:    src/INutrientDimension.ts
        deleted:    src/IPartOrigin.ts
        deleted:    src/IPhysicalDimension.ts
        deleted:    src/IProductType.ts
        deleted:    src/IUnit.ts
        deleted:    src/Identifiable.ts
        deleted:    src/Supplier.ts
        deleted:    src/index.ts
        deleted:    src/terms.ts

This is a bit alarming, but perhaps these are just relics from some outdated model or something?

You can view the full diffs for the generated TS and transpiled JS here:

v1.0.0-alpha.9...jgaehring:rc-alpha-10

I may not have much time to take a look at this closely or run further builds tomorrow, but the downstream consumers of the TS library do have the branch URL they can pull directly from GH via their package.json. So if it works for them an other ppl are cofortable with puling the trigger, I can open a PR that can get merged in and tagged (someone else may need to publish to npm, don't think I have permission), or perhaps @lecoqlibre would be able to pull from my codegen fork to see what's the matter with those missing src files, the run the build again locally and merge/tag/publish/etc.