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 #3

Closed
wants to merge 13 commits into from

Conversation

simonireilly
Copy link
Owner

@simonireilly simonireilly commented Feb 23, 2019

Q A
Bug fix? no
New feature? no
BC breaks? no
Related Issue Fix algolia#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?

algolia#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 force-pushed the issue/62_orm_adapter_pattern branch from 8f21bca to d1b7c82 Compare February 23, 2019 19:52
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