stmcginnis / gofish

Gofish is a Golang client library for DMTF Redfish and SNIA Swordfish interaction.
BSD 3-Clause "New" or "Revised" License
204 stars 109 forks source link

ListReferencedX: return ordered results. #347

Closed greyerof closed 1 month ago

greyerof commented 1 month ago

Due to the parallelization of the collection's links retrieval, the returned list of the ListReferencedX funcs cannot be trusted to be ordered. This commit ensures that the returned list keeps the same order as the collection's members list.

Update common.GetCollection() to return the list of the collection's links in the same order they were retrieved.

Then, ListReferencedX funcs use the original link list to reorder the results before returning them. This proposal uses a helper map to store the unordered resultsDue to the parallelization of the collection's links retrieval, the returned list of the ListReferencedX funcs cannot be trusted to be ordered. This commit ensures that the returned list keeps the same order as the collection's members list..

I've updated all affected packages plus the template in the tools folder.

greyerof commented 1 month ago

Sorry @stmcginnis , I should've created an issue instead... So you mean that when asking for the ComputerSystems collection, for instance, the Members array field in the response could have a different order every time you ask for it even when no computer systems have been added/removed? Same for the entries of the LogService: Isn't the order of those entries guaranteed either?

It just seemed a bit weird to me that two consecutive calls to service.Systems() would return slices with different orders for the same response from the redfish server... it feels like adding more entropy to the system, to be honest.

stmcginnis commented 1 month ago

There's nothing in the spec that guarantees this order. Most implementations probably are fairly static, but I've actually seen some changes after upgrading BMC firmware. So it's really not a good assumption to expect a deterministic order every time.

If it's useful for the code using this result, it may make more sense for the sorting to be done there to make sure it's consistent between calls and between versions. So you could do something like:

systems, err := service.Systems()
if err != nil {
    panic(err)
}

sort.Slice(systems, func(i, j int) bool {
    return systems[i].ODataID < systems[j].ODataID
})

// Now we can assume we will process the systems in the same order every time
for _, system := range systems {
    // Do something sequential
}

So I'm just a little hesitant having gofish reinforce the assumption that things will always be in a certain order if that's not a guaranteed assumption.

greyerof commented 1 month ago

Thanks for the code snippet & your quick responses. I'll close this PR, then.

stmcginnis commented 1 month ago

No problem - thanks for the time you put in to this and thinking about ways to make it better!