beam-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From al...@apache.org
Subject [beam] branch master updated: [BEAM-6652] Reinstate Maps- Validate json is deterministic.
Date Fri, 15 Feb 2019 22:20:59 GMT
This is an automated email from the ASF dual-hosted git repository.

altay pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git


The following commit(s) were added to refs/heads/master by this push:
     new b28856c  [BEAM-6652] Reinstate Maps- Validate json is deterministic.
     new 2a41235  Merge pull request #7857 from lostluck/fixtop
b28856c is described below

commit b28856c726c08e4ae6780c5f09f0f610552ae8fc
Author: Robert Burke <robert@frantil.com>
AuthorDate: Fri Feb 15 21:49:30 2019 +0000

    [BEAM-6652] Reinstate Maps- Validate json is deterministic.
---
 sdks/go/pkg/beam/coder.go                 |  7 ++++-
 sdks/go/pkg/beam/coder_test.go            | 43 ++++++++++++++++++++++++-------
 sdks/go/pkg/beam/core/typex/class.go      | 12 ++-------
 sdks/go/pkg/beam/core/typex/class_test.go |  4 +--
 4 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/sdks/go/pkg/beam/coder.go b/sdks/go/pkg/beam/coder.go
index e007b9d..62672e5 100644
--- a/sdks/go/pkg/beam/coder.go
+++ b/sdks/go/pkg/beam/coder.go
@@ -219,7 +219,12 @@ func inferCoders(list []FullType) ([]*coder.Coder, error) {
 
 // protoEnc marshals the supplied proto.Message.
 func protoEnc(in T) ([]byte, error) {
-	return proto.Marshal(in.(proto.Message))
+	buf := proto.NewBuffer(nil)
+	buf.SetDeterministic(true)
+	if err := buf.Marshal(in.(proto.Message)); err != nil {
+		return nil, err
+	}
+	return buf.Bytes(), nil
 }
 
 // protoDec unmarshals the supplied bytes into an instance of the supplied
diff --git a/sdks/go/pkg/beam/coder_test.go b/sdks/go/pkg/beam/coder_test.go
index 6bd3849..daecc97 100644
--- a/sdks/go/pkg/beam/coder_test.go
+++ b/sdks/go/pkg/beam/coder_test.go
@@ -16,27 +16,50 @@
 package beam
 
 import (
+	"reflect"
 	"testing"
-
-	"github.com/apache/beam/sdks/go/pkg/beam/core/util/reflectx"
 )
 
 func TestJSONCoder(t *testing.T) {
-	tests := []int{43, 12431235, -2, 0, 1}
+	v := "teststring"
+	tests := []interface{}{
+		43,
+		12431235,
+		-2,
+		0,
+		1,
+		true,
+		"a string",
+		map[int64]string{1: "one", 11: "oneone", 21: "twoone", 1211: "onetwooneone"},
+		struct {
+			A int
+			B *string
+			C bool
+		}{4, &v, false},
+	}
 
 	for _, test := range tests {
-		data, err := jsonEnc(test)
-		if err != nil {
-			t.Fatalf("Failed to encode %v: %v", tests, err)
+		var results []string
+		for i := 0; i < 10; i++ {
+			data, err := jsonEnc(test)
+			if err != nil {
+				t.Fatalf("Failed to encode %v: %v", tests, err)
+			}
+			results = append(results, string(data))
 		}
-		decoded, err := jsonDec(reflectx.Int, data)
+		for i, data := range results {
+			if data != results[0] {
+				t.Errorf("coder not deterministic: data[%d]: %v != %v ", i, data, results[0])
+			}
+		}
+
+		decoded, err := jsonDec(reflect.TypeOf(test), []byte(results[0]))
 		if err != nil {
 			t.Fatalf("Failed to decode: %v", err)
 		}
-		actual := decoded.(int)
 
-		if test != actual {
-			t.Errorf("Corrupt coding: %v, want %v", actual, test)
+		if !reflect.DeepEqual(decoded, test) {
+			t.Errorf("Corrupt coding: %v, want %v", decoded, test)
 		}
 	}
 }
diff --git a/sdks/go/pkg/beam/core/typex/class.go b/sdks/go/pkg/beam/core/typex/class.go
index 8d58368..cb34ad7 100644
--- a/sdks/go/pkg/beam/core/typex/class.go
+++ b/sdks/go/pkg/beam/core/typex/class.go
@@ -122,16 +122,8 @@ func isConcrete(t reflect.Type, visited map[uintptr]bool) bool {
 		return false // no unserializable types
 
 	case reflect.Map:
-		// TODO(BEAM-6652): 2019.02.11
-		// Maps have random default iteration order, and the "default coder"
-		// doesn't ensure a specific encoding order for key/value pairs in maps.
-		// To ensure correctness we're better off restricting map usage from
-		// casual users, until the default coder handles the random iteration
-		// order properly.
-		// Power users can continue to handle map coding themselves, though
-		// it remains their responsibility to encode them consistently.
-		// return isConcrete(t.Elem(), visited) && isConcrete(t.Key(), visited)
-		return false
+		return isConcrete(t.Elem(), visited) && isConcrete(t.Key(), visited)
+
 	case reflect.Array, reflect.Slice, reflect.Ptr:
 		return isConcrete(t.Elem(), visited)
 
diff --git a/sdks/go/pkg/beam/core/typex/class_test.go b/sdks/go/pkg/beam/core/typex/class_test.go
index be74254..1584dc6 100644
--- a/sdks/go/pkg/beam/core/typex/class_test.go
+++ b/sdks/go/pkg/beam/core/typex/class_test.go
@@ -51,7 +51,7 @@ func TestClassOf(t *testing.T) {
 		}{}), Concrete},
 		{reflect.TypeOf(struct{ A []int }{}), Concrete},
 		{reflect.TypeOf(reflect.Value{}), Concrete}, // ok: private fields
-		{reflect.TypeOf(map[string]int{}), Invalid},
+		{reflect.TypeOf(map[string]int{}), Concrete},
 		{reflect.TypeOf(map[string]func(){}), Invalid},
 		{reflect.TypeOf(map[error]int{}), Invalid},
 		{reflect.TypeOf([4]int{}), Concrete},
@@ -74,7 +74,7 @@ func TestClassOf(t *testing.T) {
 		{reflect.TypeOf(RecursivePtrTest{}), Concrete},
 		{reflect.TypeOf(RecursiveSliceTest{}), Concrete},
 		{reflect.TypeOf(RecursivePtrArrayTest{}), Concrete},
-		{reflect.TypeOf(RecursiveMapTest{}), Invalid},
+		{reflect.TypeOf(RecursiveMapTest{}), Concrete},
 		{reflect.TypeOf(RecursiveBadTest{}), Invalid},
 
 		{reflect.TypeOf([]X{}), Container},


Mime
View raw message