stmcginnis / gofish

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

inconsistent exporting of types #284

Closed jplimack closed 11 months ago

jplimack commented 11 months ago

for the most part the types are all publicly exported, but there are some exceptions where the struct fields are private due to collision with some funcs of the same name which are public. For example:

    // Description provides a description of this resource.
    Description string
    // Drives is a collection that indicates all the drives attached to the
    // storage controllers that this resource represents.
    drives []string //    <--- here drives is not exported *******************************
    // DrivesCount is the number of drives.
    DrivesCount int `json:"Drives@odata.count"`
    // Redundancy shall contain redundancy information for the storage subsystem.
    Redundancy []Redundancy

and later down we find

func (storage *Storage) Drives() ([]*Drive, error) {

when using the gofish package a a dependency, this breaks some functionality; specifically when reflecting through the data structure

panic: reflect.Value.Interface: cannot return value obtained from unexported field or method

To that end, please make the fields all publicly exported and rename the methods to something that does not conflict with the field names of the type. While i called out drives, that is far from the only place that this occurs

stmcginnis commented 11 months ago

Those fields are intentionally private. It is not to avoid conflicting with the later method names. The method names are named as they are precisely because of their relationship to those field.

In each of these, the fields themselves are not useful. They contain internal reference information like URIs for where the useful objects can be found. The methods defined are the way you would then access these objects.

Using gofish as a dependency is not broken by this. You just need to use the correct methods to access the data you need, rather than trying to reflect through all properties of an object. If you do need to reflect for some other purpose, your code needs to properly handle when there are private fields.

jplimack commented 11 months ago

yes, this is certainly possible, but precludes the use of existing diff packages that dont handle these exceptions well. Specifically I was trying to compare two redfish configs i have nested into a struct via https://github.com/r3labs/diff/, and Im trying to avoid needing to reimplement a custom differ

stmcginnis commented 11 months ago

Makes sense. You wouldn't want to include these private fields in that diff. For the most part, they are just references off to where to find other objects anyway. So what they lead to might be useful to diff, but the reference links themselves shouldn't be.