FEATURE: Log Search Queries

This commit is contained in:
Robin Ward 2017-07-13 13:34:31 -04:00
parent 951fd1219d
commit 97e211f837
8 changed files with 256 additions and 10 deletions

View File

@ -22,6 +22,10 @@ class SearchController < ApplicationController
search_args[:type_filter] = type if type search_args[:type_filter] = type if type
end end
search_args[:search_type] = :full_page
search_args[:ip_address] = request.remote_ip
search_args[:user_id] = current_user.id if current_user.present?
search = Search.new(params[:q], search_args) search = Search.new(params[:q], search_args)
result = search.execute result = search.execute
@ -37,7 +41,6 @@ class SearchController < ApplicationController
render_json_dump(serializer) render_json_dump(serializer)
end end
end end
end end
def query def query
@ -55,7 +58,11 @@ class SearchController < ApplicationController
search_args[:type_filter] = type if type search_args[:type_filter] = type if type
end end
search = Search.new(params[:term], search_args.symbolize_keys) search_args[:search_type] = :header
search_args[:ip_address] = request.remote_ip
search_args[:user_id] = current_user.id if current_user.present?
search = Search.new(params[:term], search_args)
result = search.execute result = search.execute
render_serialized(result, GroupedSearchResultSerializer, result: result) render_serialized(result, GroupedSearchResultSerializer, result: result)
end end

47
app/models/search_log.rb Normal file
View File

@ -0,0 +1,47 @@
require_dependency 'enum'
class SearchLog < ActiveRecord::Base
validates_presence_of :term, :ip_address
def self.search_types
@search_types ||= Enum.new(
header: 1,
full_page: 2
)
end
def self.log(term:, search_type:, ip_address:, user_id:nil)
update_sql = <<~SQL
UPDATE search_logs
SET term = :term,
created_at = :created_at
WHERE created_at > :timeframe AND
position(term IN :term) = 1 AND
((:user_id IS NULL AND ip_address = :ip_address) OR
(user_id = :user_id))
RETURNING id
SQL
rows = exec_sql(
update_sql,
term: term,
created_at: Time.zone.now,
timeframe: 5.seconds.ago,
user_id: user_id,
ip_address: ip_address
)
if rows.cmd_tuples == 0
result = create(
term: term,
search_type: search_types[search_type],
ip_address: ip_address,
user_id: user_id
)
[:created, result.id]
else
[:updated, rows[0]['id'].to_i]
end
end
end

View File

@ -947,6 +947,7 @@ en:
search_tokenize_chinese_japanese_korean: "Force search to tokenize Chinese/Japanese/Korean even on non CJK sites" search_tokenize_chinese_japanese_korean: "Force search to tokenize Chinese/Japanese/Korean even on non CJK sites"
search_prefer_recent_posts: "If searching your large forum is slow, this option tries an index of more recent posts first" search_prefer_recent_posts: "If searching your large forum is slow, this option tries an index of more recent posts first"
search_recent_posts_size: "How many recent posts to keep in the index" search_recent_posts_size: "How many recent posts to keep in the index"
log_search_queries: "Log search queries performed by users"
allow_uncategorized_topics: "Allow topics to be created without a category. WARNING: If there are any uncategorized topics, you must recategorize them before turning this off." allow_uncategorized_topics: "Allow topics to be created without a category. WARNING: If there are any uncategorized topics, you must recategorize them before turning this off."
allow_duplicate_topic_titles: "Allow topics with identical, duplicate titles." allow_duplicate_topic_titles: "Allow topics with identical, duplicate titles."
unique_posts_mins: "How many minutes before a user can make a post with the same content again" unique_posts_mins: "How many minutes before a user can make a post with the same content again"

View File

@ -1154,6 +1154,7 @@ search:
search_tokenize_chinese_japanese_korean: false search_tokenize_chinese_japanese_korean: false
search_prefer_recent_posts: false search_prefer_recent_posts: false
search_recent_posts_size: 100000 search_recent_posts_size: 100000
log_search_queries: true
uncategorized: uncategorized:
version_checks: version_checks:

View File

@ -0,0 +1,12 @@
class CreateSearchLogs < ActiveRecord::Migration
def change
create_table :search_logs do |t|
t.string :term, null: false
t.integer :user_id, null: true
t.inet :ip_address, null: false
t.integer :clicked_topic_id, null: true
t.integer :search_type, null: false
t.datetime :created_at, null: false
end
end
end

View File

