Skip to content

feat: add upsert object handler #31

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

Closed
wants to merge 5 commits into from

Conversation

ankitjena
Copy link
Contributor

@ankitjena ankitjena commented Jun 15, 2021

What kind of change does this PR introduce?

Feature

What is the current behavior?

The create object handler throws duplicate not allowed error, if object already exists

What is the new behavior?

Adds route

POST /object/upsert/:bucketName/*

which inserts an object if it doesn't exist. If it does exist it updates the object with new file.

Screen recording

Peek 2021-06-15 22-23

})
}

const objectResponse = await postgrest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the upsert function of postgrest? Saves us a API call

@inian
Copy link
Member

inian commented Jun 16, 2021

Hi Ankit, thank you so much for the PR! Looking at the code, once you use the upsert call of postgrest-js, the code will be almost exactly the same as createObject.

What do you think about adding upsert functionality into createObject itself? By default, we won't upsert but if there is a header called x-upsert: true, we upsert instead of insert.

Another advantage of this, we wont need to create a new API endpoint. The API endpoint /upsert/:bucketName/* will probably clash with the current endpoint if the user has a bucket name called upsert which is a problem

@ankitjena
Copy link
Contributor Author

Right, I was thinking of the same thing about having upsert inside createObject, having a discussion on that would have been better I guess. Also I didn't think of the clash with bucketName for the upsert endpoint. Thanks Inian. I will work on that and make the changes.

@inian
Copy link
Member

inian commented Jun 16, 2021

Thanks Ankit :)

I didn't think of the clash with bucketName for the upsert endpoint.

I learnt this the hard way as well haha #22

@inian
Copy link
Member

inian commented Jun 16, 2021

Also can you merge in the changes from master Ankit? I just made some changes to the createObject file.

@ankitjena
Copy link
Contributor Author

Also can you merge in the changes from master Ankit? I just made some changes to the createObject file.

Sure.

@inian I was trying the upsert now, for some reason it doesn't work, looked up the documentation in postgrest-js
image

const { data: results, error, status } = await postgrest
        .from<Obj>('objects')
        .insert(
          [
            {
              name: objectName,
              owner: owner,
              bucket_id: bucketName,
            },
          ],
          {
            returning: 'minimal',
            upsert: true,
          }
        )
        .single()

I get the same error

{
  "statusCode": "23505",
  "error": "Key (bucket_id, name)=(avatars, folder/cat-new.png) already exists.",
  "message": "duplicate key value violates unique constraint \"bucketid_objname\""
}

@ankitjena
Copy link
Contributor Author

ankitjena commented Jun 20, 2021

@inian I was trying the upsert now, for some reason it doesn't work, looked up the documentation in postgrest-js

Apparently, the postgrest client wasn't exposing the upsert method in query builder, I upgraded the package and using it worked. I was trying out the deprecated insert operation with upsert sadly.

@ankitjena ankitjena requested a review from inian June 20, 2021 05:54
@ankitjena
Copy link
Contributor Author

I messed up the merge somehow, closing this PR and creating another one.

@ankitjena ankitjena closed this Jun 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants