vuex-easy-firestore icon indicating copy to clipboard operation
vuex-easy-firestore copied to clipboard

Prevent initial doc insert by default (doc mode)

Open coolhand79 opened this issue 6 years ago β€’ 15 comments

I've encountered an issue when doin openDBChannel in docmode.

It seems that if the document doesn't exist, vuex-easy-firestore attempts to insert the doc.

Problem is, if the user doesn't have permission to insert, you enter an infinite loop like so: https://cl.ly/c92c545422b4

It seems when the insert fails, an error should be thrown on the openDBChannel instead.

Thanks!

coolhand79 avatar Dec 21 '18 16:12 coolhand79

Wow @coolhand79 thanks for finding this rare bug. I'll look into it.


After about two years of open source, I finally got accepted for Github Sponsors!

πŸ’œ github.com/sponsors/mesqueeb πŸ’œ

A little about me:

  • I love open-source
  • 6 months ago I got a son, and am trying to be an awesome father πŸ˜…
  • I'm doing freelance work here and there as my main job

If anyone was helped with vuex-easy-firestore, I'd greatly appreciate any support!

BTW, donations get's paid DOUBLE by GitHub! (they're alchemists... 🦾)

Going forward πŸ‘¨πŸΌβ€πŸ’»

  • I got great plans for the future of vuex-easy-firestore going above and beyond!! Look forward to it!!
  • On to many more years of open-sourcing! πŸŽ‰

mesqueeb avatar Dec 21 '18 23:12 mesqueeb

This is becoming an issue for me. Might it be possible to disable the "insert initial doc" behavior?

coolhand79 avatar Jan 17 '19 17:01 coolhand79

Why is this an issue? Why can’t your users have create permissions?

What i usually do is:

match Userdocs/{userId} {
  allow create: if request.auth.uid == userId
}

This makes sure that any user can only insert a document with it’s uid as document key.πŸ—

Can you please explain a bit more on your use case? I’d love to be able to know so i can decide if it’s common enough to change the library. 🌝

For now you can do a manual check if the doc exists or not, if not you can refrain from calling openDBChannel.

Vuex Easy Firestore was made with β™₯ by Luca Ban.
If this library helped you in any way you can support me by buying me a cup of coffee.

mesqueeb avatar Jan 18 '19 00:01 mesqueeb

@mesqueeb

This is semi-related to an issue i'm running into.

My use case is a user visits a page with a query parameter of id which represents the document id.

/doc/?id=123

The problem that i'm running into is that insertInitialDoc will create a document at whatever key is put in as the query param.

Is there a way to validate if a doc exists and if it doesn't, NOT create it?

In my case I just want to load the doc for editing if it exists, else throw a 404. (I am using nuxt if it matters).

wulfmann avatar Jan 21 '19 00:01 wulfmann

This is a good idea. I’ll implement this but the implementation will depend on when/where you execute openDBChannel or fetchAndAdd. Please provide more detail on this.

Vuex Easy Firestore was made with β™₯ by Luca Ban.
If this library helped you in any way you can support me by buying me a cup of coffee.

mesqueeb avatar Jan 21 '19 00:01 mesqueeb

@mesqueeb as this is not entirely related to the original issue i'd be happy to open an enhancement issue.

I can provide a few use cases, but i'd say it'd definitely be wise to make it flexible since my use case me be narrower than others.

wulfmann avatar Jan 21 '19 04:01 wulfmann

I'm thinking of this syntax:

const userDataModule = {
  firestorePath: 'users/{userId}',
  firestoreRefType: 'doc',
  moduleName: 'userData',
  statePropName: 'data',
  sync: {
    preventInitialDocInsertion: true
  }
}

(This would only work for 'doc' mode obviously) the question is, what kind of feedback do you want openDBChannel/fetchAndAdd to give.

If no counter-arguments, I'll push an update on it tomorrow.

Vuex Easy Firestore was made with β™₯ by Luca Ban.
If this library helped you in any way you can support me by buying me a cup of coffee.

mesqueeb avatar Jan 21 '19 15:01 mesqueeb

Thanks for your example, @wulfmann. The issue you describe is very closely related to what I was referring to.

