Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugs in /attrs PATCH endpoint #1385

Closed
Jxlle opened this issue Jun 13, 2023 · 13 comments
Closed

Bugs in /attrs PATCH endpoint #1385

Jxlle opened this issue Jun 13, 2023 · 13 comments

Comments

@Jxlle
Copy link

Jxlle commented Jun 13, 2023

When updating a context broker attribute of type Relationship using the /attrs endpoint where the new object value is an array of urns (e.g. ["urn:ngsi-ld:Cargo:002"] or ["urn:ngsi-ld:Cargo:001", "urn:ngsi-ld:Cargo:002"]), the relationship object value within the context broker gets updated to "" (empty string) instead of the array of urns.

Example: the entity of type Transport has an attribute cargo_ids which is a Relationship attribute. The attribute gets updated via .../ngsi-ld/v1/entities/urn:ngsi-ld:Transport:001/attrs/cargo_ids with the following payload:

{
    "type": "Relationship",
    "object": ["urn:ngsi-ld:Cargo:002"]
}

The output logs of the context broker (see below) show that it perceives this update as an error, but the request reponse is still 204 (No Content) as with a valid request:
time=Tuesday 13 Jun 15:40:10 2023.295Z | lvl=ERROR | corr=N/A | trans=N/A | from=N/A | srv=N/A | subsrv=N/A | comp=Orion | op=pCheckAttribute.cpp[509]:objectCheck | msg=***** ERROR: Invalid JSON type - not a string: object (status: 400)

When checking the cargo_ids attribute after this request with a GET, the following is returned:

{
    "id": "urn:ngsi-ld:Transport:001",
    "type": "Transport",
    "cargo_ids": {
        "object": "",
        "type": "Relationship"
    }
}

As you can see, the object value is now an empty string. I think that the object value should instead be updated to the given array?

This is not the case when the attribute type is Property. However, updating an attribute of type Property with an array with only one value inside of it (e.g. ["test"]), will result in the property value being set to the value within the array (e.g. "test") instead of an array with only one element inside of it. When the array has multiple values, the array is used as the value.Is this intentional?

I am using version 1.2.1.

@kzangeli
Copy link
Collaborator

ok, that sounds like a bug.
I will look into this as soon as I get the chance.

Thanks for reporting!

A word on Relationship attributes having the "object" as an array:

This is something I implemented by mistake (I though it made a whole lot of sense), but, it's against the NGSi-LD API spec,
so some day I will have to remove this "feature".

I will not remove it though until I have implemented the new (in v1.8.1) attribute type "ListRelationship", that solves the "problem" about being able to have Relationships as arrays.
Also, I will not remove it right away, but, deprecate it and wait a few releases before removing the support of arrays in Relationship attributes.

@Jxlle
Copy link
Author

Jxlle commented Jun 15, 2023

Hi @kzangeli

Thanks for the reply!
I did not know about the ListRelationship type. Is the v1.8.1 etsi ngsi-ld documentation already available somewhere? I did not find a mention of this type in v1.7.1 (2023-06): https://www.etsi.org/deliver/etsi_gs/CIM/001_099/009/01.07.01_60/gs_CIM009v010701p.pdf

@kzangeli
Copy link
Collaborator

Yes, a draft pre version of 1.8.1 has been published on the open area of ETSI isg cim.
I'll add a link to it shortly (after lunch :) right in this issue

@kzangeli
Copy link
Collaborator

https://docbox.etsi.org/isg/cim/open/drafts/ETSI_GS_CIM_009_NGSI-LD_API_DRAFTv1.8-for-PUBLIC-COMMENT.pdf

Only, bear in mind that this is not implemented in any NGSI-LD broker yet. It's fresh out of the oven, it's not even published yet, except in this "sneak-peek" of what's to come.

@Jxlle
Copy link
Author

Jxlle commented Jun 15, 2023

Thanks @kzangeli!
Yes I know :)

One last question. I might be wrong, but it seems that the showchanges property of the NotificationParams datatype (datatype of the notification property in a Subscription definition) is not implemented yet. Is there any estimate when this may be implemented? For my specific use-case, I need to know the old value of the changed attribute in my notifications.

@kzangeli
Copy link
Collaborator

Yes, you're right, I haven't implemented showChanges yet.
I frankly didn't think anybody would be interested in that feature :) so I didn't put it very high up in my ToDo ...
I'll give it more priority, now that I have an interested party for it.

But, it's not gonna be immediate, I have quite a lot to do, I'm afraid.
The bug you found is different, bugs are almost always "leave everything and fix the bug instead".

I'll do my best to have "showChanges" done before summer vacations (August)

@Jxlle
Copy link
Author

Jxlle commented Jun 15, 2023

No problem, thanks for the quick reply! Didn't think that you'd be so active.
I'll create more issues whenever I find more bugs :)

@kzangeli kzangeli mentioned this issue Jun 15, 2023
kzangeli added a commit that referenced this issue Jun 16, 2023
kzangeli added a commit that referenced this issue Jun 16, 2023
@kzangeli
Copy link
Collaborator

So, the bug has (hopefully) been fixed.
Please test and close this issue if all OK.

@Jxlle
Copy link
Author

Jxlle commented Jun 19, 2023

@kzangeli

Thanks for the quick fix, the main issue seems to be fixed!

However, I do still have a small remark. Why is it, that updating a Property with a value that is a single-item array (e.g. ["test"]) makes the value of Property equal to the item inside the array (e.g. "test"). This is not the case with a Relationship. Here, updating the object value to a single-item array (e.g. ["urn:ngsi-ld:Test:001"]) makes the object value of the Relationship equal to the single-item array (["urn:ngsi-ld:Test:001"]).

@kzangeli
Copy link
Collaborator

Yeah, I also don't like that "feature".
It's defined by json-ld and I have no other choice but to implement it that way.

Guess I never did that for relationships.
But, allowing arrays for relationship was an error anyway .

BTW, I started with "show changes" this weekend.
First part almost ready, possibly PR later today.
Don't tell my boss! :)))

I'll let you know

@Jxlle
Copy link
Author

Jxlle commented Jun 19, 2023

Ok, no problem!
I'll close this issue then.

Thanks for helping!!

@Jxlle Jxlle closed this as completed Jun 19, 2023
@kzangeli
Copy link
Collaborator

Just wanted you to know I haven't forgotten about your issues.
Got a new workstation the other day and I decided to dedicate some time to setting it up.
Got quite a few problems (with boost) to compile the broker under Ubuntu 22.04 (it came with 22.04 preinstalled and I didn't want to downgrade to 20.04).
And mongodb got me even more problems. Apparently, only mongodb 6.0 is supported under 22.04 and I can't use 6.0. I need a 4.4 or a 5.x (the mongo legacy cxx driver is no longer supported in 6.0.
So, took me an entire day to set it up.
Then, of course, meetings and stuff.

But, rest assured, I'm on the "showChanges" thing (found two bugs in the PATCH Entity operation while debugging "showChanges" and I need to fix those two in the same PR ,as functersts for showChanges fail).
After showChanges (which I only implement under "-experimental", like everything else that is new), I will start with the problem you found with -experimental, and with the subscriptions "ebbing out with time".

I kind of doubt it, but, hopefully I can fix all this before next Saturday cause next Sunday (July 2) I go to Athens for a European project meeting. Will be gone/busy all that week.

Anyway, just wanted to update you on all this.

@Jxlle
Copy link
Author

Jxlle commented Jun 23, 2023

@kzangeli

No problem, thanks for being so active!! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants