-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix(time): added nullable Time with better json support #44
Changes from all commits
2b5542e
328fae8
da40095
e412216
969829f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
package sqle | ||
|
||
import ( | ||
"database/sql" | ||
"database/sql/driver" | ||
"encoding/json" | ||
"time" | ||
) | ||
|
||
var nullTimeJsonBytes = []byte("null") | ||
|
||
const nullTimeJson = "null" | ||
|
||
// Time represents a nullable time value. | ||
type Time struct { | ||
sql.NullTime | ||
} | ||
|
||
// NewTime creates a new Time object with the given time and valid flag. | ||
func NewTime(t time.Time, valid bool) Time { | ||
return Time{NullTime: sql.NullTime{Time: t, Valid: valid}} | ||
} | ||
|
||
// Scan implements the [sql.Scanner] interface. | ||
func (t *Time) Scan(value any) error { // skipcq: GO-W1029 | ||
return t.NullTime.Scan(value) | ||
} | ||
|
||
// Value implements the [driver.Valuer] interface. | ||
func (t Time) Value() (driver.Value, error) { // skipcq: GO-W1029 | ||
return t.NullTime.Value() | ||
} | ||
|
||
// Time returns the underlying time.Time value of the Time struct. | ||
func (t *Time) Time() time.Time { // skipcq: GO-W1029 | ||
return t.NullTime.Time | ||
} | ||
|
||
// MarshalJSON implements the json.Marshaler interface | ||
func (t Time) MarshalJSON() ([]byte, error) { // skipcq: GO-W1029 | ||
if t.Valid { | ||
return json.Marshal(t.NullTime.Time) | ||
} | ||
return nullTimeJsonBytes, nil | ||
} | ||
|
||
// UnmarshalJSON implements the json.Unmarshaler interface | ||
func (t *Time) UnmarshalJSON(data []byte) error { // skipcq: GO-W1029 | ||
if len(data) == 0 || string(data) == nullTimeJson { | ||
t.NullTime.Time = time.Time{} | ||
t.NullTime.Valid = false | ||
return nil | ||
} | ||
|
||
var v time.Time | ||
err := json.Unmarshal(data, &v) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
t.NullTime.Time = v | ||
t.NullTime.Valid = true | ||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
package sqle | ||
|
||
import ( | ||
"database/sql" | ||
"encoding/json" | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestTimeInSQL(t *testing.T) { | ||
|
||
now := time.Now() | ||
d, err := sql.Open("sqlite3", "file::memory:") | ||
require.NoError(t, err) | ||
|
||
_, err = d.Exec("CREATE TABLE `times` (`id` id NOT NULL,`created_at` datetime, PRIMARY KEY (`id`))") | ||
require.NoError(t, err) | ||
|
||
result, err := d.Exec("INSERT INTO `times`(`id`) VALUES(?)", 10) | ||
require.NoError(t, err) | ||
|
||
rows, err := result.RowsAffected() | ||
require.NoError(t, err) | ||
require.Equal(t, int64(1), rows) | ||
|
||
result, err = d.Exec("INSERT INTO `times`(`id`, `created_at`) VALUES(?, ?)", 20, now) | ||
require.NoError(t, err) | ||
|
||
rows, err = result.RowsAffected() | ||
require.NoError(t, err) | ||
require.Equal(t, int64(1), rows) | ||
|
||
var t10 Time | ||
err = d.QueryRow("SELECT `created_at` FROM `times` WHERE id=?", 10).Scan(&t10) | ||
require.NoError(t, err) | ||
|
||
require.EqualValues(t, false, t10.Valid) | ||
|
||
var t20 Time | ||
err = d.QueryRow("SELECT `created_at` FROM `times` WHERE id=?", 20).Scan(&t20) | ||
require.NoError(t, err) | ||
|
||
require.EqualValues(t, true, t20.Valid) | ||
require.EqualValues(t, now.UTC(), t20.Time().UTC()) | ||
|
||
result, err = d.Exec("INSERT INTO `times`(`id`,`created_at`) VALUES(?, ?)", 11, t10) | ||
require.NoError(t, err) | ||
|
||
rows, err = result.RowsAffected() | ||
require.NoError(t, err) | ||
require.Equal(t, int64(1), rows) | ||
|
||
result, err = d.Exec("INSERT INTO `times`(`id`, `created_at`) VALUES(?, ?)", 21, t20) | ||
require.NoError(t, err) | ||
|
||
rows, err = result.RowsAffected() | ||
require.NoError(t, err) | ||
require.Equal(t, int64(1), rows) | ||
|
||
var t11 Time | ||
err = d.QueryRow("SELECT `created_at` FROM `times` WHERE id=?", 11).Scan(&t11) | ||
require.NoError(t, err) | ||
|
||
require.EqualValues(t, false, t11.Valid) | ||
|
||
var t21 Time | ||
err = d.QueryRow("SELECT `created_at` FROM `times` WHERE id=?", 21).Scan(&t21) | ||
require.NoError(t, err) | ||
|
||
require.EqualValues(t, true, t21.Valid) | ||
require.EqualValues(t, now.UTC(), t21.Time().UTC()) | ||
|
||
} | ||
Comment on lines
+12
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Consider adding tests for error conditions and edge cases. The current tests cover the basic functionality well, but there are no tests for error conditions such as database connection failures, SQL syntax errors, or invalid time values. Additionally, edge cases like leap seconds or timezone differences are not tested. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Add tests for JSON serialization and deserialization of null or invalid times. The tests for JSON handling are good for valid times, but they do not cover cases where the time is null or invalid. This is important to ensure robustness in handling different kinds of input. |
||
|
||
func TestTimeInJSON(t *testing.T) { | ||
|
||
sysTime := time.Now() | ||
|
||
bufSysTime, err := json.Marshal(sysTime) | ||
require.NoError(t, err) | ||
|
||
sqleTime := NewTime(sysTime, true) | ||
|
||
bufSqleTime, err := json.Marshal(sqleTime) | ||
require.NoError(t, err) | ||
|
||
require.Equal(t, bufSysTime, bufSqleTime) | ||
|
||
var jsSqleTime Time | ||
// Unmarshal sqle.Time from time.Time json bytes | ||
err = json.Unmarshal(bufSysTime, &jsSqleTime) | ||
require.NoError(t, err) | ||
|
||
require.True(t, sysTime.Equal(jsSqleTime.Time())) | ||
require.Equal(t, true, jsSqleTime.Valid) | ||
|
||
var jsSysTime time.Time | ||
// Unmarshal time.Time from sqle.Time json bytes | ||
err = json.Unmarshal(bufSqleTime, &jsSysTime) | ||
require.NoError(t, err) | ||
require.True(t, sysTime.Equal(jsSysTime)) | ||
|
||
var nullTime Time | ||
err = json.Unmarshal([]byte("null"), &nullTime) | ||
require.NoError(t, err) | ||
require.Equal(t, false, nullTime.Valid) | ||
|
||
bufNull, err := json.Marshal(nullTime) | ||
require.NoError(t, err) | ||
require.Equal(t, []byte("null"), bufNull) | ||
} |
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.
suggestion (code_refinement): Consider handling specific JSON unmarshal errors for better error reporting.
Handling specific JSON unmarshal errors could provide more context and help in debugging issues related to date format errors or other JSON related issues.