@mesqueeb related to your question - the use case I have is where documents are public for all to read, but only users with certain permission are allowed to create them. Users with permission will create many documents, not just one (thus, their uid wouldn't suffice).

preventInitialDocInsertion: true Would work, though my preference would be to be pessimistic wrt permissions, rather than optimistic.

coolhand79 avatar Jan 21 '19 16:01 coolhand79

Actually I do tend to agree there that it might be better to be pessimistic there.

As an aside this caused a very strange time to debug permissions on the table since I had allowed read but didn’t realize it was also trying to write.

Maybe we could expand the docs on the insert doc part as a part of this.

wulfmann avatar Jan 21 '19 22:01 wulfmann

Thanks for your comments. This is important to me. I also agree that turning off initialDocInsert off by default is better, but this would be a breaking change. And I want to prevent breaking changes as long as possible, then once I have a bunch make a big update one day in the future.

I want to double check two things to both of you:

  • with "pessimistic write permissions" you mean turning it off by default right?
  • if I do add it like I said, through an option inside the module config, would that work for your use cases?

Finally I wanna mention that obviously i'll improve the docs, be more verbal about that my lib will make an insert and how to prevent it.

Vuex Easy Firestore was made with β™₯ by Luca Ban.
If this library helped you in any way you can support me by buying me a cup of coffee.

mesqueeb avatar Jan 22 '19 00:01 mesqueeb

@wulfmann @coolhand79 Waiting for your feedback on my last post to start implementing this.

Vuex Easy Firestore was made with β™₯ by Luca Ban.
If this library helped you in any way you can support me by buying me a cup of coffee.

mesqueeb avatar Jan 22 '19 09:01 mesqueeb

  • with "pessimistic write permissions" you mean turning it off by default right?

Yes

  • if I do add it like I said, through an option inside the module config, would that work for your use cases?

Yes, I think so. I would actually suggest that it could be a higher level option as well:

const easyFirestore = createEasyFirestore(
    [modules...],
    {
        autoInsertInitialDocs: true, // default false
        logging: true
    }
);

and then again on the modules

const userDataModule = {
  firestorePath: 'users/{userId}',
  firestoreRefType: 'doc',
  moduleName: 'userData',
  statePropName: 'data',
  sync: {
    autoInsertInitialDoc: true
  }
}

WRT breaking changes... maybe a method for doing this type of change would be similar to how Firestore did the timestamp change?

coolhand79 avatar Jan 22 '19 16:01 coolhand79

Guys I have updated the library and added the ability to prevent initial doc insertion. Please read the new documentation chapter on this.

As for my reasoning for not wanting a breaking change: Vue 3 will probably come out around June.

When that happens I will update the library to be compatible with Vuex of course, and this will have breaking changes either way. That is the time I will make many optimizations like defaulting to not inserting the initial doc.

Vuex Easy Firestore was made with β™₯ by Luca Ban.
If this library helped you in any way you can support me by buying me a cup of coffee.

mesqueeb avatar Jan 23 '19 03:01 mesqueeb

@coolhand79 @wulfmann Do you guys have any advice for me, to prevent the bigger problem we have in #172 ?

Please read that thread and drop a comment if you have advice. I am thinking disabling initialDocInsert by default and bumping a major version, but this doesn't solve everything.

The multiple tab error can still reset docs in the firebase database without the user's intention......

mesqueeb avatar Mar 18 '19 10:03 mesqueeb

I will prevent initial doc insert (in 'doc' mode) by default in v2.0. So I added the milestone and will re-open πŸ™ƒ


After about two years of open source, I finally got accepted for Github Sponsors!

πŸ’œ github.com/sponsors/mesqueeb πŸ’œ

A little about me:

  • I love open-source
  • 6 months ago I got a son, and am trying to be an awesome father πŸ˜…
  • I'm doing freelance work here and there as my main job

If anyone was helped with vuex-easy-firestore, I'd greatly appreciate any support!

BTW, donations get's paid DOUBLE by GitHub! (they're alchemists... 🦾)

Going forward πŸ‘¨πŸΌβ€πŸ’»

  • I got great plans for the future of vuex-easy-firestore going above and beyond!! Look forward to it!!
  • On to many more years of open-sourcing! πŸŽ‰

mesqueeb avatar Apr 24 '19 21:04 mesqueeb