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

Non-ActiveRecord paginator support #90

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

Conversation

gaorlov
Copy link

@gaorlov gaorlov commented Jun 4, 2018

First and foremost: thanks so much for putting this gem out here!

Problem Statement

I have a service that uses Elasticsearch instead of ActiveRecord and I would like to return the result set counts. Because ActiveRecord::Relation is referenced directly in the counting method, the app explodes. Under the current implementation, my options are to:

  • monkeypatch Pagination.count_records in every Elasticsearch-backed service I write
  • do the counting in the controller and pass it into jsonapi_render via options[:count]

Option one is really a strawman, and it felt odd to sprinkle options: { count: MyEsModel.count( params ) } in every single controller action across multiple controllers in several projects.

Proposed Contribution

I extracted the underlying counting interface in to a RecordCounter module that responds to count( records, params = {}, options ={} ) and loops through dedicated Counter classes that know how to count a specific class type. So like, ActiveRecord::Relation objects go to the ActiveRecordCounter and Arrays go to the ArrayCounter class.

The #{Type}Counter classes register themselves with RecordCounter on declaration with counts #{Type.to_s.underscore} making adding new counters as easy as just declaring them in your project.

What this does

I will admit that this is a somewhat involved change, but there are several benefits to this added complexity. It:

  • eliminates the hard dependency on ActiveRecord::Relation in the counting mechanism
  • allows for easy extensions to counting, both in adding counting for other backend technologies, as well as modifying existing counting functionality without having to modify JSONAPI::Utils

Example

To use my use case, an Elasticsearch-backed system would implement a counter approximately like so:

class ElasticsearchCounter < JSONAPI::Utils::Support::Pagination::RecordCounter::BaseCounter
  counts "elasticsearch/persistence/repository/response/results"

  # the one thing we have to define
  def count
    # not the actual implementation 
    model_class = @records.first.class

    query = model_class.query_from_params @params

    model_class.count query
  end
end

gaorlov added 10 commits May 31, 2018 09:38
updating testing instructions to run all the tests
Moving counting logic into (.*)Counter classes to allow for extensibility
Moving countind delegation into RecordCounter class to allow for a single entrypoint
Passing params into RecordCounters to allow them to do complex counting based on the query
moving filter application out of *Counter classes due to scoping issues
updating instructions to run tests
allowing apply_sort to fall through with no modifications for unexpected result sets
fixing RecordCounter.count to pass request params into the counter classes
@gaorlov
Copy link
Author

gaorlov commented Jun 27, 2018

@tiagopog, I would love some feedback on this.

@gaorlov
Copy link
Author

gaorlov commented Aug 1, 2018

@tiagopog bump?

@gaorlov
Copy link
Author

gaorlov commented May 1, 2019

@tiagopog have you had a chance to look at this at all?

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.

None yet

1 participant