JSON rendering: contextElements should be list, not object

[fgalan]

UpdateContextRequest en JSON mal construido. Es JSON valido, pero se supone que "contextElements" es un vector. Ejemplo:

{
    "contextElements": {
        "contextElement": {
            "type": "Room",
            "isPattern": "false",
            "id": "Room1",
            "attributes": [
            {
                "name": "temperature",
                "type": "centigrade",
                "value": "23"
            },
            {
                "name": "pressure",
                "type": "mmHg",
                "value": "720"
            }
            ]
        }
    },
    "updateAction": "APPEND"
}

Pasa lo mismo con un discoverContextAvailabilityResponse. Quizá haya más.

Una vez arreglado, habría que corregir también la documentación en la wiki consecuentemente, incluyendo un warning para usuarios de versiones antiguas de que el formato ha cambiado.

[LeandroGuillen]

This is kind of a weird bug. The "problem" is that the broker will accept this request (see above), which is syntactically valid JSON. But it will treat the message as if it were a true vector.

There isn't really nothing to change in the code other than making the broker prevent this type of request. Should we do that? The message would be: Even if you only have ONE contextElement we force you to write a contextElements vector with one item inside. This would make sense in the name of consistency.

In any case, I have updated the wiki so that the only example available is the one with a vector.

[fgalan]

IMHO, the parser shouldn't allow the ambiguity of allowing both object or vector for "contextElements" field, i.e. it should enforce the use of a vector and report a syntax error bug (the error message you mention would be ok) is the client tries to use an object.

If we implement that, the wiki should include a release note warning about that in pre-0.10.0 the use of both options was allowed, but in 0.10.0 we have fix this in order to enforce vectors.

We have detected this problem in "contextElements" but... it may happen in some other case in which we use vectors? Maybe is worth the time to make a whole revision of this and come with a common solution for all cases in the JSON parser logic.

[LeandroGuillen]

I have devised a solution to this problem, but I am not entirely sure this is right (https://github.com/telefonicaid/fiware-orion/commit/824a367a469724e0aad4209438bf9d4b1a7bdd84). In short, it looks for a structure like this:

"somethings": {
  "something" : {
  }
}

And returns an error if found. Could we see this kind of structure in a JSON document that is legal?

[kzangeli]

I'm not sure it's a good idea to put this 'semantic' check in the parser.

Also, it is a bit more complicated than checking just for 'ies'. Some plurals just add an 's' and others change a 'y' for 'ie' (plus the trailing 's' in the plural.

But especially, you'd have to compare the two stems also. Something like this:

tag1 = "somethings"; tag2 = "something";

if ((lastChar(tag2) != 'y') && (strncmp(tag1, tag2, strlen(tag2)) == 0) checkBraces(); else if (lastChar(tag2) == 'y') && (strncmp(tag1, tag2, strlen(tag2)- 1)) == 0) checkBraces();

// There may be more ways of changing for English plurals ...

[LeandroGuillen]

I believe all those cases are covered (y->ies and _->_s). There may be other plurals in English, but since it is us who define the tags I didn't see this as a problem. Although it could be annoying if we ever define a tag named "box" since we need the "-es". Any idea on how to solve this without checking names and plurals?

[kzangeli]

I don't see any other way.

It is as you say; if we want this check it will have to be a bit manual. But the two stems have to be compared as well as both endings. Either that or we make a complete list of singular-plurals and we add to that list (std::vector) for all the pairs we want checked:

typedef struct PluralCheck { std::string singular; std::string plural } PluralCheck;

std::vector pluralVec = [ { "box", "boxes" }, { "entityId", "entityIds" }, ... ];

[LeandroGuillen]

All cases of UpdateContextRequest are correct in the wiki now.

[fgalan]

I think that it is too "ad hoc" solution. What about if the client uses, eg.:

"somethings": {
  "foo" : {
  }
}

From my point of view, the ideal solution would be to check that the content of the "somethings" field is a vector and raise an error if an object, string, number, whatever no-vector element in general is found.

[LeandroGuillen]

I may have not explained clearly enough, and the code doesn't seem to help :) It doesn't have a problem with that code at all.

The checking to see if it is a vector is done implicitly by the boost tree parser. It is commented in the jsonParse() function. I'll add some more explanatory comments to it, though.

[fgalan]

Closed (at code level) in PR #235 . However, we will keep this issue open until @fgalan can add the warning mentioned in the issue body about this in the wiki.

EDIT (Leandro): Typo in the link. Now it links to the correct PR.

[fgalan]

Text added in the following section https://forge.fi-ware.eu/plugins/mediawiki/wiki/fiware/index.php/Publish/Subscribe_Broker_-_Orion_Context_Broker_-_User_and_Programmers_Guide#Introduction_to_Programmers_Guide, thus closing the issue.