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

Suppress unnecessary query #220

Merged
merged 1 commit into from Feb 13, 2017

Conversation

Projects
None yet
3 participants
@morygonzalez
Copy link
Member

morygonzalez commented Sep 17, 2016

この Pull Request が解決する問題

  • パーマリンクの生成時に毎回 options テーブルに対してクエリが流れていて、管理画面でエントリ一覧を表示するときなどに N+1 が発生しているのを修正しました。
  • テーマのモバイルかどうかの判定のために sites テーブルに対して複数回クエリが流れていたのを修正しました。
@morygonzalez

This comment has been minimized.

Copy link
Member Author

morygonzalez commented Sep 17, 2016

テスト盛大に落ちてるので直します…

@morygonzalez

This comment has been minimized.

Copy link
Member Author

morygonzalez commented Sep 17, 2016

こういうのを使って request ごとのキャッシュにしないと管理画面で permalink の設定を変更した後に反映されないですね。

https://github.com/steveklabnik/request_store

@komagata

This comment has been minimized.

Copy link
Member

komagata commented Sep 19, 2016

なるほどー

end

def permalink_format
@permalink_format = RequestStore[:permalink_format] ||= Option.permalink_format

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Dec 28, 2016

Line is too long. [85/80]

@@ -258,11 +258,15 @@ class << post
end

def custom_permalink?
Option.permalink_enabled == "true"
@custom_permalink = RequestStore[:custom_permalink] ||= Option.permalink_enabled == "true"

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Dec 28, 2016

Line is too long. [96/80]

@@ -145,3 +145,4 @@ def load_plugin(app)
require 'lokka/helpers/render_helper'
require 'lokka/app'
require 'securerandom'
require 'request_store'

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Dec 28, 2016

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@morygonzalez morygonzalez force-pushed the morygonzalez:suppress-unnecessary-query branch from 10a1702 to 03d42c2 Dec 28, 2016

@komagata

This comment has been minimized.

Copy link
Member

komagata commented Jan 5, 2017

@morygonzalez こちら、もう上記の問題はOKな状態ですかね。

@morygonzalez

This comment has been minimized.

Copy link
Member Author

morygonzalez commented Jan 5, 2017

@komagata おー、すんません。年末に CI 回したまま忘れてました。念のためもう一回手元で動作を確認してみます。もうちょいお待ちください 🙇🏻

@morygonzalez morygonzalez changed the title Suppress unnecessary query [WIP]Suppress unnecessary query Jan 5, 2017

@morygonzalez morygonzalez force-pushed the morygonzalez:suppress-unnecessary-query branch from 03d42c2 to 86c9145 Jan 5, 2017

%td= link_to t('edit'), url("/admin/users/#{user.id}/edit"), :class => 'button'
%td
- unless user == current_user
= link_to t('delete'), url("/admin/users/#{user.id}"), :class => 'button', :confirm => t('are_you_sure'), :method => :delete

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Line is too long. [136/80]

%td.name= link_to user.name, url("/admin/users/#{user.id}/edit")
%td= user.email
%td= l(user.created_at, :format => :long)
%td= link_to t('edit'), url("/admin/users/#{user.id}/edit"), :class => 'button'

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Line is too long. [87/80]

%th= t('user.created_at')
%th(colspan="2")
- @users.each_with_index do |user, i|
%tr{:class => (i + 1).odd? ? 'odd' : 'even'}

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Hash attribute should start with one space after the opening brace
Hash attribute should end with one space before the closing brace

%th= t('user.name')
%th= t('user.email')
%th= t('user.created_at')
%th(colspan="2")

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Prefer the hash attributes syntax (%tag{ lang: 'en' }) over HTML attributes syntax (%tag(lang=en))

%tr{:class => (i + 1).odd? ? 'odd' : 'even'}
%td.title= link_to "#{tag.name}(#{tag.taggings.count})", url(tag.link)
%td= link_to t('edit'), url("/admin/tags/#{tag.id}/edit"), :class => 'button'
%td= link_to t('delete'), url("/admin/tags/#{tag.id}"), :class => 'button', :confirm => t('are_you_sure'), :method => :delete

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Line is too long. [133/80]

- @tags.each_with_index do |tag, i|
%tr{:class => (i + 1).odd? ? 'odd' : 'even'}
%td.title= link_to "#{tag.name}(#{tag.taggings.count})", url(tag.link)
%td= link_to t('edit'), url("/admin/tags/#{tag.id}/edit"), :class => 'button'

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Line is too long. [85/80]

%th= t('tag.name')
%th(colspan="2")
- @tags.each_with_index do |tag, i|
%tr{:class => (i + 1).odd? ? 'odd' : 'even'}

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Hash attribute should start with one space after the opening brace
Hash attribute should end with one space before the closing brace

