diff --git a/model/Database.go b/model/Database.go index 1269bd3..4c66490 100644 --- a/model/Database.go +++ b/model/Database.go @@ -9,12 +9,15 @@ import ( "os" "path/filepath" "reflect" + "regexp" + "strconv" "sync" "time" "github.com/davecgh/go-spew/spew" "github.com/pkg/errors" "github.com/sergi/go-diff/diffmatchpatch" + log "github.com/sirupsen/logrus" // Register SQLite driver _ "github.com/mattn/go-sqlite3" @@ -394,19 +397,74 @@ func fetchFromSQLite(sqlite *sql.DB, modelType Model) ([]Model, error) { } mn, err := m.scanRow(rows) if err != nil { - return nil, errors.Wrap(err, "Error while scanning results from SQLite database") + // For some reason a row might contain NULL entries, even though the schema + // shouldn't allow this. Instead of failing the whole import, we can simply skip + // this entry as the data anyway wouldn't be valid. + if !isNullableMismatch(err, rows) { + return nil, errors.Wrapf(err, "Error while scanning row for %T", modelType) + } + log.Warnf("Nullable mismatch in %T at index %d detected. Skipping entry", m, i) + i++ + continue } result[mn.ID()] = mn i++ } err = rows.Err() if err != nil { - return nil, errors.Wrap(err, "Error while scanning results from SQLite database") + return nil, errors.Wrapf(err, "Error while scanning results for %T from SQLite database", modelType) } return result, nil } +// isNullableMismatch checks if a given error is due to a NULL entry in a column that only allows +// non-NULL entries. If this is the case and no other schema mismatch is detected, true is returned. +func isNullableMismatch(err error, rows *sql.Rows) bool { + if err == nil || rows == nil { + return false + } + + re := regexp.MustCompile(`Scan error on column index (\d+), name "(\w+)": converting NULL to (\w+) is unsupported`) + matches := re.FindStringSubmatch(err.Error()) + if len(matches) != 4 { + return false + } + + index, err := strconv.ParseInt(matches[1], 0, 64) + if err != nil { + return false + } + + ct, err := rows.ColumnTypes() + if err != nil { + return false + } + + if len(ct) <= int(index) { + return false + } + + column := ct[index] + if column == nil { + return false + } + + if column.Name() != matches[2] { + return false + } + + if typeName, ok := dbTypeToGoType[column.DatabaseTypeName()]; !ok || typeName != matches[3] { + return false + } + + if n, _ := column.Nullable(); n { + return false + } + + return true +} + // getTableEntryCount returns the number of entries in a given table func getTableEntryCount(sqlite *sql.DB, tableName string) (int, error) { var count int @@ -625,3 +683,9 @@ func createEmptySQLiteDB(filename string) error { return nil } + +// maps a DatabaseTypeName to a go type name +var dbTypeToGoType = map[string]string{ + "INTEGER": "int", + "TEXT": "string", +} diff --git a/model/Database_test.go b/model/Database_test.go index 723d854..2d6fb77 100644 --- a/model/Database_test.go +++ b/model/Database_test.go @@ -148,7 +148,7 @@ func Test_fetchFromSQLite(t *testing.T) { bookmark, err := fetchFromSQLite(sqlite, &Bookmark{}) assert.NoError(t, err) - assert.Len(t, bookmark, 3) + assert.Len(t, bookmark, 4) assert.Equal(t, &Bookmark{2, 3, 7, 4, "Philippians 4", sql.NullString{String: "12 I know how to be low on provisions and how to have an abundance. In everything and in all circumstances I have learned the secret of both how to be full and how to hunger, both how to have an abundance and how to do without. ", Valid: true}, 0, sql.NullInt32{}}, bookmark[2]) inputField, err := fetchFromSQLite(sqlite, &InputField{}) @@ -173,7 +173,7 @@ func Test_fetchFromSQLite(t *testing.T) { tagMap, err := fetchFromSQLite(sqlite, &TagMap{}) assert.NoError(t, err) - assert.Len(t, tagMap, 3) + assert.Len(t, tagMap, 4) assert.Equal(t, &TagMap{2, sql.NullInt32{Int32: 0, Valid: false}, sql.NullInt32{Int32: 0, Valid: false}, sql.NullInt32{Int32: 2, Valid: true}, 2, 1}, tagMap[2]) userMark, err := fetchFromSQLite(sqlite, &UserMark{}) @@ -182,6 +182,160 @@ func Test_fetchFromSQLite(t *testing.T) { assert.Equal(t, &UserMark{2, 1, 2, 0, "2C5E7B4A-4997-4EDA-9CFF-38A7599C487B", 1}, userMark[2]) } +func Test_isNullableMismatch(t *testing.T) { + type args struct { + err error + rows func() *sql.Rows + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Nullable mismatch with int", + args: args{ + err: fmt.Errorf(`Scan error on column index 2, name "NotNullable": converting NULL to int is unsupported`), + rows: func() *sql.Rows { + return mockSQLRows(t, + sqlmock.NewColumn("ID").OfType("INTEGER", 1), + sqlmock.NewColumn("Nullable").OfType("INTEGER", sql.NullInt32{}).Nullable(true), + sqlmock.NewColumn("NotNullable").OfType("INTEGER", nil)) + }, + }, + want: true, + }, + { + name: "Nullable mismatch with text", + args: args{ + err: fmt.Errorf(`Scan error on column index 1, name "NotNullable": converting NULL to string is unsupported`), + rows: func() *sql.Rows { + return mockSQLRows(t, + sqlmock.NewColumn("ID").OfType("INTEGER", 1), + sqlmock.NewColumn("NotNullable").OfType("TEXT", nil), + sqlmock.NewColumn("Nullable").OfType("INTEGER", sql.NullInt32{}).Nullable(true)) + }, + }, + want: true, + }, + { + name: "No nullable mismatch", + args: args{ + err: fmt.Errorf(`Scan error on column index 1, name "Nullable": converting NULL to int is unsupported`), + rows: func() *sql.Rows { + return mockSQLRows(t, + sqlmock.NewColumn("ID").OfType("INTEGER", 1), + sqlmock.NewColumn("Nullable").OfType("INTEGER", sql.NullInt32{}).Nullable(true), + sqlmock.NewColumn("NotNullable").OfType("INTEGER", 3)) + }, + }, + want: false, + }, + { + name: "Empty", + }, + { + name: "Different error", + args: args{ + err: fmt.Errorf(`Scan error on column index 1, name "Nullable": converting mock to int is unsupported`), + rows: func() *sql.Rows { + return mockSQLRows(t, + sqlmock.NewColumn("ID").OfType("INTEGER", 1), + sqlmock.NewColumn("Nullable").OfType("INTEGER", sql.NullInt32{}).Nullable(true), + sqlmock.NewColumn("NotNullable").OfType("INTEGER", 3)) + }, + }, + }, + { + name: "No error", + args: args{ + rows: func() *sql.Rows { + return mockSQLRows(t, + sqlmock.NewColumn("ID").OfType("INTEGER", 1), + sqlmock.NewColumn("Nullable").OfType("INTEGER", sql.NullInt32{}).Nullable(true), + sqlmock.NewColumn("NotNullable").OfType("INTEGER", 3)) + }, + }, + }, + { + name: "No rows", + args: args{ + err: fmt.Errorf(`Scan error on column index 1, name "Nullable": converting NULL to int is unsupported`), + }, + }, + { + name: "Column name not matching column index", + args: args{ + err: fmt.Errorf(`Scan error on column index 2, name "WrongName": converting NULL to int is unsupported`), + rows: func() *sql.Rows { + return mockSQLRows(t, + sqlmock.NewColumn("ID").OfType("INTEGER", 1), + sqlmock.NewColumn("Nullable").OfType("INTEGER", sql.NullInt32{}).Nullable(true), + sqlmock.NewColumn("NotNullable").OfType("INTEGER", 3)) + }, + }, + want: false, + }, + { + name: "Type mismatch", + args: args{ + err: fmt.Errorf(`Scan error on column index 2, name "NotNullable": converting NULL to string is unsupported`), + rows: func() *sql.Rows { + return mockSQLRows(t, + sqlmock.NewColumn("ID").OfType("INTEGER", 1), + sqlmock.NewColumn("Nullable").OfType("INTEGER", sql.NullInt32{}).Nullable(true), + sqlmock.NewColumn("NotNullable").OfType("INTEGER", 3)) + }, + }, + want: false, + }, + { + name: "Negative index", + args: args{ + err: fmt.Errorf(`Scan error on column index -5, name "NotNullable": converting NULL to string is unsupported`), + rows: func() *sql.Rows { + return mockSQLRows(t, + sqlmock.NewColumn("ID").OfType("INTEGER", 1), + sqlmock.NewColumn("Nullable").OfType("INTEGER", sql.NullInt32{}).Nullable(true), + sqlmock.NewColumn("NotNullable").OfType("TEXT", nil)) + }, + }, + want: false, + }, + { + name: "More columns in error than in rows - don't panic with index out of range", + args: args{ + err: fmt.Errorf(`Scan error on column index 2, name "NotNullable": converting NULL to int is unsupported`), + rows: func() *sql.Rows { + return mockSQLRows(t, + sqlmock.NewColumn("ID").OfType("int", 1), + sqlmock.NewColumn("Nullable").OfType("int", sql.NullInt32{}).Nullable(true)) + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.args.rows == nil { + tt.args.rows = func() *sql.Rows { return nil } + } + got := isNullableMismatch(tt.args.err, tt.args.rows()) + assert.Equal(t, tt.want, got) + }) + } +} + +func mockSQLRows(t *testing.T, columns ...*sqlmock.Column) *sql.Rows { + db, mock, err := sqlmock.New() + assert.NoError(t, err) + defer db.Close() + + mock.ExpectQuery("Placeholder").WillReturnRows(mock.NewRowsWithColumnDefinition(columns...)) + result, err := db.Query("Placeholder") + assert.NoError(t, err) + return result +} + func TestDatabase_importSQLite(t *testing.T) { db := Database{} @@ -191,12 +345,12 @@ func TestDatabase_importSQLite(t *testing.T) { // As we already test the correctness in Test_fetchFromSQLite, // it should be sufficient to just double-check the size of the slices. assert.Len(t, db.BlockRange, 5) - assert.Len(t, db.Bookmark, 3) + assert.Len(t, db.Bookmark, 4) assert.Len(t, db.InputField, 4) assert.Len(t, db.Location, 9) assert.Len(t, db.Note, 3) assert.Len(t, db.Tag, 3) - assert.Len(t, db.TagMap, 3) + assert.Len(t, db.TagMap, 4) assert.Len(t, db.UserMark, 5) path = filepath.Join("testdata", "error_playlistMedia.db") diff --git a/model/manifest_test.go b/model/manifest_test.go index bd9e4fe..05beb3a 100644 --- a/model/manifest_test.go +++ b/model/manifest_test.go @@ -14,7 +14,7 @@ var exampleManifest = &manifest{ CreationDate: time.Now().Format("2006-01-02"), UserDataBackup: userDataBackup{ LastModifiedDate: time.Now().Format("2006-01-02T15:04:05-07:00"), - Hash: "55e87dda924edb6a6c93871ee3c642119da4ade73fab05049bbd32ee5411dfeb", + Hash: "19f994bb2fcda04e213920ddee729337296d90cad108bf281c6f6bbf9f2e5fce", DatabaseName: "user_data.db", SchemaVersion: 8, DeviceName: "go-jwlm", diff --git a/model/testdata/user_data.db b/model/testdata/user_data.db index 6e82be9..d95baa6 100755 Binary files a/model/testdata/user_data.db and b/model/testdata/user_data.db differ