search

aquarat / google-api-go-client

Automatically exported from code.google.com/p/google-api-go-client
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

fields with "format": "date-time" should use time.Time, not string #20

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
The Go json marshaller will nicely convert time.Time to/from RFC3339 strings. 
Discovery already tells us which string fields need to be interpreted as 
RFC3339, so the generator should take care of that for us.

Original issue reported on code.google.com by ago...@google.com on 30 Nov 2012 at 4:49

GoogleCodeExporter commented 9 years ago 
I have the obvious patch for this. But it will break the API of this client. Is 
that ok?

Original comment by ago...@google.com on 30 Nov 2012 at 5:37

GoogleCodeExporter commented 9 years ago 
# HG changeset patch
# User Adam Goode <agoode@google.com>
# Date 1354684256 18000
# Node ID 4e26f8e050b6aa8abc9a417c1aac886670412cb5
# Parent  2253ff9f7a543f13da7119aaba26932f443d613d
Use *time.Time for fields instead of plain string when format is date or 
date-time

diff -r 2253ff9f7a54 -r 4e26f8e050b6 examples/tasks.go
--- a/examples/tasks.go Mon Nov 26 17:35:03 2012 -0800
+++ b/examples/tasks.go Wed Dec 05 00:10:56 2012 -0500
@@ -3,6 +3,7 @@
 import (
    "log"
    "net/http"
+   "time"

    tasks "code.google.com/p/google-api-go-client/tasks/v1"
 )
@@ -13,10 +14,11 @@

 func tasksMain(client *http.Client, argv []string) {
    taskapi, _ := tasks.New(client)
+   due := time.Date(2011, time.October, 15, 12, 00, 00, 000, time.UTC)
    task, err := taskapi.Tasks.Insert("@default", &tasks.Task{
        Title: "finish this API code generator thing",
        Notes: "ummmm",
-       Due:   "2011-10-15T12:00:00.000Z",
+       Due:   &due,
    }).Do()
    log.Printf("Got task, err: %#v, %v", task, err)
 }
diff -r 2253ff9f7a54 -r 4e26f8e050b6 google-api-go-generator/gen.go
--- a/google-api-go-generator/gen.go    Mon Nov 26 17:35:03 2012 -0800
+++ b/google-api-go-generator/gen.go    Wed Dec 05 00:10:56 2012 -0500
@@ -26,7 +26,7 @@
 )

 // goGenVersion is the version of the Go code generator
-const goGenVersion = "0.5"
+const goGenVersion = "0.6"

 var (
    apiToGenerate = flag.String("api", "*", "The API ID to generate, like 'tasks:v1'. A value of '*' means all.")
@@ -361,6 +361,7 @@
        "net/url",
        "strconv",
        "strings",
+       "time",
    } {
        p("\t%q\n", pkg)
    }
@@ -373,6 +374,7 @@
    pn("var _ = url.Parse")
    pn("var _ = googleapi.Version")
    pn("var _ = errors.New")
+   pn("var _ = time.Now")
    pn("")
    pn("const apiId = %q", jstr(m, "id"))
    pn("const apiName = %q", jstr(m, "name"))
@@ -1317,6 +1319,8 @@
        switch format {
        case "int64", "uint64", "int32", "uint32":
            gotype = format
+       case "date", "date-time":
+           gotype = "*time.Time"
        }
    case "number":
        gotype = "float64"

Original comment by ago...@google.com on 5 Dec 2012 at 5:12

GoogleCodeExporter commented 9 years ago 
Actually, this is invalid, since it seems like Google will generate and accept 
non-RFC3339 dates (outside of the year range 0000-9999). So a custom marshaller 
will be required.

Original comment by ago...@google.com on 11 Dec 2012 at 5:04

GoogleCodeExporter commented 9 years ago 
Perhaps the correct solution is to fix Go: 
https://code.google.com/p/go/issues/detail?id=4556

Original comment by ago...@google.com on 16 Dec 2012 at 4:44

GoogleCodeExporter commented 9 years ago 
I have a solution that adds googleapi.Time.

Original comment by ago...@google.com on 18 Dec 2012 at 5:22

GoogleCodeExporter commented 9 years ago 
*time.Time is weird.  time.Time would be the proper type.

But I'm concerned about changing this... it would break all existing callers.  
I don't know how many that is.

Original comment by bradfitz@google.com on 18 Dec 2012 at 6:36

GoogleCodeExporter commented 9 years ago 
*time.Time is weird, but omitIfEmpty doesn't work on plain time.Time.

Original comment by ago...@google.com on 18 Dec 2012 at 6:40

GoogleCodeExporter commented 9 years ago 
Sorry, I meant "omitempty", as in `json:"modifiedTime,omitempty"`

Original comment by ago...@google.com on 18 Dec 2012 at 6:41

GoogleCodeExporter commented 9 years ago 
The other solution is to leave at string, so that omitempty works, and callers 
don't have to change, but provide parse/unparse in googleapi for the particular 
time format that Google APIs require (both in sending and receiving).

package googleapi
func ParseTime(s string) (time.Time, err)
func FormatTime(t time.Time) err

Original comment by ago...@google.com on 19 Dec 2012 at 2:35

GoogleCodeExporter commented 9 years ago 
oops I mean
FormatTime(t time.Time) string

Original comment by ago...@google.com on 19 Dec 2012 at 2:35