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

Issue/62 orm adapter pattern #339

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

simonireilly
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Related Issue Fix #62
Need Doc update no

Describe your change

This change implements a Plug and Adapter for the ORM's supported by the algoliasearch-rails gem.

When the adapter is required we should invoke DatabaseAdapter.class_method_defined_within(*args) which is the Plug class.

This should pass this on to the required adapter. The required adapter must also implement this method e.g. DatabaseAdapter::RequiredAdapter.class_method_defined_within(*args).

Main

  • DatabaseAdapter is a plug for various orm adapter classes
    • DatabaseAdapter::ActiveRecord
    • DatabaseAdapter::Sequel
    • DatabaseAdapter::Mongoid
  • DatabaseAdapter implements logic for determining which ORM adapter should be used

Test

  • Unit specs are added for all the new classes.
    • /spec/algoliasearch/database_adaper_spec.rb
    • /spec/algoliasearch/database_adapter/active_record_spec.rb
    • /spec/algoliasearch/database_adapter/sequel_spec.rb
    • /spec/algoliasearch/database_adapter/mongoid_spec.rb
  • The ability to switch database in tests is added, to seperate unit spec concerns from integration concerns
    • spec/spec_helper.rb
  • No new integration specs are added as all should continue to work as described

What problem is this fixing?

#62 is correct, the codebase is very hard to read for new contributors and difficult in general because of various methods implemented for different ORM's.

This change will reduce the amount of complexity in the main algoliasearch-rails.rb and will also provide a separation of concerns for ORM's.

@simonireilly simonireilly marked this pull request as ready for review February 24, 2019 10:38
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.

Introduce an Adapter pattern to handle various ORMs
1 participant