DMTF / libredfish

libRedfish is a C client library that allows for Creation of Entities (POST), Read of Entities (GET), Update of Entities (PATCH), Deletion of Entities (DELETE), running Actions (POST), receiving events, and providing some basic query abilities.
Other
48 stars 22 forks source link

Inconsistency of implementation in Redpath #160

Closed miaoxw closed 1 year ago

miaoxw commented 1 year ago

Recently I am making some exploration for further use of libredfish. I tried to call getPayloadByPath after reading documents and README page,

In such an environment that contains one member of System, the redPath /Systems returns the payload below:

{
    "@odata.context":"/redfish/v1/$metadata#ComputerSystemCollection.ComputerSystemCollection",
    "@odata.id":"/redfish/v1/Systems",
    "@odata.type":"#ComputerSystemCollection.ComputerSystemCollection",
    "Description":"Collection of Computer Systems",
    "Members":[
        {
            "@odata.id":"/redfish/v1/Systems/System.Embedded.1"
        }
    ],
    "Members@odata.count":1,
    "Name":"Computer System Collection"
}

But when I tried to access its only member, getPayloadByPath returns NULL for error when I passed the redPath /Systems/Members[1]. In an occasional attempt, I got the right result with /Systems/Members[0].

According to the freshly released Redpath Specification, index is 1-based instead of 0-based in conventional programming language (JSON in this issue which caused the problem). This is also specially noted in the example query at line 102:

* /Chassis[1]: Returns the first Chassis resource

Therefore I think implementation of redpath in the current version has conflicts with specification.

#iwork4dell

mraineri commented 1 year ago

We might want to consider making an errata to the Redpath spec; the spec was intended to be based off libredfish, and 0-based indexing into JSON arrays seems more natural. Will need to discuss this others.

dchanman commented 1 year ago

Errata works for Google, it turns out our current implementation was also 0-indexing.

miaoxw commented 1 year ago

Our Redpath Specification says Redpath is based on XPath and XPath is 1-based. However, keeping the library as-is is indeed an option too. Either implmementation makes sense. I think it is OK as long as lib and spec are consistent.

mraineri commented 1 year ago

We've published an updated version of the RedPath spec to clarify the indexing is 0-based: https://www.dmtf.org/sites/default/files/standards/documents/DSP0285_1.0.1.pdf

Selects the index number JSON entity from an array or object. For arrays, the index is 0-based.

I also see the README isn't consistent either; I'll make a fix for this.

mraineri commented 1 year ago

Just updated the README:

/Chassis[0] - Will return the first Chassis instance

@miaoxw if everything looks good at this point, I'd like to close the issue.

miaoxw commented 1 year ago

@mraineri Hi Mike, it looks good to me now. You are safe to close this issue :)