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.

Sign up for GitHub

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

Jump to bottom

feat(datastore): support civil package types save #3202

Merged
merged 8 commits into from Dec 2, 2020
Merged

feat(datastore): support civil package types save #3202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3cec386
feat(datastore): support civil package types save
Nov 11, 2020
9b8e294
refactor
Nov 12, 2020
b1f8648
refactor
Nov 12, 2020
f8ba17f
split cases
Nov 13, 2020
b538f80
use UTC location
Nov 16, 2020
209fcc1
add loading civil types
Nov 17, 2020
9c1d2d0
fix the test data
Nov 17, 2020
bd86eb7
Merge branch 'master' into civil_support
crwilcox Dec 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 17 additions & 4 deletions datastore/load.go
View file
Open in desktop
Expand Up @@ -20,15 +20,19 @@ import (
"strings"
"time"

"cloud.google.com/go/civil"
"cloud.google.com/go/internal/fields"
pb "google.golang.org/genproto/googleapis/datastore/v1"
)

var (
typeOfByteSlice = reflect.TypeOf([]byte(nil))
typeOfTime = reflect.TypeOf(time.Time{})
typeOfGeoPoint = reflect.TypeOf(GeoPoint{})
typeOfKeyPtr = reflect.TypeOf(&Key{})
typeOfByteSlice = reflect.TypeOf([]byte(nil))
typeOfTime = reflect.TypeOf(time.Time{})
typeOfCivilDate = reflect.TypeOf(civil.Date{})
typeOfCivilDateTime = reflect.TypeOf(civil.DateTime{})
typeOfCivilTime = reflect.TypeOf(civil.Time{})
typeOfGeoPoint = reflect.TypeOf(GeoPoint{})
typeOfKeyPtr = reflect.TypeOf(&Key{})
)

// typeMismatchReason returns a string explaining why the property p could not
Expand Down Expand Up @@ -379,6 +383,15 @@ func setVal(v reflect.Value, p Property) (s string) {
return typeMismatchReason(p, v)
}
v.Set(reflect.ValueOf(x))
case typeOfCivilDate:
date := civil.DateOf(pValue.(time.Time))
v.Set(reflect.ValueOf(date))
case typeOfCivilDateTime:
dateTime := civil.DateTimeOf(pValue.(time.Time))
v.Set(reflect.ValueOf(dateTime))
case typeOfCivilTime:
timeVal := civil.TimeOf(pValue.(time.Time))
v.Set(reflect.ValueOf(timeVal))
Comment on lines +386 to +394
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @IlyaFaer for working on this!

I'm following up via #3089.

