ostafen / clover

A lightweight document-oriented NoSQL database written in pure Golang.
MIT License
680 stars 55 forks source link

Error while marshalling json InsertOne #43

Closed Vahanerevan closed 2 years ago

Vahanerevan commented 2 years ago

Issue happens when json marshalling document items cause error, in that case the doc id is nil and next line doc.Get(objectIdField).(string) cause error .

func (db *DB) InsertOne(collectionName string, doc *Document) (string, error) {
    err := db.Insert(collectionName, doc)
    //if error happens then  doc.Get(objectIdField) is nil
    return doc.Get(objectIdField).(string), err
}

//Should be something like

func (db *DB) InsertOne(collectionName string, doc *Document) (string, error) {
    err := db.Insert(collectionName, doc)
    if doc.Get(objectIdField) == nil {
        return "",err
    }
    return doc.Get(objectIdField).(string), err
}
ostafen commented 2 years ago

Hi, @Vahanerevan, thank you for reporting this issue. Do you want to open a PR?

Vahanerevan commented 2 years ago

Sure I did not go deep in code so There are 2 ways to fix this issue

1. Idk if there can be case: where id will not be string . Should we return error ?

if _, ok := doc.Get(objectIdField).(string); !ok {
        return "",err
    }

2.

 func (db *DB) InsertOne(collectionName string, doc *Document) (string, error) {
    err := db.Insert(collectionName, doc)
    if doc.Get(objectIdField) == nil {
        return "",err
    }
    return doc.Get(objectIdField).(string), err
}

Just let me know which one will fit here . I will open PR.

Thank you.

ostafen commented 2 years ago

I think the cleanest way is to use the ObjectId() method of the document struct, which returns an empty string in the case the objectIdField is not set:

func (db *DB) InsertOne(collectionName string, doc *Document) (string, error) {
    err := db.Insert(collectionName, doc)
    return doc.ObjectId(), err
}
ostafen commented 2 years ago

@Vahanerevan, any update on this?