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

Serialization of platforms not returning expected value. #366

Closed
ravitejareddybejawada opened this issue Dec 19, 2023 · 1 comment · Fixed by microsoft/kiota#4131
Closed
Assignees
Labels
bug Something isn't working

Comments

@ravitejareddybejawada
Copy link

I am trying to create a Device Management Configuration Policy using Post api. In the process I am trying to set the platform to "windows10 and later" which enumerates to WINDOWS10_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS in the file /[email protected]/models/device_management_configuration_platforms.go

when I try to print the platform value, android,windows10X is printed instead of windows10.
image (3)

Upon investigation, we identified value for WINDOWS10_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS is set to 5 (const WINDOWS10_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS DeviceManagementConfigurationPlatforms = 5) instead of 32(2**5) which would facilitate the left shift algorithm used in String method.

I think similar changes are needed wherever the left shift algorithm is used.

@baywet
Copy link
Member

baywet commented Dec 19, 2023

Hi @ravitejareddybejawada

Thanks for bringing this up to our attention.

This

const (
    // Default. No platform type specified.
    NONE_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS DeviceManagementConfigurationPlatforms = iota
    // Settings for Android platform.
    ANDROID_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS
    // Settings for iOS platform.
    IOS_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS
    // Settings for MacOS platform.
    MACOS_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS
    // Windows 10 X.
    WINDOWS10X_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS
    // Settings for Windows 10 platform.
    WINDOWS10_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS
    // Settings for Linux platform.
    LINUX_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS
    // Evolvable enumeration sentinel value. Do not use.
    UNKNOWNFUTUREVALUE_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS
)

Should instead be generated like that

const (
    // Default. No platform type specified.
    NONE_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS DeviceManagementConfigurationPlatforms = 1
    // Settings for Android platform.
    ANDROID_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS = 2
    // Settings for iOS platform.
    IOS_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS = 4
    // Settings for MacOS platform.
    MACOS_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS = 8
    // Windows 10 X.
    WINDOWS10X_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS = 16
    // Settings for Windows 10 platform.
    WINDOWS10_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS = 32
    // Settings for Linux platform.
    LINUX_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS = 64
    // Evolvable enumeration sentinel value. Do not use.
    UNKNOWNFUTUREVALUE_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS = 128
)

And this

func (i DeviceManagementConfigurationPlatforms) String() string {
    var values []string
    for p := DeviceManagementConfigurationPlatforms(1); p <= UNKNOWNFUTUREVALUE_DEVICEMANAGEMENTCONFIGURATIONPLATFORMS; p <<= 1 {
        if i&p == p {
            values = append(values, []string{"none", "android", "iOS", "macOS", "windows10X", "windows10", "linux", "unknownFutureValue"}[p])
        }
    }
    return strings.Join(values, ",")
}

should instead be generated like that

func (i DeviceManagementConfigurationPlatforms) String() string {
    var values []string
    for p := 0; p < 6; p++ {
        mantis := math.Pow(2, p)
        if i&mantis == mantis {
            values = append(values, []string{"none", "android", "iOS", "macOS", "windows10X", "windows10", "linux", "unknownFutureValue"}[p])
        }
    }
    return strings.Join(values, ",")
}

Note: Ideally the array for the values would be declared outside of the loop so we don't pay an allocation cost on every iteration.

I'll create an issue in the generator shortly to cover this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants