Skip to content
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

Support a dynamic collection name #4556

Closed
wants to merge 1 commit into from
Closed

Support a dynamic collection name #4556

wants to merge 1 commit into from

Conversation

reidmorrison
Copy link
Contributor

@reidmorrison reidmorrison commented Sep 27, 2018

Sometimes we want to create a model and then use that model for different Mongo collections, ideally chosen dynamically at run-time.

For Example, for a static one-to-one mapping between a model and its class:

Band.create!(name: "Depeche Mode")
band = Band.where(name: "Depeche Mode").first
band.rating = 23
band.save!

Dynamically changing the collection name at run-time:

Band.with(collection: 'other_bands').create!(name: "Depeche Mode")

band = Band.with(collection: 'other_bands').where(name: "Depeche Mode").first
band.rating = 23
band.save!

In Mongoid 5.4, the above kind of worked. It was extremely inefficient and led to all kinds of problems because every call to with resulted in a brand new persistence context. Additionally, the save! would not save the changed document back to the other_bands collection.

In Mongoid 6/7 it uses a block in order to cleanup the persistence context on completion:

Band.with(collection: 'other_bands') do
  Band.create!(name: "Depeche Mode")
  band = Band.with(collection: 'other_bands').where(name: "Depeche Mode").first
  band.rating = 23
  band.save!
end

Even in the above case, creating a brand new persistence context when only the collection_name has changed is extremely inefficient, and it requires the constant use of the block anytime a modified document is saved.

How about we make collection a special case since it does not need a new persistence context every time the collection name is changed. And, if we make the instance of the model remember its collection name then we don't have to complicate any code that attempts to persist any changes to that document.

That would make the following code possible:

Band.with(collection: 'other_bands').create!(name: "Depeche Mode")

band = Band.with(collection: 'other_bands').first

# Don't have to carry around separately the name of the collection
# that this instance of `band` comes from.
# And avoids yet another persistence context to save the change.
band.rating = 23
band.save!

Or even allow the collection name to be supplied directly when creating a new model:

Band.create!(collection_name: 'other_bands', name: "Depeche Mode")

@p-mongo
Copy link
Contributor

p-mongo commented Sep 27, 2018

Just to make sure I understand, does this patch propose the following behavior:

class Band
  include Mongoid::Document
  store_in collection: "artists"
end

# anywhere in the code
Band.with(collection: 'poets')

# anywhere else in the code at a later time
# queries poets, not artists
Band.all

@reidmorrison
Copy link
Contributor Author

reidmorrison commented Sep 27, 2018

Not at all, the persistence context is not modified when .with is called with just a collection_name.

Band.with(collection: 'poets') would return all Band objects within the poets collection.

Calling Band.all later on in the code would return all Band documents in the artists collection.

@p-mongo
Copy link
Contributor

p-mongo commented Sep 27, 2018

OK, I am playing with this and I am not yet sold on the type of result of #with being argument-dependent:

Band.with(collection: foo) # => Mongoid::Criteria

Band.with(collection: foo, database: bar) # => Error: no block given

Band.with(collection: foo, database: bar) { |c| } # => Band, c is also Band

@p-mongo
Copy link
Contributor

p-mongo commented Sep 27, 2018

When you say that creating a persistence context is "extremely inefficient", can you describe your use case and performance you are getting in greater detail please?

@reidmorrison
Copy link
Contributor Author

Makes sense that we should probably not override the existing with method, maybe add with_colllection?

Create a document in the other collection:

Band.create!(collection_name: 'other', name: 'U2')

Find the document in the other collection in one place in the code:

band = Band.with_colllection('other').first

Somewhere else in the application make changes and save the document without having to know which collection this document came from:

band.rating = 45
band.save!

@p-mongo
Copy link
Contributor

p-mongo commented Sep 28, 2018

Can you elaborate on performance issues you are having with the existing implementation?

@reidmorrison
Copy link
Contributor Author

The purpose of this pull request has nothing to do with performance, it is to obtain the above functionality so as to remove the burden from developers to have to figure out which collection a document came from in order to modify it.

@p-mongo
Copy link
Contributor

p-mongo commented Oct 2, 2018

Okay, then, will a non-block form of #with fulfill all of your requirements?

# collection change only
band = Band.with(collection: 'other_bands').where(name: "Depeche Mode").first
band.rating = 23
band.save!

# db and collection change
band = Band.with(db: 'foo', collection: 'other_bands').where(name: "Depeche Mode").first
band.rating = 23
band.save!

@reidmorrison
Copy link
Contributor Author

Yes

@johnnyshields
Copy link
Contributor

I agree we should add a non-block form of #with which returns a Mongoid::Criteria upon which queries can be chained.

@p-mongo
Copy link
Contributor

p-mongo commented Oct 21, 2018

I created https://jira.mongodb.org/browse/MONGOID-4629 to track the work on a non-block form of #with.

@reidmorrison
Copy link
Contributor Author

Close this PR in favor of the above ticket

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants