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

Add Nazrin::DataAccessor::Struct #25

Merged
merged 1 commit into from
Jan 30, 2018
Merged

Add Nazrin::DataAccessor::Struct #25

merged 1 commit into from
Jan 30, 2018

Conversation

AMHOL
Copy link
Contributor

@AMHOL AMHOL commented Jan 25, 2018

This PR adds a Struct data-accessor, if the class that includes Nazrin::Searchable AND defines a domain_name config, the schema will be read using an AWS-Cloudsearch describe_index_fields call and data will be loaded from Cloudsearch and injected into the constructor when a search is made.

Example:

class User
  include Nazrin::Searchable

  searchable_configure do |config|
    config.domain_name = 'my-users-domain'
  end

  def initialize(attributes)
    @attributes = attributes
  end
end

User.search.query_parser('structured').query('matchall').execute

The schema is read and transformed into a mapping which looks like:

{
  'id' => 'int',
  'name' => 'text',
  'permissions' => 'literal-array'
}

If the response looks like:

{
  'data' => {
    'hits' => {
      'hit' => [
        {
          'id' => '1',
          'fields' => {
            'id' => ['1'],
            'name' => ['Joan'],
            'permissions' => ['admin']
          }
        }
      ]
    }
  }
}

It will result in a call like:

User.new('id' => '1', 'name' => 'Joan', 'permissions' => ['admin'])

@tsuwatch tsuwatch self-requested a review January 26, 2018 03:46
@@ -1,6 +1,9 @@
module Nazrin
module Searchable
class Configuration
attr_reader :domain_name
attr_writer :domain_name
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attr_accessor is preferable.

@tsuwatch
Copy link
Owner

tsuwatch commented Jan 26, 2018

It makes sense.
I would like to include Nazrin::Searchable, not Nazrin::Struct if possible.
But I have no idea yet.
Do you have any idea?

@AMHOL
Copy link
Contributor Author

AMHOL commented Jan 28, 2018

Perhaps we could make it the default data processor if the class doesn't descend from AR::Base or Mongoid::Document, but I like the idea of including Nazrin::Struct as it allows you to load the data from Cloudfront even when using AR or Mongo, if you choose to.

@tsuwatch
Copy link
Owner

tsuwatch commented Jan 29, 2018

I understand.
I think it is a little hard to expect the behavior, Struct is DataAccessor.
How about making a conditional branch with domain_name?

@AMHOL
Copy link
Contributor Author

AMHOL commented Jan 29, 2018

@tsuwatch I implemented what you asked here, I've also invited you as a collaborator on my fork, so if you're happy with those changes, feel free to merge the PR there and let me know if you want me to squash the commits / update the description in this PR.

@AMHOL AMHOL changed the title Add Nazrin::Struct Add Nazrin::DataAccessor::Struct Jan 30, 2018
@tsuwatch
Copy link
Owner

Please, squash commits, update description.

@AMHOL
Copy link
Contributor Author

AMHOL commented Jan 30, 2018

@tsuwatch done 😄

@tsuwatch tsuwatch merged commit cb58c36 into tsuwatch:master Jan 30, 2018
@tsuwatch tsuwatch deleted the feature/nazrin-struct branch January 30, 2018 04:28
@tsuwatch tsuwatch restored the feature/nazrin-struct branch January 30, 2018 04:28
@AMHOL AMHOL deleted the feature/nazrin-struct branch January 30, 2018 04:29
@AMHOL
Copy link
Contributor Author

AMHOL commented Jan 30, 2018

Thanks @tsuwatch 😄

@tsuwatch
Copy link
Owner

Merged, thank you so much!!! 😆

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

2 participants