%table.tags
%tr
%th= t('tag.name')
%th(colspan="2")

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Prefer the hash attributes syntax (%tag{ lang: 'en' }) over HTML attributes syntax (%tag(lang=en))

%td= l(snippet.created_at, :format => :long)
%td= l(snippet.updated_at, :format => :long)
%td= link_to t('edit'), snippet.edit_link, :class => 'button'
%td= link_to t('delete'), url("/admin/snippets/#{snippet.id}"), :class => 'button', :confirm => t('are_you_sure'), :method => :delete

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Line is too long. [141/80]

%th= t('snippet.updated_at')
%th(colspan="2")
- @snippets.each_with_index do |snippet, i|
%tr{:class => (i + 1).odd? ? 'odd' : 'even'}

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Hash attribute should start with one space after the opening brace
Hash attribute should end with one space before the closing brace

%th= t('snippet.body')
%th= t('snippet.created_at')
%th= t('snippet.updated_at')
%th(colspan="2")

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Prefer the hash attributes syntax (%tag{ lang: 'en' }) over HTML attributes syntax (%tag(lang=en))

@@ -5,6 +5,7 @@
%meta(http-equiv="Content-Type" content="text/html; charset=utf-8")
%meta(http-equiv="Content-Style-Type" content="text/css")
%meta(http-equiv="Content-Script-Type" content="text/javascript")
%meta(name="viewport" content="width=device-width")

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Prefer the hash attributes syntax (%tag{ lang: 'en' }) over HTML attributes syntax (%tag(lang=en))

@@ -4,6 +4,7 @@
%meta(http-equiv="Content-Type" content="text/html; charset=utf-8")
%meta(http-equiv="Content-Style-Type" content="text/css")
%meta(http-equiv="Content-Script-Type" content="text/javascript")
%meta(name="viewport" content="width=device-width")

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Prefer the hash attributes syntax (%tag{ lang: 'en' }) over HTML attributes syntax (%tag(lang=en))

%ul
%li.list= link_to t('sites'), url('/admin/site/edit')
%li.list= link_to t('permalink.title'), url('/admin/permalink')

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Files should end with a trailing newline

%td= l field_name.created_at.to_time
%td= l field_name.updated_at.to_time
%td
= link_to t('delete'), url("/admin/field_names/#{field_name.id}"), :class => 'button', :confirm => t('are_you_sure'), :method => :delete

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Line is too long. [144/80]

%th= t('field_name.updated_at')
%th
- @field_names.each_with_index do |field_name, i|
%tr{:class => (i + 1).odd? ? 'odd' : 'even'}

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Hash attribute should start with one space after the opening brace
Hash attribute should end with one space before the closing brace

@morygonzalez morygonzalez force-pushed the morygonzalez:suppress-unnecessary-query branch from 86c9145 to 2d59314 Jan 5, 2017

@@ -0,0 +1,12 @@
module Lokka

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Missing frozen string literal comment.

@@ -141,7 +141,9 @@ def load_plugin(app)
require 'lokka/models/markup'
require 'lokka/importer'
require 'lokka/before'
require 'lokka/after'

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Jan 5, 2017

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@@ -0,0 +1,12 @@
module Lokka
module After

This comment has been minimized.

Copy link
@morygonzalez

morygonzalez Jan 5, 2017

Author Member

theme や custom_permalink の値を RequestStore でキャッシュすると設定を変更したときにリロードしただけでは更新されないので(プロセスが kill されると更新した値になる)、 after hook で RequestStore をリセットするようにしました。

This comment has been minimized.

Copy link
@morygonzalez

morygonzalez Jan 5, 2017

Author Member

む、やっぱり use RequestStore::Middleware するだけでよさそう。
https://github.com/steveklabnik/request_store/blob/master/lib/request_store/middleware.rb

@morygonzalez morygonzalez force-pushed the morygonzalez:suppress-unnecessary-query branch from 2d59314 to f16860f Jan 8, 2017

@morygonzalez morygonzalez changed the title [WIP]Suppress unnecessary query Suppress unnecessary query Feb 11, 2017

Suppress unnecessary query
Use request_store to suppress query

@morygonzalez morygonzalez force-pushed the morygonzalez:suppress-unnecessary-query branch from f16860f to ee7c410 Feb 11, 2017

@morygonzalez

This comment has been minimized.

Copy link
Member Author

morygonzalez commented Feb 12, 2017

@komagata 時間かかってすみません。 Puma でも確認しましたが問題なさそうでしたので Merge してオッケーです!

@komagata

This comment has been minimized.

Copy link
Member

komagata commented Feb 13, 2017

@morygonzalez あざーす!

@komagata komagata merged commit 5674f46 into lokka:master Feb 13, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound 3 violations found.

@morygonzalez morygonzalez deleted the morygonzalez:suppress-unnecessary-query branch Feb 13, 2017

morygonzalez pushed a commit to morygonzalez/lokka that referenced this pull request Feb 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.