There's an issue loading civil dates and times on a machine when the timezone is set to anything other than UTC. Time properties are loaded in a machine's local location (see #916) and therefore need to be converted to UTC before using them to create civil values.

I think this is the adjustment that's needed:

case typeOfCivilDate:
	date := civil.DateOf(pValue.(time.Time).In(time.UTC))
	v.Set(reflect.ValueOf(date))
case typeOfCivilDateTime:
	dateTime := civil.DateTimeOf(pValue.(time.Time).In(time.UTC))
	v.Set(reflect.ValueOf(dateTime))
case typeOfCivilTime:
	timeVal := civil.TimeOf(pValue.(time.Time).In(time.UTC))
	v.Set(reflect.ValueOf(timeVal))

default:
ent, ok := pValue.(*Entity)
if !ok {
Expand Down
64 changes: 64 additions & 0 deletions datastore/load_test.go
View file
Open in desktop
Expand Up @@ -20,8 +20,10 @@ import (
"testing"
"time"

"cloud.google.com/go/civil"
"cloud.google.com/go/internal/testutil"
pb "google.golang.org/genproto/googleapis/datastore/v1"
"google.golang.org/protobuf/types/known/timestamppb"
)

type Simple struct {
Expand Down Expand Up @@ -484,6 +486,68 @@ func TestLoadToInterface(t *testing.T) {
dst: &withUntypedInterface{Field: 1e9},
want: &withUntypedInterface{Field: "Newly set"},
},
{
name: "struct with civil.Date",
src: &pb.Entity{
Key: keyToProto(testKey0),
Properties: map[string]*pb.Value{
"Date": {ValueType: &pb.Value_TimestampValue{TimestampValue: &timestamppb.Timestamp{Seconds: 1605474000}}},
},
},
dst: &struct{ Date civil.Date }{
Date: civil.Date{},
},
want: &struct{ Date civil.Date }{
Date: civil.Date{
Year: 2020,
Month: 11,
Day: 15,
},
},
},
{
name: "struct with civil.DateTime",
src: &pb.Entity{
Key: keyToProto(testKey0),
Properties: map[string]*pb.Value{
"DateTime": {ValueType: &pb.Value_TimestampValue{TimestampValue: &timestamppb.Timestamp{Seconds: 1605504600}}},
},
},
dst: &struct{ DateTime civil.DateTime }{
DateTime: civil.DateTime{},
},
want: &struct{ DateTime civil.DateTime }{
DateTime: civil.DateTime{
Date: civil.Date{
Year: 2020,
Month: 11,
Day: 16,
},
Time: civil.Time{
Hour: 5,
Minute: 30,
},
},
},
},
{
name: "struct with civil.Time",
src: &pb.Entity{
Key: keyToProto(testKey0),
Properties: map[string]*pb.Value{
"Time": {ValueType: &pb.Value_TimestampValue{TimestampValue: &timestamppb.Timestamp{Seconds: 1605504600}}},
},
},
dst: &struct{ Time civil.Time }{
Time: civil.Time{},
},
want: &struct{ Time civil.Time }{
Time: civil.Time{
Hour: 5,
Minute: 30,
},
},
},
}

for _, tc := range testCases {
Expand Down
23 changes: 23 additions & 0 deletions datastore/save.go
View file
Open in desktop
Expand Up @@ -21,6 +21,7 @@ import (
"time"
"unicode/utf8"

"cloud.google.com/go/civil"
timepb "github.com/golang/protobuf/ptypes/timestamp"
pb "google.golang.org/genproto/googleapis/datastore/v1"
llpb "google.golang.org/genproto/googleapis/type/latlng"
Expand Down Expand Up @@ -53,6 +54,28 @@ func reflectFieldSave(props *[]Property, p Property, name string, opts saveOpts,
switch x := v.Interface().(type) {
case *Key, time.Time, GeoPoint:
p.Value = x
case civil.Date:
p.Value = x.In(time.UTC)
*props = append(*props, p)
return nil
case civil.Time:
var format string
if x.Nanosecond == 0 {
format = "15:04:05"
} else {
format = "15:04:05.000000000"
}
val, err := time.Parse(format, x.String())
if err != nil {
return fmt.Errorf("datastore: error while parsing civil.Time: %v", err)
}
p.Value = val
*props = append(*props, p)
return nil
case civil.DateTime:
p.Value = x.In(time.UTC)
*props = append(*props, p)
return nil
default:
switch v.Kind() {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
Expand Down
82 changes: 82 additions & 0 deletions datastore/save_test.go
View file
Open in desktop
Expand Up @@ -15,9 +15,11 @@
package datastore

import (
"fmt"
"testing"
"time"

"cloud.google.com/go/civil"
"cloud.google.com/go/internal/testutil"
pb "google.golang.org/genproto/googleapis/datastore/v1"
)
Expand Down Expand Up @@ -321,6 +323,27 @@ func TestSaveFieldsWithInterface(t *testing.T) {
N2 interface{}
}

civDateVal := civil.Date{
Year: 2020,
Month: 11,
Day: 10,
}
civTimeValNano := civil.Time{
Hour: 1,
Minute: 1,
Second: 1,
Nanosecond: 1,
}
civTimeVal := civil.Time{
Hour: 1,
Minute: 1,
Second: 1,
}
timeValNano, _ := time.Parse("15:04:05.000000000", civTimeValNano.String())
timeVal, _ := time.Parse("15:04:05", civTimeVal.String())
dateTimeStr := fmt.Sprintf("%v %v", civDateVal.String(), civTimeVal.String())
dateTimeVal, _ := time.ParseInLocation("2006-01-02 15:04:05", dateTimeStr, time.UTC)

cases := []struct {
name string
in interface{}
Expand Down Expand Up @@ -371,6 +394,65 @@ func TestSaveFieldsWithInterface(t *testing.T) {
{Name: "Map", Value: []Property{}},
},
},
{
name: "civil.Date",
in: &struct {
CivDate civil.Date
}{
CivDate: civDateVal,
},
want: []Property{
{
Name: "CivDate",
Value: civDateVal.In(time.UTC),
},
},
},
{
name: "civil.Time-nano",
in: &struct {
CivTimeNano civil.Time
}{
CivTimeNano: civTimeValNano,
},
want: []Property{
{
Name: "CivTimeNano",
Value: timeValNano,
},
},
},
{
name: "civil.Time",
in: &struct {
CivTime civil.Time
}{
CivTime: civTimeVal,
},
want: []Property{
{
Name: "CivTime",
Value: timeVal,
},
},
},
{
name: "civil.DateTime",
in: &struct {
CivDateTime civil.DateTime
}{
CivDateTime: civil.DateTime{
Date: civDateVal,
Time: civTimeVal,
},
},
want: []Property{
{
Name: "CivDateTime",
Value: dateTimeVal,
},
},
},
IlyaFaer marked this conversation as resolved.
Show resolved Hide resolved
{
name: "Nested",
in: &n3{
Expand Down