ausocean / openfish

OpenFish is an open-source system written in GoLang for classifying marine species. Tasks involve importing video or image data, classifying and annotating data (both manually and automatically), searching, and more. It is expected that OpenFish will use utilize computer vision and machine learning techniques.
https://ausocean.github.io/openfish/
Other
6 stars 0 forks source link

Update now creates non-extant entities #139

Closed scruzin closed 2 months ago

scruzin commented 2 months ago

Also, add a helper function, NewEntity, to create a new entity.

scruzin commented 2 months ago

Can you take a look @scott97 at why this has broken some tests?

scott97 commented 2 months ago

Can you take a look @scott97 at why this has broken some tests?

=== RUN   TestUpdateUserForNonExistentEntity
    users_test.go:137: Did not receive expected error when updating non-existent user

I have a test for trying to update a non-existent entity which looks for an error that no longer occurs. In the openfish api clients can supply a partial object that only contains the parts of the entity to update. So if the entity does not exist, we can't create one since not all the information is guaranteed to be supplied by the client. I think it is easily fixable with no changes to the datastore package needed, I'll look into it.

scott97 commented 2 months ago

@scruzin

How is this for the update function? Takes an extra argument upsert - update or insert in a single operation

func (s *CloudStore) Update(ctx context.Context, key *Key, fn func(Entity), dst Entity, upsert bool) error {
    _, err := s.client.RunInTransaction(ctx, func(tx *datastore.Transaction) error {
        err := tx.Get(key, dst)
        if err != nil && !(upsert && err == ErrNoSuchEntity) {
            return err
        }
        fn(dst)
        _, err = tx.Put(key, dst)
        return err
    })
    return err
}

I have tested this, all tests pass now. Downside is that oceantv is going to need changes to its usage of Update, to include the last argument.

scruzin commented 2 months ago

Can you take a look @scott97 at why this has broken some tests?

=== RUN   TestUpdateUserForNonExistentEntity
    users_test.go:137: Did not receive expected error when updating non-existent user

I have a test for trying to update a non-existent entity which looks for an error that no longer occurs. In the openfish api clients can supply a partial object that only contains the parts of the entity to update. So if the entity does not exist, we can't create one since not all the information is guaranteed to be supplied by the client. I think it is easily fixable with no changes to the datastore package needed, I'll look into it.

We've decided to revert to the original functionality.