Minor changes on chapters 4 and 5

[relu91]

This PR reports @egekorkan review changes in chapters 4 and 5 (no comments were left on chapter 3). See the related #230 (comment) and the pdf document

---

Preview | Diff

[zolkis]

Why to remove the algorithm name?

[zolkis]

Same: I don't think algorithm names and params should be removed.

[relu91]

I agree, see my review.

[relu91]

I think this element should be kept, is it @zolkis?

[zolkis]

Yes.

[relu91]

@egekorkan comment on this change:

The JSON Schema validation should be before step 2 and that way if something fails the schema validation, it must have values for terms that do not have defaults. The schema validation does not expect a TD with defaults filled in.

[relu91]

I'd leave as it was. As far as I understand if a property is marked as required in JSONSchema even if it has a default value, it must appear in the validating object. However, I'd add a step to make the process a little bit clearer:

1. Check if it is an Object
2. Check if any of the mandatory fields with no default are not defined.
3. Fill mandatory fields with default
4. Do JSONSchema validation

[zolkis]

I agree with the sequence proposed above.

[relu91]

I added a filling default step in the validation algorithm. This is because AFAIK JSONschema validation fails if a required property is not present even if it has a default value. @egekorkan could you check this?

[relu91]

It seems there is a relevant discussion about this topic in eclipse-thingweb/node-wot#302

[egekorkan]

The thing is that the values that have a default value are not required in the official validation schema, i.e. if a property form does not have an op value, it will pass validation. Thus, an algorithm that fills the defaults should check that a TD is valid in the first place. In Thingweb Playground, the first schema validation corresponds to the validation with the official schema whereas the second validation that says with defaults correspond to a validation where all the values that have defaults should be filled. The previous example with a property with no op fails the second validation.

[zolkis]

Suggestion: usually these complex steps should be more detailed, e.g. using sub-steps with a loop, or an explicit loop, for instance

For each mandatory property defined in [[!WOT-TD]] that have default definitions, add the property to |td| if it's missing and set its value to the default value specified in [[!WOT-TD]].

[zolkis]

Looks like there are no mandatory properties in the TD spec that would have default values, so this text needs updating. We need an initialization policy for the default values of mandatory TD properties. I suggest we create a separate issue about that in the TD repo, then link it here and also in a new issue in this repo. Then we can resolve this and merge this PR.

[relu91]

In Thingweb Playground, the first schema validation corresponds to the validation with the official schema whereas the second validation that says with defaults correspond to a validation where all the values that have defaults should be filled. The previous example with a property with no op fails the second validation.

The thing here is that if it has a default, it probably does not need to be defined.... I agree with @danielpeintner here:

However I don't think that any implementation needs to add these default values. What would be the idea of having them in the first place if they NEED to get added. My take on this is that IF there is no value the default values kicks in for any processing. The reason to NOT HAVE them is manifold:

• reduced data exchange (TD more compact)

• simpler to see non defaults

• avoiding to exchange knowledge that is default anyway

[relu91]

Meanwhile, I updated the validation algorithm to have a really basic starting point. I removed the following steps:

1. If td is not an object, throw a "TypeError" and abort these steps.
2. If any of the mandatory properties defined in [WOT-TD] for Thing that don't have default definitions are missing from td, throw a "TypeError" and abort these steps.

They are both covered by the JSONSchema validation process. The only difference is that JSONSchema is more restrictive on point 2. It fails even if the mandatory property has a default assigned. However, since Looks like there are no mandatory properties in the TD spec that would have default values I think we can omit it.

[zolkis]

Please include an editor's note mentioning that more steps are going to be included for filling the mandatory properties defined in the TD spec, once the default values are specified in the TD spec (and please file an issue on the TD spec for this).