Fix edit_note data (#108)

* Lock in dep

* Log v2 edit data when editing

* Migrate edit_note actions with incorrect data
This commit is contained in:
Sung Won Cho 2018-09-08 15:52:57 +10:00 committed by GitHub
commit 0a2413070f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 472 additions and 47 deletions

View file

@ -40,3 +40,7 @@
[[constraint]]
name = "github.com/satori/go.uuid"
version = "1.1.0"
[[constraint]]
name = "github.com/dnote/actions"
version = "0.1.0"

View file

@ -29,42 +29,17 @@ func Execute() error {
func Prepare(ctx infra.DnoteCtx) error {
err := core.MigrateToDnoteDir(ctx)
if err != nil {
return errors.Wrap(err, "Failed to initialize dnote dir")
return errors.Wrap(err, "initializing dnote dir")
}
fresh, err := core.IsFreshInstall(ctx)
err = core.InitFiles(ctx)
if err != nil {
return errors.Wrap(err, "Failed to check if fresh install")
}
err = core.InitDnoteDir(ctx)
if err != nil {
return errors.Wrap(err, "Failed to create dnote dir")
}
err = core.InitConfigFile(ctx)
if err != nil {
return errors.Wrap(err, "Failed to generate config file")
}
err = core.InitDnoteFile(ctx)
if err != nil {
return errors.Wrap(err, "Failed to create dnote file")
}
err = core.InitTimestampFile(ctx)
if err != nil {
return errors.Wrap(err, "Failed to create dnote upgrade file")
}
err = core.InitActionFile(ctx)
if err != nil {
return errors.Wrap(err, "Failed to create action file")
}
err = migrate.InitSchemaFile(ctx, fresh)
if err != nil {
return errors.Wrap(err, "Failed to create migration file")
return errors.Wrap(err, "initiating files")
}
err = migrate.Migrate(ctx)
if err != nil {
return errors.Wrap(err, "Failed to perform migration")
return errors.Wrap(err, "running migration")
}
return nil

View file

@ -62,11 +62,12 @@ func LogActionRemoveNote(ctx infra.DnoteCtx, noteUUID, bookName string) error {
}
func LogActionEditNote(ctx infra.DnoteCtx, noteUUID, bookName, content string, ts int64) error {
b, err := json.Marshal(actions.EditNoteDataV1{
b, err := json.Marshal(actions.EditNoteDataV2{
NoteUUID: noteUUID,
FromBook: bookName,
Content: content,
Content: &content,
})
if err != nil {
return errors.Wrap(err, "Failed to marshal data into JSON")
}

51
core/action_test.go Normal file
View file

@ -0,0 +1,51 @@
package core
import (
"encoding/json"
"testing"
"github.com/dnote/actions"
"github.com/dnote/cli/testutils"
"github.com/pkg/errors"
)
func TestLogActionEditNote(t *testing.T) {
// Setup
ctx := testutils.InitCtx("../tmp")
testutils.SetupTmp(ctx)
defer testutils.ClearTmp(ctx)
testutils.WriteFile(ctx, "../testutils/fixtures/dnote3.json", "dnote")
InitFiles(ctx)
if err := LogActionEditNote(ctx, "f0d0fbb7-31ff-45ae-9f0f-4e429c0c797f", "js", "updated content", 1536168581); err != nil {
t.Fatalf("Failed to perform %s", err.Error())
}
b := testutils.ReadFile(ctx, "actions")
var got []actions.Action
if err := json.Unmarshal(b, &got); err != nil {
panic(errors.Wrap(err, "unmarshalling actions"))
}
var actionData actions.EditNoteDataV2
if err := json.Unmarshal(got[0].Data, &actionData); err != nil {
panic(errors.Wrap(err, "unmarshalling action data"))
}
testutils.AssertEqual(t, len(got), 1, "action length mismatch")
testutils.AssertNotEqual(t, got[0].UUID, "", "action uuid mismatch")
testutils.AssertEqual(t, got[0].Schema, 2, "action schema mismatch")
testutils.AssertEqual(t, got[0].Type, actions.ActionEditNote, "action type mismatch")
testutils.AssertNotEqual(t, got[0].Timestamp, 0, "action timestamp mismatch")
testutils.AssertEqual(t, actionData.NoteUUID, "f0d0fbb7-31ff-45ae-9f0f-4e429c0c797f", "action data note_uuid mismatch")
testutils.AssertEqual(t, actionData.FromBook, "js", "action data from_book mismatch")
testutils.AssertEqual(t, *actionData.Content, "updated content", "action data content mismatch")
if actionData.ToBook != nil {
t.Errorf("action data to_book mismatch. Expected %+v. Got %+v", nil, actionData.ToBook)
}
if actionData.Public != nil {
t.Errorf("action data public mismatch. Expected %+v. Got %+v", nil, actionData.ToBook)
}
}

View file

@ -11,6 +11,7 @@ import (
"github.com/dnote/actions"
"github.com/dnote/cli/infra"
"github.com/dnote/cli/migrate"
"github.com/dnote/cli/utils"
"github.com/pkg/errors"
"github.com/spf13/cobra"
@ -57,8 +58,8 @@ func GetDnoteTmpContentPath(ctx infra.DnoteCtx) string {
return fmt.Sprintf("%s/%s", ctx.DnoteDir, TmpContentFilename)
}
// InitActionFile populates action file if it does not exist
func InitActionFile(ctx infra.DnoteCtx) error {
// initActionFile populates action file if it does not exist
func initActionFile(ctx infra.DnoteCtx) error {
path := GetActionPath(ctx)
if utils.FileExists(path) {
@ -97,8 +98,43 @@ func getEditorCommand() string {
}
}
// InitConfigFile populates a new config file if it does not exist yet
func InitConfigFile(ctx infra.DnoteCtx) error {
// InitFiles creates, if necessary, the dnote directory and files inside
func InitFiles(ctx infra.DnoteCtx) error {
fresh, err := isFreshInstall(ctx)
if err != nil {
return errors.Wrap(err, "Failed to check if fresh install")
}
err = initDnoteDir(ctx)
if err != nil {
return errors.Wrap(err, "Failed to create dnote dir")
}
err = initConfigFile(ctx)
if err != nil {
return errors.Wrap(err, "Failed to generate config file")
}
err = initDnoteFile(ctx)
if err != nil {
return errors.Wrap(err, "Failed to create dnote file")
}
err = initTimestampFile(ctx)
if err != nil {
return errors.Wrap(err, "Failed to create dnote upgrade file")
}
err = initActionFile(ctx)
if err != nil {
return errors.Wrap(err, "Failed to create action file")
}
err = migrate.InitSchemaFile(ctx, fresh)
if err != nil {
return errors.Wrap(err, "Failed to create migration file")
}
return nil
}
// initConfigFile populates a new config file if it does not exist yet
func initConfigFile(ctx infra.DnoteCtx) error {
path := GetConfigPath(ctx)
if utils.FileExists(path) {
@ -124,8 +160,8 @@ func InitConfigFile(ctx infra.DnoteCtx) error {
return nil
}
// InitDnoteDir initializes dnote directory if it does not exist yet
func InitDnoteDir(ctx infra.DnoteCtx) error {
// initDnoteDir initializes dnote directory if it does not exist yet
func initDnoteDir(ctx infra.DnoteCtx) error {
path := ctx.DnoteDir
if utils.FileExists(path) {
@ -139,8 +175,8 @@ func InitDnoteDir(ctx infra.DnoteCtx) error {
return nil
}
// InitDnoteFile creates an empty dnote file
func InitDnoteFile(ctx infra.DnoteCtx) error {
// initDnoteFile creates an empty dnote file
func initDnoteFile(ctx infra.DnoteCtx) error {
path := GetDnotePath(ctx)
if utils.FileExists(path) {
@ -156,8 +192,8 @@ func InitDnoteFile(ctx infra.DnoteCtx) error {
return err
}
// InitTimestampFile creates an empty dnote upgrade file
func InitTimestampFile(ctx infra.DnoteCtx) error {
// initTimestampFile creates an empty dnote upgrade file
func initTimestampFile(ctx infra.DnoteCtx) error {
path := GetTimestampPath(ctx)
if utils.FileExists(path) {
@ -459,8 +495,8 @@ func MigrateToDnoteDir(ctx infra.DnoteCtx) error {
return nil
}
// IsFreshInstall checks if the dnote files have been initialized
func IsFreshInstall(ctx infra.DnoteCtx) (bool, error) {
// isFreshInstall checks if the dnote files have been initialized
func isFreshInstall(ctx infra.DnoteCtx) (bool, error) {
path := ctx.DnoteDir
_, err := os.Stat(path)

View file

@ -138,7 +138,7 @@ func TestReduceRemoveNote(t *testing.T) {
testutils.AssertEqual(t, otherBook.Notes[0].Content, "wc -l to count words", "other book remaining note content mismatch")
}
func TestReduceEditNote(t *testing.T) {
func TestReduceEditNote_V1_Content(t *testing.T) {
// Setup
ctx := testutils.InitCtx("../tmp")
@ -184,7 +184,7 @@ func TestReduceEditNote(t *testing.T) {
testutils.AssertEqual(t, otherBook.Notes[0].Content, "wc -l to count words", "other book remaining note content mismatch")
}
func TestReduceEditNote_changeBook(t *testing.T) {
func TestReduceEditNote_V1_ChangeBook(t *testing.T) {
// Setup
ctx := testutils.InitCtx("../tmp")

View file

@ -208,7 +208,7 @@ func TestEdit_ContentFlag(t *testing.T) {
book := dnote["js"]
action := actionSlice[0]
var actionData actions.EditNoteDataV1
var actionData actions.EditNoteDataV2
err = json.Unmarshal(action.Data, &actionData)
if err != nil {
log.Fatalf("Failed to unmarshal the action data: %s", err)
@ -216,8 +216,12 @@ func TestEdit_ContentFlag(t *testing.T) {
testutils.AssertEqual(t, len(actionSlice), 1, "There should be 1 action")
testutils.AssertEqual(t, action.Type, actions.ActionEditNote, "action type mismatch")
testutils.AssertEqual(t, actionData.Content, "foo bar", "action data name mismatch")
testutils.AssertEqual(t, action.Schema, 2, "action schema mismatch")
testutils.AssertEqual(t, *actionData.Content, "foo bar", "action data name mismatch")
testutils.AssertEqual(t, actionData.FromBook, "js", "action data from_book mismatch")
if actionData.ToBook != nil {
t.Errorf("action data to_book mismatch. Expected %+v. Got %+v", nil, actionData.ToBook)
}
testutils.AssertEqual(t, actionData.NoteUUID, "f0d0fbb7-31ff-45ae-9f0f-4e429c0c797f", "action data note_uuis mismatch")
testutils.AssertNotEqual(t, action.Timestamp, 0, "action timestamp mismatch")
testutils.AssertEqual(t, len(book.Notes), 2, "Book should have one note")

View file

@ -0,0 +1,100 @@
[
{
"uuid": "6db32903-34de-489e-b0f6-7d6de28d4dae",
"schema": 1,
"type": "add_book",
"data": { "book_name": "js" },
"timestamp": 1536371120
},
{
"uuid": "d05db626-262c-4dc6-98e0-a53c8e823d32",
"schema": 2,
"type": "add_note",
"data": {
"note_uuid": "bd287e73-d4a1-4bd7-86a3-fe25f2f32c7d",
"book_name": "js",
"content": "hello world",
"public": false
},
"timestamp": 1536371120
},
{
"uuid": "89c49736-eab0-49a5-9a4c-5a607a100d89",
"schema": 1,
"type": "add_book",
"data": { "book_name": "linux" },
"timestamp": 1536371129
},
{
"uuid": "2e0b213c-0ebb-4253-ab1a-c989007b1718",
"schema": 2,
"type": "add_note",
"data": {
"note_uuid": "8c5a9f78-77a2-4793-a099-dabe233aa814",
"book_name": "linux",
"content": "hello linux",
"public": false
},
"timestamp": 1536371129
},
{
"uuid": "e4c8d8db-56a7-43e2-9085-b1b52d9aa8d7",
"schema": 2,
"type": "add_note",
"data": {
"note_uuid": "adb9b7ea-94da-4165-9333-dcf41daa132b",
"book_name": "js",
"content": "hello js",
"public": false
},
"timestamp": 1536371137
},
{
"uuid": "9b6bada1-df42-4cbb-b076-5219bb3cd3ba",
"schema": 2,
"type": "add_note",
"data": {
"note_uuid": "30e5a702-45aa-4138-9f41-8175691d7474",
"book_name": "js",
"content": "hello js2",
"public": false
},
"timestamp": 1536371139
},
{
"uuid": "3c364f1d-5da0-4beb-9cd9-6388cb7e55e9",
"schema": 2,
"type": "edit_note",
"data": {
"note_uuid": "adb9b7ea-94da-4165-9333-dcf41daa132b",
"from_book": "js",
"to_book": null,
"content": "hello edited world",
"public": null
},
"timestamp": 1536371154
},
{
"uuid": "553f9cda-b3ee-4c7f-b11a-f7eaf4d45048",
"schema": 2,
"type": "edit_note",
"data": {
"note_uuid": "8c5a9f78-77a2-4793-a099-dabe233aa814",
"from_book": "linux",
"to_book": null,
"content": "hello edited linux",
"public": null
},
"timestamp": 1536371169
},
{
"uuid": "b9980b72-eaa9-4d02-a31b-c580234a9452",
"schema": 1,
"type": "remove_note",
"data": {
"note_uuid": "30e5a702-45aa-4138-9f41-8175691d7474",
"book_name": "js"
},
"timestamp": 1536371181
}
]

View file

@ -0,0 +1,98 @@
[
{
"uuid": "6db32903-34de-489e-b0f6-7d6de28d4dae",
"schema": 1,
"type": "add_book",
"data": { "book_name": "js" },
"timestamp": 1536371120
},
{
"uuid": "d05db626-262c-4dc6-98e0-a53c8e823d32",
"schema": 2,
"type": "add_note",
"data": {
"note_uuid": "bd287e73-d4a1-4bd7-86a3-fe25f2f32c7d",
"book_name": "js",
"content": "hello world",
"public": false
},
"timestamp": 1536371120
},
{
"uuid": "89c49736-eab0-49a5-9a4c-5a607a100d89",
"schema": 1,
"type": "add_book",
"data": { "book_name": "linux" },
"timestamp": 1536371129
},
{
"uuid": "2e0b213c-0ebb-4253-ab1a-c989007b1718",
"schema": 2,
"type": "add_note",
"data": {
"note_uuid": "8c5a9f78-77a2-4793-a099-dabe233aa814",
"book_name": "linux",
"content": "hello linux",
"public": false
},
"timestamp": 1536371129
},
{
"uuid": "e4c8d8db-56a7-43e2-9085-b1b52d9aa8d7",
"schema": 2,
"type": "add_note",
"data": {
"note_uuid": "adb9b7ea-94da-4165-9333-dcf41daa132b",
"book_name": "js",
"content": "hello js",
"public": false
},
"timestamp": 1536371137
},
{
"uuid": "9b6bada1-df42-4cbb-b076-5219bb3cd3ba",
"schema": 2,
"type": "add_note",
"data": {
"note_uuid": "30e5a702-45aa-4138-9f41-8175691d7474",
"book_name": "js",
"content": "hello js2",
"public": false
},
"timestamp": 1536371139
},
{
"uuid": "3c364f1d-5da0-4beb-9cd9-6388cb7e55e9",
"schema": 2,
"type": "edit_note",
"data": {
"note_uuid": "adb9b7ea-94da-4165-9333-dcf41daa132b",
"from_book": "js",
"to_book": "",
"content": "hello edited world"
},
"timestamp": 1536371154
},
{
"uuid": "553f9cda-b3ee-4c7f-b11a-f7eaf4d45048",
"schema": 2,
"type": "edit_note",
"data": {
"note_uuid": "8c5a9f78-77a2-4793-a099-dabe233aa814",
"from_book": "linux",
"to_book": "",
"content": "hello edited linux"
},
"timestamp": 1536371169
},
{
"uuid": "b9980b72-eaa9-4d02-a31b-c580234a9452",
"schema": 1,
"type": "remove_note",
"data": {
"note_uuid": "30e5a702-45aa-4138-9f41-8175691d7474",
"book_name": "js"
},
"timestamp": 1536371181
}
]

View file

@ -27,6 +27,7 @@ const (
migrationV4
migrationV5
migrationV6
migrationV7
)
var migrationSequence = []int{
@ -36,6 +37,7 @@ var migrationSequence = []int{
migrationV4,
migrationV5,
migrationV6,
migrationV7,
}
type schema struct {
@ -93,6 +95,8 @@ func performMigration(ctx infra.DnoteCtx, migrationID int) error {
migrationError = migrateToV5(ctx)
case migrationV6:
migrationError = migrateToV6(ctx)
case migrationV7:
migrationError = migrateToV7(ctx)
default:
return errors.Errorf("Unrecognized migration id %d", migrationID)
}

View file

@ -324,3 +324,39 @@ func TestMigrateToV6(t *testing.T) {
t.Errorf("Payload does not match.\nActual: %+v\nExpected: %+v", got, expected)
}
}
func TestMigrateToV7(t *testing.T) {
ctx := testutils.InitCtx("../tmp")
// set up
testutils.SetupTmp(ctx)
testutils.WriteFile(ctx, "./fixtures/7-pre-actions.json", "actions")
defer testutils.ClearTmp(ctx)
// execute
if err := migrateToV7(ctx); err != nil {
t.Fatal(errors.Wrap(err, "migrating").Error())
}
// test
b := testutils.ReadFile(ctx, "actions")
var got []migrateToV7Action
if err := json.Unmarshal(b, &got); err != nil {
t.Fatal(errors.Wrap(err, "unmarshalling the result").Error())
}
b2 := testutils.ReadFileAbs("./fixtures/7-post-actions.json")
var expected []migrateToV7Action
if err := json.Unmarshal(b, &expected); err != nil {
t.Fatal(errors.Wrap(err, "unmarshalling the result into Dnote").Error())
}
ok, err := testutils.IsEqualJSON(b, b2)
if err != nil {
t.Fatal(errors.Wrap(err, "comparing JSON").Error())
}
if !ok {
t.Errorf("Result does not match.\nActual: %+v\nExpected: %+v", got, expected)
}
}

View file

@ -315,3 +315,69 @@ func migrateToV6(ctx infra.DnoteCtx) error {
return nil
}
// migrateToV7 migrates data of edit_note action to the proper version which is
// EditNoteDataV2. Due to a bug, edit logged actions with schema version '2'
// but with a data of EditNoteDataV1. https://github.com/dnote/cli/issues/107
func migrateToV7(ctx infra.DnoteCtx) error {
actionPath := fmt.Sprintf("%s/actions", ctx.DnoteDir)
b, err := ioutil.ReadFile(actionPath)
if err != nil {
return errors.Wrap(err, "reading actions file")
}
var preActions []migrateToV7Action
postActions := []migrateToV7Action{}
err = json.Unmarshal(b, &preActions)
if err != nil {
return errors.Wrap(err, "unmarhsalling existing actions")
}
for _, action := range preActions {
var newAction migrateToV7Action
if action.Type == migrateToV7ActionTypeEditNote {
var oldData migrateToV7EditNoteDataV1
if e := json.Unmarshal(action.Data, &oldData); e != nil {
return errors.Wrapf(e, "unmarshalling data of action with uuid %s", action.Data)
}
newData := migrateToV7EditNoteDataV2{
NoteUUID: oldData.NoteUUID,
FromBook: oldData.FromBook,
ToBook: nil,
Content: &oldData.Content,
Public: nil,
}
d, e := json.Marshal(newData)
if e != nil {
return errors.Wrapf(e, "marshalling new data of action with uuid %s", action.Data)
}
newAction = migrateToV7Action{
UUID: action.UUID,
Schema: action.Schema,
Type: action.Type,
Timestamp: action.Timestamp,
Data: d,
}
} else {
newAction = action
}
postActions = append(postActions, newAction)
}
d, err := json.Marshal(postActions)
if err != nil {
return errors.Wrap(err, "marshalling new actions")
}
err = ioutil.WriteFile(actionPath, d, 0644)
if err != nil {
return errors.Wrap(err, "writing new actions to a file")
}
return nil
}

View file

@ -130,3 +130,27 @@ type migrateToV6PostBook struct {
}
type migrateToV6PreDnote map[string]migrateToV6PreBook
type migrateToV6PostDnote map[string]migrateToV6PostBook
// v7
var migrateToV7ActionTypeEditNote = "edit_note"
type migrateToV7Action struct {
UUID string `json:"uuid"`
Schema int `json:"schema"`
Type string `json:"type"`
Data json.RawMessage `json:"data"`
Timestamp int64 `json:"timestamp"`
}
type migrateToV7EditNoteDataV1 struct {
NoteUUID string `json:"note_uuid"`
FromBook string `json:"from_book"`
ToBook string `json:"to_book"`
Content string `json:"content"`
}
type migrateToV7EditNoteDataV2 struct {
NoteUUID string `json:"note_uuid"`
FromBook string `json:"from_book"`
ToBook *string `json:"to_book"`
Content *string `json:"content"`
Public *bool `json:"public"`
}

View file

@ -45,6 +45,17 @@ func WriteFile(ctx infra.DnoteCtx, fixturePath string, filename string) {
}
}
func WriteFileWithContent(ctx infra.DnoteCtx, content []byte, filename string) {
dp, err := filepath.Abs(filepath.Join(ctx.DnoteDir, filename))
if err != nil {
panic(err)
}
if err := ioutil.WriteFile(dp, content, 0644); err != nil {
panic(err)
}
}
func ReadFile(ctx infra.DnoteCtx, filename string) []byte {
path := filepath.Join(ctx.DnoteDir, filename)
@ -127,3 +138,18 @@ func ReadJSON(path string, destination interface{}) {
panic(errors.Wrap(err, "Failed to get event"))
}
}
// IsEqualJSON deeply compares two JSON byte slices
func IsEqualJSON(s1, s2 []byte) (bool, error) {
var o1 interface{}
var o2 interface{}
if err := json.Unmarshal(s1, &o1); err != nil {
return false, errors.Wrap(err, "unmarshalling first JSON")
}
if err := json.Unmarshal(s2, &o2); err != nil {
return false, errors.Wrap(err, "unmarshalling second JSON")
}
return reflect.DeepEqual(o1, o2), nil
}