Field federation:backends in ValidationResult

[christophfriedrich]

I just pushed a fix (109ac35) because that field got filled incorrectly, but that was just a copy&paste error.

However, I feel there's also a semantic error here: The code ALWAYS fills that field.

IMO that's incorrect: Not only should this field be non-existent for responses from non-federated backends in general, but in the case of federated backends its presence or not actually has a deeper meaning, see https://github.com/Open-EO/openeo-api/blob/draft/extensions/federation/README.md#validation

• Endpoint returns without errors:
  • federation:backends is included in the response: [...]
  • federation:backends is not included in the response: [...]
• Endpoint returns errors:
  • federation:backends is included in the response: [...]
  • federation:backends is not included in the response: [...]

But currently, the field is mandatory in a ValidationResult object:

openeo-js-client/src/typedefs.js

Lines 263 to 272 in d7ef9c4

	/**
	* An array, but enriched with additional details from an openEO API response.
	*
	* Adds the property `federation:backends`.
	*
	* @typedef ValidationResult
	* @augments Array
	* @type {Array.<ApiError>}
	* @property {Array.<string>} ["federation:backends"] The back-ends that support / do not support the process.
	*/

To fix this, in analogy to e.g.

openeo-js-client/src/typedefs.js

Line 260 in d7ef9c4

* @property {?Array.<Link>} links

I'd change line 271 to:

@property {?Array.<string>} ["federation:backends"]

and change the line I fixed in the commit linked above

openeo-js-client/src/connection.js

Line 736 in 109ac35

errors['federation:backends'] = Array.isArray(response.data['federation:backends']) ? response.data['federation:backends'] : [];

to:

if (Array.isArray(response.data['federation:backends']) && response.data['federation:backends'].length > 0) {
	errors['federation:backends'] = response.data['federation:backends'];
}

okay? @m-mohr

[m-mohr]

Sounds reasonable to me. Just to make sure, I think the ? in front of the datatype implies null, not undefined (please check). So in the end we switch meanings a bit in the JS client in that not provided/undefines here will mean null, which is fine, because it's unambiguous.

So it might be that neither my nor your code is correct and maybe it should be:

errors['federation:backends'] = Array.isArray(response.data['federation:backends']) ? response.data['federation:backends'] : null;

[christophfriedrich]

Indeed it does mean a nullable type (source). In that case I would actually change my suggestion to an optional property:

@property {Array.<string>} ["federation:backends"]?

This way we retain the meaning of "undefined", which is more appropriate IMO.

[m-mohr]

I wasn't aware that was a thing and I think we never used it yet, but if it works, why not 😅

[christophfriedrich]

Wait, I threw Typescript and JSDoc syntax together... The syntax for optional in JSDoc is:

@param {number} [foo]
// or:
@param {number=} foo

(see my source link from above, but further down on that page)

So I think

@property {Array.<string>} ["federation:backends"]

already makes it optional? So far I thought the [] are there because they are somehow necessary if you use the quotes because of the : in the property name...

[m-mohr]

I thought so, too. It's a mess in JS with JSDoc and TS sometimes being different. In the worst case just describe it in text or we use null, which also is totally reasonable.