ToValue converting array into map[string]interface{}

[snej]

The latest Otto revisions break the Couchbase Sync Gateway -- it's more trouble decoding Otto values to Go string arrays. This time the problem is that ToValue returns not a Go []string but a map[string]interface{} where the keys are stringified array indexes, i.e. it looks like {"0":"foo", "1":"bar", "2":"baz"} instead of ["foo", "bar", "baz"].

func TestOttoValueToStringArray(t *testing.T) {
    env := otto.New()
    value, _ := env.ToValue([]string{"foo", "bar", "baz"})
    strings := ottoValueToStringArray(value)
    //assert.DeepEquals(t, strings, []string{"foo", "bar", "baz"})  // go.assert package
}

func ottoValueToStringArray(value otto.Value) []string {
    nativeValue, _ := value.Export()
    switch nativeValue := nativeValue.(type) {
    case string:
        return []string{nativeValue}
    case []interface{}:
        result := make([]string, 0, len(nativeValue))
        for _, item := range nativeValue {
            if str, ok := item.(string); ok {
                result = append(result, str)
            }
        }
        return result
    default:
        log.Printf("ottoValueToStringArray can't decode %#v", nativeValue)
    }
    return nil
}

This hits the Printf call, which logs:

ottoValueToStringArray can't decode map[string]interface {}{"0":"foo", "1":"bar", "2":"baz"}

Of course it's also possible that ToValue is converting the Go array into a JS object with integer keys; I know that in JS there's a very subtle distinction between arrays and objects.

[snej]

FYI, the current Otto revision I'm testing is 04ea4a2. The one we've been using that works fine is adf21d0 from 5/15 ("No rush to change the interface of Value.Export()"). I'm running Go 1.1 on Mac OS X 10.8.4.

[snej]

If I call value.Type() at the start of ottoValueToStringArray(), the result is "GoArray". So it looks like the problem is in converting a JS GoArray back into a native Go array.

[robertkrimen]

Sorry about that. The problem is (probably) around here:

https://github.com/robertkrimen/otto/blob/master/value.go#L625

This is the commit that "broke" it:

6669f98

We have different types for a Go slice versus a Go array, now.

I'm not sure exactly why the export tests did not catch this, though...

[snej]

Maybe line 628 should read

if object.class == "Array” || object.class == “GoArray” {

?

[snej]

Yeah, that fixed it.

[snej]

The bug isn't fixed. My test is still failing — it's that assertion that I commented out when pasting it above. The result of Export is now correctly a string array, but unfortunately its value is nil :(

    channelmapper_test.go:33
    assert.DeepEquals(t, strings, []string{"foo", "bar", "baz"})    

    expected: []string{"foo", "bar", "baz"}
         got: []string(nil)
--- FAIL: TestOttoValueToStringArray (0.08 seconds)

The unit test you added (Test_issue24) doesn't catch this because it only verifies the return type, not the actual value.

[snej]

Oops, never mind — my own code was broken (it was still expecting Value to return []interface{}, not []string!)