-
Notifications
You must be signed in to change notification settings - Fork 8k
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
refactor(json): Restore gin support for app engine #1026
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1026 +/- ##
=======================================
Coverage 96.61% 96.61%
=======================================
Files 16 16
Lines 1712 1712
=======================================
Hits 1654 1654
Misses 50 50
Partials 8 8
Continue to review full report at Codecov.
|
LGTM this is a good example of go build tags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@appleboy can you add a section at README, explaining shortly how to compile/build using the jsonite tag? same as in the body of this PR 😉 thanks!
@javierprovecho Done. Already update readme. |
I have objection to switch json engine to jsoniter since jsoniter doesn't have 100% compatibility to encoding/json. package main
import (
"fmt"
"log"
"github.com/json-iterator/go"
)
func test(s string) {
var err error
var v interface{}
err = jsoniter.Unmarshal([]byte(s), &v)
if err != nil {
log.Print(err)
return
}
fmt.Println(v)
}
func main() {
test("--2") // 2 [Why not error?]
test("002") // 2 [Why not error?]
test("00.2") // 0.2 [Why not error?]
test("1e1") // 10 [Why not float?]
test("1.0e1") // 10 [Why not float?]
} |
@mattn json-iterator/go#135 working on that |
@mattn all fixed |
Still not. package main
import (
"encoding/json"
"log"
"reflect"
"github.com/json-iterator/go"
)
func test(s string) {
var err1, err2 error
var v1, v2 interface{}
log.Println("testing:", s)
err1 = jsoniter.Unmarshal([]byte(s), &v1)
err2 = json.Unmarshal([]byte(s), &v2)
if err1 != nil && err2 == nil {
log.Printf("jsoniter return error but encoding/json not return error:", err1)
} else if err1 == nil && err2 != nil {
log.Printf("jsoniter not return error but encoding/json return error:", err2)
} else if !reflect.DeepEqual(v1, v2) {
log.Printf("there are different between jsoniter and encoding/json: %v, %v", v1, v2)
}
}
func main() {
test("-")
test("--")
test("0000000000000000000000000000000000000000000")
test("1*1")
test("1%")
test("1&")
}
|
@mattn have you updated the dependency? I can not reproduce the issues on latest master branch. |
json/json.go
Outdated
// Use of this source code is governed by a MIT style | ||
// license that can be found in the LICENSE file. | ||
|
||
//+build !jsoniter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing space?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
json/jsoniter.go
Outdated
// Use of this source code is governed by a MIT style | ||
// license that can be found in the LICENSE file. | ||
|
||
//+build jsoniter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing space?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
@taowen Yes, head/master. |
@mattn it has already returned error, I don not think the value difference is a big issue. User should check the error, and discard the value. |
Maybe, difference still exists. package main
import (
"encoding/json"
"log"
"reflect"
"github.com/json-iterator/go"
)
func test(s string) {
var err1, err2 error
var v1, v2 interface{}
log.Printf("testing: %q", s)
err1 = jsoniter.Unmarshal([]byte(s), &v1)
err2 = json.Unmarshal([]byte(s), &v2)
if err1 != nil && err2 == nil {
log.Printf("jsoniter return error but encoding/json not return error: %v", err1)
} else if err1 == nil && err2 != nil {
log.Printf("jsoniter not return error but encoding/json return error: %v", err2)
} else if err1 == nil && err2 == nil && !reflect.DeepEqual(v1, v2) {
log.Printf("there are different between jsoniter and encoding/json: %v, %v", v1, v2)
}
}
func main() {
test("10.")
}
// 2017/07/18 20:54:40 testing: "10."
// 2017/07/18 20:54:40 jsoniter not return error but encoding/json return error: invalid character ' ' after decimal point in numeric literal I will not use gin again unless it is a perfectly compatible with encoding/json. |
@mattn "10." fixed. Surprise to find out |
Create new folder to support multiple json package. restore gin support for app engine (disable jsonite through tags) use jsoniter $ go build -tags=jsoniter . use default json $ go build . Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
package main
import (
"encoding/json"
"log"
"reflect"
"github.com/json-iterator/go"
)
func test(s string) {
var err1, err2 error
var v1, v2 interface{}
log.Printf("testing: %q", s)
err1 = jsoniter.Unmarshal([]byte(s), &v1)
err2 = json.Unmarshal([]byte(s), &v2)
if err1 != nil && err2 == nil {
log.Printf("jsoniter return error but encoding/json not return error: %v", err1)
} else if err1 == nil && err2 != nil {
log.Printf("jsoniter not return error but encoding/json return error: %v", err2)
} else if err1 == nil && err2 == nil && !reflect.DeepEqual(v1, v2) {
log.Printf("there are different between jsoniter and encoding/json: %v, %v", v1, v2)
}
}
func main() {
test(`"\uD83D"`)
test(`"\U0001f64f"`)
}
// 2017/07/18 23:29:53 testing: "\"\\uD83D\""
// 2017/07/18 23:29:53 jsoniter return error but encoding/json not return error: ReadString: expects \u after utf16 surrogate, but \ not found, parsing 8 ..."\uD83D"... at "\uD83D"
// 2017/07/18 23:29:53 testing: "\"\\U0001f64f\""
// 2017/07/18 23:29:53 jsoniter not return error but encoding/json return error: invalid character 'U' in string escape code |
Also
|
@mattn surrogate issues, fixed |
|
@mattn |
jsoniter is not faster than encoding/json. |
@mattn please share your test code |
I don't write code anything. Just test |
@mattn Benchmark_xxx is not well maintained, I just updated that one json-iterator/go@12cd299 It was referencing to a non-existing file "/tmp/large-file.json" The new implementation of
|
I did point problem about JSON decoder. I'm not sure, but there are problems in encoder, probably. I don't have enough time to check them. |
@mattn thank you |
@mattn this pr will make jsonite optional, so like nothing happened between v1.2 and future v1.3. I encourage everyone to use the vendoring system for avoiding unstable changes made between the latest version and next version. |
In order to restore Gin support for app engine (disable jsonite through tags), create new folder to support multiple JSON packages.
use jsoniter
use default json
fix #1023
cc @javierprovecho @taowen @codedance