@ -196,6 +196,15 @@ class Search
# Query a term # Query a term
def execute def execute
if SiteSetting.log_search_queries?
SearchLog.log(
term: @term,
search_type: @opts[:search_type],
ip_address: @opts[:ip_address],
user_id: @opts[:user_id]
)
end
unless @filters.present? unless @filters.present?
min_length = @opts[:min_search_term_length] || SiteSetting.min_search_term_length min_length = @opts[:min_search_term_length] || SiteSetting.min_search_term_length
terms = (@term || '').split(/\s(?=(?:[^"]|"[^"]*")*$)/).reject {|t| t.length < min_length } terms = (@term || '').split(/\s(?=(?:[^"]|"[^"]*")*$)/).reject {|t| t.length < min_length }

View File

@ -3,7 +3,6 @@ require 'rails_helper'
describe SearchController do describe SearchController do
context "integration" do context "integration" do
before do before do
SearchIndexer.enable SearchIndexer.enable
end end
@ -20,10 +19,42 @@ describe SearchController do
end end
end end
context "#query" do
it "logs the search term" do
SiteSetting.log_search_queries = true
xhr :get, :query, term: 'wookie'
expect(response).to be_success
expect(SearchLog.where(term: 'wookie')).to be_present
end
it "doesn't log when disabled" do
SiteSetting.log_search_queries = false
xhr :get, :query, term: 'wookie'
expect(response).to be_success
expect(SearchLog.where(term: 'wookie')).to be_blank
end
end
context "#show" do
it "logs the search term" do
SiteSetting.log_search_queries = true
xhr :get, :show, q: 'bantha'
expect(response).to be_success
expect(SearchLog.where(term: 'bantha')).to be_present
end
it "doesn't log when disabled" do
SiteSetting.log_search_queries = false
xhr :get, :show, q: 'bantha'
expect(response).to be_success
expect(SearchLog.where(term: 'bantha')).to be_blank
end
end
let(:search_context) { {type: 'user', id: 'eviltrout'} } let(:search_context) { {type: 'user', id: 'eviltrout'} }
context "basics" do pending "basics" do
let(:guardian) { Guardian.new } let(:guardian) { Guardian.new }
let(:search) { mock() } let(:search) { mock() }
@ -61,7 +92,7 @@ describe SearchController do
end end
context "search context" do pending "search context" do
it "raises an error with an invalid context type" do it "raises an error with an invalid context type" do
expect { expect {
@ -76,7 +107,6 @@ describe SearchController do
end end
context "with a user" do context "with a user" do
let(:user) { Fabricate(:user) } let(:user) { Fabricate(:user) }
it "raises an error if the user can't see the context" do it "raises an error if the user can't see the context" do
@ -85,7 +115,6 @@ describe SearchController do
expect(response).not_to be_success expect(response).not_to be_success
end end
it 'performs the query with a search context' do it 'performs the query with a search context' do
guardian = Guardian.new guardian = Guardian.new
Guardian.stubs(:new).returns(guardian) Guardian.stubs(:new).returns(guardian)
@ -96,10 +125,7 @@ describe SearchController do
xhr :get, :query, term: 'test', search_context: {type: 'user', id: user.username} xhr :get, :query, term: 'test', search_context: {type: 'user', id: user.username}
end end
end end
end end

View File

@ -0,0 +1,143 @@
require 'rails_helper'
RSpec.describe SearchLog, type: :model do
describe ".log" do
context "when anonymous" do
it "logs and updates the search" do
Timecop.freeze do
action, log_id = SearchLog.log(
term: 'jabba',
search_type: :header,
ip_address: '192.168.0.33'
)
expect(action).to eq(:created)
log = SearchLog.find(log_id)
expect(log.term).to eq('jabba')
expect(log.search_type).to eq(SearchLog.search_types[:header])
expect(log.ip_address).to eq('192.168.0.33')
action, updated_log_id = SearchLog.log(
term: 'jabba the hut',
search_type: :header,
ip_address: '192.168.0.33'
)
expect(action).to eq(:updated)
expect(updated_log_id).to eq(log_id)
end
end
it "creates a new search with a different prefix" do
Timecop.freeze do
action, _ = SearchLog.log(
term: 'darth',
search_type: :header,
ip_address: '127.0.0.1'
)
expect(action).to eq(:created)
action, _ = SearchLog.log(
term: 'anakin',
search_type: :header,
ip_address: '127.0.0.1'
)
expect(action).to eq(:created)
end
end
it "creates a new search with a different ip" do
Timecop.freeze do
action, _ = SearchLog.log(
term: 'darth',
search_type: :header,
ip_address: '127.0.0.1'
)
expect(action).to eq(:created)
action, _ = SearchLog.log(
term: 'darth',
search_type: :header,
ip_address: '127.0.0.2'
)
expect(action).to eq(:created)
end
end
end
context "when logged in" do
let(:user) { Fabricate(:user) }
it "logs and updates the search" do
Timecop.freeze do
action, log_id = SearchLog.log(
term: 'hello',
search_type: :full_page,
ip_address: '192.168.0.1',
user_id: user.id
)
expect(action).to eq(:created)
log = SearchLog.find(log_id)
expect(log.term).to eq('hello')
expect(log.search_type).to eq(SearchLog.search_types[:full_page])
expect(log.ip_address).to eq('192.168.0.1')
expect(log.user_id).to eq(user.id)
action, updated_log_id = SearchLog.log(
term: 'hello dolly',
search_type: :header,
ip_address: '192.168.0.33',
user_id: user.id
)
expect(action).to eq(:updated)
expect(updated_log_id).to eq(log_id)
end
end
it "logs again if time has passed" do
Timecop.freeze(10.minutes.ago) do
action, _ = SearchLog.log(
term: 'hello',
search_type: :full_page,
ip_address: '192.168.0.1',
user_id: user.id
)
expect(action).to eq(:created)
end
Timecop.freeze do
action, _ = SearchLog.log(
term: 'hello',
search_type: :full_page,
ip_address: '192.168.0.1',
user_id: user.id
)
expect(action).to eq(:created)
end
end
it "logs again with a different user" do
Timecop.freeze do
action, _ = SearchLog.log(
term: 'hello',
search_type: :full_page,
ip_address: '192.168.0.1',
user_id: user.id
)
expect(action).to eq(:created)
action, _ = SearchLog.log(
term: 'hello dolly',
search_type: :full_page,
ip_address: '192.168.0.1',
user_id: Fabricate(:user).id
)
expect(action).to eq(:created)
end
end
end
end
end