Commit e94cd6fd authored by Nick Thomas's avatar Nick Thomas

Add markdown cache columns to the database, but don't use them yet

This commit adds a number of _html columns and, with the exception of Note,
starts updating them whenever the content of their partner fields changes.

Note has a collision with the note_html attr_accessor; that will be fixed later

A background worker for clearing these cache columns is also introduced - use
`rake cache:clear` to set it off. You can clear the database or Redis caches
separately by running `rake cache:clear:db` or `rake cache:clear:redis`,
respectively.
parent 4a90e25f
...@@ -51,17 +51,15 @@ module GitlabMarkdownHelper ...@@ -51,17 +51,15 @@ module GitlabMarkdownHelper
context[:project] ||= @project context[:project] ||= @project
html = Banzai.render(text, context) html = Banzai.render(text, context)
banzai_postprocess(html, context)
end
context.merge!( def markdown_field(object, field)
current_user: (current_user if defined?(current_user)), object = object.for_display if object.respond_to?(:for_display)
return "" unless object.present?
# RelativeLinkFilter html = Banzai.render_field(object, field)
requested_path: @path, banzai_postprocess(html, object.banzai_render_context(field))
project_wiki: @project_wiki,
ref: @ref
)
Banzai.post_process(html, context)
end end
def asciidoc(text) def asciidoc(text)
...@@ -196,4 +194,18 @@ module GitlabMarkdownHelper ...@@ -196,4 +194,18 @@ module GitlabMarkdownHelper
icon(options[:icon]) icon(options[:icon])
end end
end end
# Calls Banzai.post_process with some common context options
def banzai_postprocess(html, context)
context.merge!(
current_user: (current_user if defined?(current_user)),
# RelativeLinkFilter
requested_path: @path,
project_wiki: @project_wiki,
ref: @ref
)
Banzai.post_process(html, context)
end
end end
class AbuseReport < ActiveRecord::Base class AbuseReport < ActiveRecord::Base
include CacheMarkdownField
cache_markdown_field :message, pipeline: :single_line
belongs_to :reporter, class_name: 'User' belongs_to :reporter, class_name: 'User'
belongs_to :user belongs_to :user
...@@ -7,6 +11,9 @@ class AbuseReport < ActiveRecord::Base ...@@ -7,6 +11,9 @@ class AbuseReport < ActiveRecord::Base
validates :message, presence: true validates :message, presence: true
validates :user_id, uniqueness: { message: 'has already been reported' } validates :user_id, uniqueness: { message: 'has already been reported' }
# For CacheMarkdownField
alias_method :author, :reporter
def remove_user(deleted_by:) def remove_user(deleted_by:)
user.block user.block
DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true) DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true)
......
class Appearance < ActiveRecord::Base class Appearance < ActiveRecord::Base
include CacheMarkdownField
cache_markdown_field :description
validates :title, presence: true validates :title, presence: true
validates :description, presence: true validates :description, presence: true
validates :logo, file_size: { maximum: 1.megabyte } validates :logo, file_size: { maximum: 1.megabyte }
......
class ApplicationSetting < ActiveRecord::Base class ApplicationSetting < ActiveRecord::Base
include CacheMarkdownField
include TokenAuthenticatable include TokenAuthenticatable
add_authentication_token_field :runners_registration_token add_authentication_token_field :runners_registration_token
add_authentication_token_field :health_check_access_token add_authentication_token_field :health_check_access_token
...@@ -17,6 +19,11 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -17,6 +19,11 @@ class ApplicationSetting < ActiveRecord::Base
serialize :domain_whitelist, Array serialize :domain_whitelist, Array
serialize :domain_blacklist, Array serialize :domain_blacklist, Array
cache_markdown_field :sign_in_text
cache_markdown_field :help_page_text
cache_markdown_field :shared_runners_text, pipeline: :plain_markdown
cache_markdown_field :after_sign_up_text
attr_accessor :domain_whitelist_raw, :domain_blacklist_raw attr_accessor :domain_whitelist_raw, :domain_blacklist_raw
validates :session_expire_delay, validates :session_expire_delay,
......
class BroadcastMessage < ActiveRecord::Base class BroadcastMessage < ActiveRecord::Base
include CacheMarkdownField
include Sortable include Sortable
cache_markdown_field :message, pipeline: :broadcast_message
validates :message, presence: true validates :message, presence: true
validates :starts_at, presence: true validates :starts_at, presence: true
validates :ends_at, presence: true validates :ends_at, presence: true
......
# This module takes care of updating cache columns for Markdown-containing
# fields. Use like this in the body of your class:
#
# include CacheMarkdownField
# cache_markdown_field :foo
# cache_markdown_field :bar
# cache_markdown_field :baz, pipeline: :single_line
#
# Corresponding foo_html, bar_html and baz_html fields should exist.
module CacheMarkdownField
# Knows about the relationship between markdown and html field names, and
# stores the rendering contexts for the latter
class FieldData
extend Forwardable
def initialize
@data = {}
end
def_delegators :@data, :[], :[]=
def_delegator :@data, :keys, :markdown_fields
def html_field(markdown_field)
"#{markdown_field}_html"
end
def html_fields
markdown_fields.map {|field| html_field(field) }
end
end
# Dynamic registries don't really work in Rails as it's not guaranteed that
# every class will be loaded, so hardcode the list.
CACHING_CLASSES = %w[
AbuseReport
Appearance
ApplicationSetting
BroadcastMessage
Issue
Label
MergeRequest
Milestone
Namespace
Note
Project
Release
Snippet
]
def self.caching_classes
CACHING_CLASSES.map(&:constantize)
end
extend ActiveSupport::Concern
included do
cattr_reader :cached_markdown_fields do
FieldData.new
end
# Returns the default Banzai render context for the cached markdown field.
def banzai_render_context(field)
raise ArgumentError.new("Unknown field: #{field.inspect}") unless
cached_markdown_fields.markdown_fields.include?(field)
# Always include a project key, or Banzai complains
project = self.project if self.respond_to?(:project)
context = cached_markdown_fields[field].merge(project: project)
# Banzai is less strict about authors, so don't always have an author key
context[:author] = self.author if self.respond_to?(:author)
context
end
# Allow callers to look up the cache field name, rather than hardcoding it
def markdown_cache_field_for(field)
raise ArgumentError.new("Unknown field: #{field}") unless
cached_markdown_fields.markdown_fields.include?(field)
cached_markdown_fields.html_field(field)
end
# Always exclude _html fields from attributes (including serialization).
# They contain unredacted HTML, which would be a security issue
alias_method :attributes_before_markdown_cache, :attributes
def attributes
attrs = attributes_before_markdown_cache
cached_markdown_fields.html_fields.each do |field|
attrs.delete(field)
end
attrs
end
end
class_methods do
private
# Specify that a field is markdown. Its rendered output will be cached in
# a corresponding _html field. Any custom rendering options may be provided
# as a context.
def cache_markdown_field(markdown_field, context = {})
raise "Add #{self} to CacheMarkdownField::CACHING_CLASSES" unless
CacheMarkdownField::CACHING_CLASSES.include?(self.to_s)
cached_markdown_fields[markdown_field] = context
html_field = cached_markdown_fields.html_field(markdown_field)
cache_method = "#{markdown_field}_cache_refresh".to_sym
invalidation_method = "#{html_field}_invalidated?".to_sym
define_method(cache_method) do
html = Banzai::Renderer.cacheless_render_field(self, markdown_field)
__send__("#{html_field}=", html)
true
end
# The HTML becomes invalid if any dependent fields change. For now, assume
# author and project invalidate the cache in all circumstances.
define_method(invalidation_method) do
changed_fields = changed_attributes.keys
invalidations = changed_fields & [markdown_field.to_s, "author", "project"]
!invalidations.empty?
end
before_save cache_method, if: invalidation_method
end
end
end
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
# #
module Issuable module Issuable
extend ActiveSupport::Concern extend ActiveSupport::Concern
include CacheMarkdownField
include Participable include Participable
include Mentionable include Mentionable
include Subscribable include Subscribable
...@@ -13,6 +14,9 @@ module Issuable ...@@ -13,6 +14,9 @@ module Issuable
include Awardable include Awardable
included do included do
cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description
belongs_to :author, class_name: "User" belongs_to :author, class_name: "User"
belongs_to :assignee, class_name: "User" belongs_to :assignee, class_name: "User"
belongs_to :updated_by, class_name: "User" belongs_to :updated_by, class_name: "User"
......
...@@ -4,6 +4,10 @@ class GlobalLabel ...@@ -4,6 +4,10 @@ class GlobalLabel
delegate :color, :description, to: :@first_label delegate :color, :description, to: :@first_label
def for_display
@first_label
end
def self.build_collection(labels) def self.build_collection(labels)
labels = labels.group_by(&:title) labels = labels.group_by(&:title)
......
...@@ -4,6 +4,10 @@ class GlobalMilestone ...@@ -4,6 +4,10 @@ class GlobalMilestone
attr_accessor :title, :milestones attr_accessor :title, :milestones
alias_attribute :name, :title alias_attribute :name, :title
def for_display
@first_milestone
end
def self.build_collection(milestones) def self.build_collection(milestones)
milestones = milestones.group_by(&:title) milestones = milestones.group_by(&:title)
...@@ -17,6 +21,7 @@ class GlobalMilestone ...@@ -17,6 +21,7 @@ class GlobalMilestone
@title = title @title = title
@name = title @name = title
@milestones = milestones @milestones = milestones
@first_milestone = milestones.find {|m| m.description.present? } || milestones.first
end end
def safe_title def safe_title
......
class Label < ActiveRecord::Base class Label < ActiveRecord::Base
include CacheMarkdownField
include Referable include Referable
include Subscribable include Subscribable
...@@ -8,6 +9,8 @@ class Label < ActiveRecord::Base ...@@ -8,6 +9,8 @@ class Label < ActiveRecord::Base
None = LabelStruct.new('No Label', 'No Label') None = LabelStruct.new('No Label', 'No Label')
Any = LabelStruct.new('Any Label', '') Any = LabelStruct.new('Any Label', '')
cache_markdown_field :description, pipeline: :single_line
DEFAULT_COLOR = '#428BCA' DEFAULT_COLOR = '#428BCA'
default_value_for :color, DEFAULT_COLOR default_value_for :color, DEFAULT_COLOR
......
...@@ -6,12 +6,16 @@ class Milestone < ActiveRecord::Base ...@@ -6,12 +6,16 @@ class Milestone < ActiveRecord::Base
Any = MilestoneStruct.new('Any Milestone', '', -1) Any = MilestoneStruct.new('Any Milestone', '', -1)
Upcoming = MilestoneStruct.new('Upcoming', '#upcoming', -2) Upcoming = MilestoneStruct.new('Upcoming', '#upcoming', -2)
include CacheMarkdownField
include InternalId include InternalId
include Sortable include Sortable
include Referable include Referable
include StripAttribute include StripAttribute
include Milestoneish include Milestoneish
cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description
belongs_to :project belongs_to :project
has_many :issues has_many :issues
has_many :labels, -> { distinct.reorder('labels.title') }, through: :issues has_many :labels, -> { distinct.reorder('labels.title') }, through: :issues
......
class Namespace < ActiveRecord::Base class Namespace < ActiveRecord::Base
acts_as_paranoid acts_as_paranoid
include CacheMarkdownField
include Sortable include Sortable
include Gitlab::ShellAdapter include Gitlab::ShellAdapter
cache_markdown_field :description, pipeline: :description
has_many :projects, dependent: :destroy has_many :projects, dependent: :destroy
belongs_to :owner, class_name: "User" belongs_to :owner, class_name: "User"
......
...@@ -6,6 +6,7 @@ class Project < ActiveRecord::Base ...@@ -6,6 +6,7 @@ class Project < ActiveRecord::Base
include Gitlab::VisibilityLevel include Gitlab::VisibilityLevel
include Gitlab::CurrentSettings include Gitlab::CurrentSettings
include AccessRequestable include AccessRequestable
include CacheMarkdownField
include Referable include Referable
include Sortable include Sortable
include AfterCommitQueue include AfterCommitQueue
...@@ -17,6 +18,8 @@ class Project < ActiveRecord::Base ...@@ -17,6 +18,8 @@ class Project < ActiveRecord::Base
UNKNOWN_IMPORT_URL = 'http://unknown.git' UNKNOWN_IMPORT_URL = 'http://unknown.git'
cache_markdown_field :description, pipeline: :description
delegate :feature_available?, :builds_enabled?, :wiki_enabled?, :merge_requests_enabled?, to: :project_feature, allow_nil: true delegate :feature_available?, :builds_enabled?, :wiki_enabled?, :merge_requests_enabled?, to: :project_feature, allow_nil: true
default_value_for :archived, false default_value_for :archived, false
......
class Release < ActiveRecord::Base class Release < ActiveRecord::Base
include CacheMarkdownField
cache_markdown_field :description
belongs_to :project belongs_to :project
validates :description, :project, :tag, presence: true validates :description, :project, :tag, presence: true
......
class Snippet < ActiveRecord::Base class Snippet < ActiveRecord::Base
include Gitlab::VisibilityLevel include Gitlab::VisibilityLevel
include Linguist::BlobHelper include Linguist::BlobHelper
include CacheMarkdownField
include Participable include Participable
include Referable include Referable
include Sortable include Sortable
include Awardable include Awardable
cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :content
# If file_name changes, it invalidates content
alias_method :default_content_html_invalidator, :content_html_invalidated?
def content_html_invalidated?
default_content_html_invalidator || file_name_changed?
end
default_value_for :visibility_level, Snippet::PRIVATE default_value_for :visibility_level, Snippet::PRIVATE
belongs_to :author, class_name: 'User' belongs_to :author, class_name: 'User'
......
# This worker clears all cache fields in the database, working in batches.
class ClearDatabaseCacheWorker
include Sidekiq::Worker
BATCH_SIZE = 1000
def perform
CacheMarkdownField.caching_classes.each do |kls|
fields = kls.cached_markdown_fields.html_fields
clear_cache_fields = fields.each_with_object({}) do |field, memo|
memo[field] = nil
end
Rails.logger.debug("Clearing Markdown cache for #{kls}: #{fields.inspect}")
kls.unscoped.in_batches(of: BATCH_SIZE) do |relation|
relation.update_all(clear_cache_fields)
end
end
nil
end
end
# Port ActiveRecord::Relation#in_batches from ActiveRecord 5.
# https://github.com/rails/rails/blob/ac027338e4a165273607dccee49a3d38bc836794/activerecord/lib/active_record/relation/batches.rb#L184
# TODO: this can be removed once we're using AR5.
raise "Vendored ActiveRecord 5 code! Delete #{__FILE__}!" if ActiveRecord::VERSION::MAJOR >= 5
module ActiveRecord
module Batches
# Differences from upstream: enumerator support was removed, and custom
# order/limit clauses are ignored without a warning.
def in_batches(of: 1000, start: nil, finish: nil, load: false)
raise "Must provide a block" unless block_given?
relation = self.reorder(batch_order).limit(of)
relation = relation.where(arel_table[primary_key].gteq(start)) if start
relation = relation.where(arel_table[primary_key].lteq(finish)) if finish
batch_relation = relation
loop do
if load
records = batch_relation.records
ids = records.map(&:id)
yielded_relation = self.where(primary_key => ids)
yielded_relation.load_records(records)
else
ids = batch_relation.pluck(primary_key)
yielded_relation = self.where(primary_key => ids)
end
break if ids.empty?
primary_key_offset = ids.last
raise ArgumentError.new("Primary key not included in the custom select clause") unless primary_key_offset
yield yielded_relation
break if ids.length < of
batch_relation = relation.where(arel_table[primary_key].gt(primary_key_offset))
end
end
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddMarkdownCacheColumns < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
COLUMNS = {
abuse_reports: [:message],
appearances: [:description],
application_settings: [
:sign_in_text,
:help_page_text,
:shared_runners_text,
:after_sign_up_text
],
broadcast_messages: [:message],
issues: [:title, :description],
labels: [:description],
merge_requests: [:title, :description],
milestones: [:title, :description],
namespaces: [:description],
notes: [:note],
projects: [:description],
releases: [:description],
snippets: [:title, :content],
}
def change
COLUMNS.each do |table, columns|
columns.each do |column|
add_column table, "#{column}_html", :text
end
end
end
end
...@@ -23,6 +23,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -23,6 +23,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.text "message" t.text "message"
t.datetime "created_at" t.datetime "created_at"
t.datetime "updated_at" t.datetime "updated_at"
t.text "message_html"
end end
create_table "appearances", force: :cascade do |t| create_table "appearances", force: :cascade do |t|
...@@ -32,6 +33,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -32,6 +33,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.string "logo" t.string "logo"
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.text "description_html"
end end
create_table "application_settings", force: :cascade do |t| create_table "application_settings", force: :cascade do |t|
...@@ -92,6 +94,10 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -92,6 +94,10 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.text "domain_blacklist" t.text "domain_blacklist"
t.boolean "koding_enabled" t.boolean "koding_enabled"
t.string "koding_url" t.string "koding_url"
t.text "sign_in_text_html"
t.text "help_page_text_html"
t.text "shared_runners_text_html"
t.text "after_sign_up_text_html"
end end
create_table "audit_events", force: :cascade do |t| create_table "audit_events", force: :cascade do |t|
...@@ -135,6 +141,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -135,6 +141,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.datetime "updated_at" t.datetime "updated_at"
t.string "color" t.string "color"
t.string "font" t.string "font"
t.text "message_html"
end end
create_table "ci_application_settings", force: :cascade do |t| create_table "ci_application_settings", force: :cascade do |t|
...@@ -469,6 +476,8 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -469,6 +476,8 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.date "due_date" t.date "due_date"
t.integer "moved_to_id" t.integer "moved_to_id"
t.integer "lock_version" t.integer "lock_version"
t.text "title_html"
t.text "description_html"
end end
add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
...@@ -517,6 +526,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -517,6 +526,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.boolean "template", default: false t.boolean "template", default: false
t.string "description" t.string "description"
t.integer "priority" t.integer "priority"
t.text "description_html"
end end
add_index "labels", ["priority"], name: "index_labels_on_priority", using: :btree add_index "labels", ["priority"], name: "index_labels_on_priority", using: :btree
...@@ -632,6 +642,8 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -632,6 +642,8 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.datetime "deleted_at" t.datetime "deleted_at"
t.string "in_progress_merge_commit_sha" t.string "in_progress_merge_commit_sha"
t.integer "lock_version" t.integer "lock_version"
t.text "title_html"
t.text "description_html"
end end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
...@@ -666,6 +678,8 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -666,6 +678,8 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.datetime "updated_at" t.datetime "updated_at"
t.string "state" t.string "state"
t.integer "iid" t.integer "iid"
t.text "title_html"
t.text "description_html"
end end
add_index "milestones", ["description"], name: "index_milestones_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} add_index "milestones", ["description"], name: "index_milestones_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
...@@ -689,6 +703,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -689,6 +703,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.boolean "request_access_enabled", default: true, null: false t.boolean "request_access_enabled", default: true, null: false
t.datetime "deleted_at" t.datetime "deleted_at"
t.boolean "lfs_enabled" t.boolean "lfs_enabled"
t.text "description_html"
end end
add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree
...@@ -721,6 +736,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -721,6 +736,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.integer "resolved_by_id" t.integer "resolved_by_id"
t.string "discussion_id" t.string "discussion_id"
t.string "original_discussion_id" t.string "original_discussion_id"
t.text "note_html"
end end
add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree
...@@ -872,6 +888,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -872,6 +888,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.boolean "request_access_enabled", default: true, null: false t.boolean "request_access_enabled", default: true, null: false
t.boolean "has_external_wiki" t.boolean "has_external_wiki"
t.boolean "lfs_enabled" t.boolean "lfs_enabled"
t.text "description_html"
end end
add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree
...@@ -922,6 +939,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -922,6 +939,7 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.integer "project_id" t.integer "project_id"
t.datetime "created_at" t.datetime "created_at"
t.datetime "updated_at" t.datetime "updated_at"
t.text "description_html"
end end
add_index "releases", ["project_id", "tag"], name: "index_releases_on_project_id_and_tag", using: :btree add_index "releases", ["project_id", "tag"], name: "index_releases_on_project_id_and_tag", using: :btree
...@@ -976,6 +994,8 @@ ActiveRecord::Schema.define(version: 20160926145521) do ...@@ -976,6 +994,8 @@ ActiveRecord::Schema.define(version: 20160926145521) do
t.string "file_name" t.string "file_name"
t.string "type" t.string "type"
t.integer "visibility_level", default: 0, null: false t.integer "visibility_level", default: 0, null: false
t.text "title_html"
t.text "content_html"
end end
add_index "snippets", ["author_id"], name: "index_snippets_on_author_id", using: :btree add_index "snippets", ["author_id"], name: "index_snippets_on_author_id", using: :btree
......
...@@ -3,6 +3,10 @@ module Banzai ...@@ -3,6 +3,10 @@ module Banzai
Renderer.render(text, context) Renderer.render(text, context)
end end
def self.render_field(object, field)
Renderer.render_field(object, field)
end
def self.cache_collection_render(texts_and_contexts) def self.cache_collection_render(texts_and_contexts)
Renderer.cache_collection_render(texts_and_contexts) Renderer.cache_collection_render(texts_and_contexts)
end end
......
...@@ -31,6 +31,34 @@ module Banzai ...@@ -31,6 +31,34 @@ module Banzai
end end
end end
# Convert a Markdown-containing field on an object into an HTML-safe String
# of HTML. This method is analogous to calling render(object.field), but it
# can cache the rendered HTML in the object, rather than Redis.
#
# The context to use is learned from the passed-in object by calling
# #banzai_render_context(field), and cannot be changed. Use #render, passing
# it the field text, if a custom rendering is needed. The generated context
# is returned along with the HTML.
def render_field(object, field)
html_field = object.markdown_cache_field_for(field)
html = object.__send__(html_field)
return html if html.present?
html = cacheless_render_field(object, field)
object.update_column(html_field, html) unless object.new_record? || object.destroyed?
html
end
# Same as +render_field+, but without consulting or updating the cache field
def cacheless_render_field(object, field)
text = object.__send__(field)
context = object.banzai_render_context(field)
cacheless_render(text, context)
end
# Perform multiple render from an Array of Markdown String into an # Perform multiple render from an Array of Markdown String into an
# Array of HTML-safe String of HTML. # Array of HTML-safe String of HTML.
# #
......
namespace :cache do namespace :cache do
CLEAR_BATCH_SIZE = 1000 # There seems to be no speedup when pushing beyond 1,000 namespace :clear do
REDIS_CLEAR_BATCH_SIZE = 1000 # There seems to be no speedup when pushing beyond 1,000
REDIS_SCAN_START_STOP = '0' # Magic value, see http://redis.io/commands/scan REDIS_SCAN_START_STOP = '0' # Magic value, see http://redis.io/commands/scan
desc "GitLab | Clear redis cache" desc "GitLab | Clear redis cache"
task :clear => :environment do task redis: :environment do
Gitlab::Redis.with do |redis| Gitlab::Redis.with do |redis|
cursor = REDIS_SCAN_START_STOP cursor = REDIS_SCAN_START_STOP
loop do loop do
cursor, keys = redis.scan( cursor, keys = redis.scan(
cursor, cursor,
match: "#{Gitlab::Redis::CACHE_NAMESPACE}*", match: "#{Gitlab::Redis::CACHE_NAMESPACE}*",
count: CLEAR_BATCH_SIZE count: REDIS_CLEAR_BATCH_SIZE
) )
redis.del(*keys) if keys.any? redis.del(*keys) if keys.any?
...@@ -19,4 +20,14 @@ namespace :cache do ...@@ -19,4 +20,14 @@ namespace :cache do
end end
end end
end end
desc "GitLab | Clear database cache (in the background)"
task db: :environment do
ClearDatabaseCacheWorker.perform_async
end
task all: [:db, :redis]
end
task clear: 'cache:clear:all'
end end
...@@ -9,6 +9,9 @@ FactoryGirl.define do ...@@ -9,6 +9,9 @@ FactoryGirl.define do
namespace namespace
creator creator
# Behaves differently to nil due to cache_has_external_issue_tracker
has_external_issue_tracker false
trait :public do trait :public do
visibility_level Gitlab::VisibilityLevel::PUBLIC visibility_level Gitlab::VisibilityLevel::PUBLIC
end end
...@@ -92,6 +95,8 @@ FactoryGirl.define do ...@@ -92,6 +95,8 @@ FactoryGirl.define do
end end
factory :redmine_project, parent: :project do factory :redmine_project, parent: :project do
has_external_issue_tracker true
after :create do |project| after :create do |project|
project.create_redmine_service( project.create_redmine_service(
active: true, active: true,
...@@ -105,6 +110,8 @@ FactoryGirl.define do ...@@ -105,6 +110,8 @@ FactoryGirl.define do
end end
factory :jira_project, parent: :project do factory :jira_project, parent: :project do
has_external_issue_tracker true
after :create do |project| after :create do |project|
project.create_jira_service( project.create_jira_service(
active: true, active: true,
......
require 'spec_helper'
describe Banzai::Renderer do
def expect_render(project = :project)
expected_context = { project: project }
expect(renderer).to receive(:cacheless_render) { :html }.with(:markdown, expected_context)
end
def expect_cache_update
expect(object).to receive(:update_column).with("field_html", :html)
end
def fake_object(*features)
markdown = :markdown if features.include?(:markdown)
html = :html if features.include?(:html)
object = double(
"object",
banzai_render_context: { project: :project },
field: markdown,
field_html: html
)
allow(object).to receive(:markdown_cache_field_for).with(:field).and_return("field_html")
allow(object).to receive(:new_record?).and_return(features.include?(:new))
allow(object).to receive(:destroyed?).and_return(features.include?(:destroyed))
object
end
describe "#render_field" do
let(:renderer) { Banzai::Renderer }
let(:subject) { renderer.render_field(object, :field) }
context "with an empty cache" do
let(:object) { fake_object(:markdown) }
it "caches and returns the result" do
expect_render
expect_cache_update
expect(subject).to eq(:html)
end
end
context "with a filled cache" do
let(:object) { fake_object(:markdown, :html) }
it "uses the cache" do
expect_render.never
expect_cache_update.never
should eq(:html)
end
end
context "new object" do
let(:object) { fake_object(:new, :markdown) }
it "doesn't cache the result" do
expect_render
expect_cache_update.never
expect(subject).to eq(:html)
end
end
context "destroyed object" do
let(:object) { fake_object(:destroyed, :markdown) }
it "doesn't cache the result" do
expect_render
expect_cache_update.never
expect(subject).to eq(:html)
end
end
end
end
...@@ -26,10 +26,11 @@ describe 'Import/Export attribute configuration', lib: true do ...@@ -26,10 +26,11 @@ describe 'Import/Export attribute configuration', lib: true do
it 'has no new columns' do it 'has no new columns' do
relation_names.each do |relation_name| relation_names.each do |relation_name|
relation_class = relation_class_for_name(relation_name) relation_class = relation_class_for_name(relation_name)
relation_attributes = relation_class.new.attributes.keys
expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class} to exist in safe_model_attributes" expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class} to exist in safe_model_attributes"
current_attributes = parsed_attributes(relation_name, relation_class.attribute_names) current_attributes = parsed_attributes(relation_name, relation_attributes)
safe_attributes = safe_model_attributes[relation_class.to_s] safe_attributes = safe_model_attributes[relation_class.to_s]
new_attributes = current_attributes - safe_attributes new_attributes = current_attributes - safe_attributes
......
...@@ -9,6 +9,10 @@ RSpec.describe AbuseReport, type: :model do ...@@ -9,6 +9,10 @@ RSpec.describe AbuseReport, type: :model do
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:reporter).class_name('User') } it { is_expected.to belong_to(:reporter).class_name('User') }
it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:user) }
it "aliases reporter to author" do
expect(subject.author).to be(subject.reporter)
end
end end
describe 'validations' do describe 'validations' do
......
require 'spec_helper'
describe CacheMarkdownField do
CacheMarkdownField::CACHING_CLASSES << "ThingWithMarkdownFields"
# The minimum necessary ActiveModel to test this concern
class ThingWithMarkdownFields
include ActiveModel::Model
include ActiveModel::Dirty
include ActiveModel::Serialization
class_attribute :attribute_names
self.attribute_names = []
def attributes
attribute_names.each_with_object({}) do |name, hsh|
hsh[name.to_s] = send(name)
end
end
extend ActiveModel::Callbacks
define_model_callbacks :save
include CacheMarkdownField
cache_markdown_field :foo
cache_markdown_field :baz, pipeline: :single_line
def self.add_attr(attr_name)
self.attribute_names += [attr_name]
define_attribute_methods(attr_name)
attr_reader(attr_name)
define_method("#{attr_name}=") do |val|
send("#{attr_name}_will_change!") unless val == send(attr_name)
instance_variable_set("@#{attr_name}", val)
end
end
[:foo, :foo_html, :bar, :baz, :baz_html].each do |attr_name|
add_attr(attr_name)
end
def initialize(*)
super
# Pretend new is load
clear_changes_information
end
def save
run_callbacks :save do
changes_applied
end
end
end
CacheMarkdownField::CACHING_CLASSES.delete("ThingWithMarkdownFields")
def thing_subclass(new_attr)
Class.new(ThingWithMarkdownFields) { add_attr(new_attr) }
end
let(:markdown) { "`Foo`" }
let(:html) { "<p><code>Foo</code></p>" }
let(:updated_markdown) { "`Bar`" }
let(:updated_html) { "<p><code>Bar</code></p>" }
subject { ThingWithMarkdownFields.new(foo: markdown, foo_html: html) }
describe ".attributes" do
it "excludes cache attributes" do
expect(thing_subclass(:qux).new.attributes.keys.sort).to eq(%w[bar baz foo qux])
end
end
describe ".cache_markdown_field" do
it "refuses to allow untracked classes" do
expect { thing_subclass(:qux).__send__(:cache_markdown_field, :qux) }.to raise_error(RuntimeError)
end
end
context "an unchanged markdown field" do
before do
subject.foo = subject.foo
subject.save
end
it { expect(subject.foo).to eq(markdown) }
it { expect(subject.foo_html).to eq(html) }
it { expect(subject.foo_html_changed?).not_to be_truthy }
end
context "a changed markdown field" do
before do
subject.foo = updated_markdown
subject.save
end
it { expect(subject.foo_html).to eq(updated_html) }
end
context "a non-markdown field changed" do
before do
subject.bar = "OK"
subject.save
end
it { expect(subject.bar).to eq("OK") }
it { expect(subject.foo).to eq(markdown) }
it { expect(subject.foo_html).to eq(html) }
end
describe '#banzai_render_context' do
it "sets project to nil if the object lacks a project" do
context = subject.banzai_render_context(:foo)
expect(context).to have_key(:project)
expect(context[:project]).to be_nil
end
it "excludes author if the object lacks an author" do
context = subject.banzai_render_context(:foo)
expect(context).not_to have_key(:author)
end
it "raises if the context for an unrecognised field is requested" do
expect{subject.banzai_render_context(:not_found)}.to raise_error(ArgumentError)
end
it "includes the pipeline" do
context = subject.banzai_render_context(:baz)
expect(context[:pipeline]).to eq(:single_line)
end
it "returns copies of the context template" do
template = subject.cached_markdown_fields[:baz]
copy = subject.banzai_render_context(:baz)
expect(copy).not_to be(template)
end
context "with a project" do
subject { thing_subclass(:project).new(foo: markdown, foo_html: html, project: :project) }
it "sets the project in the context" do
context = subject.banzai_render_context(:foo)
expect(context).to have_key(:project)
expect(context[:project]).to eq(:project)
end
it "invalidates the cache when project changes" do
subject.project = :new_project
allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html)
subject.save
expect(subject.foo_html).to eq(updated_html)
expect(subject.baz_html).to eq(updated_html)
end
end
context "with an author" do
subject { thing_subclass(:author).new(foo: markdown, foo_html: html, author: :author) }
it "sets the author in the context" do
context = subject.banzai_render_context(:foo)
expect(context).to have_key(:author)
expect(context[:author]).to eq(:author)
end
it "invalidates the cache when author changes" do
subject.author = :new_author
allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html)
subject.save
expect(subject.foo_html).to eq(updated_html)
expect(subject.baz_html).to eq(updated_html)
end
end
end
end
...@@ -518,7 +518,7 @@ describe Project, models: true do ...@@ -518,7 +518,7 @@ describe Project, models: true do
end end
describe '#cache_has_external_issue_tracker' do describe '#cache_has_external_issue_tracker' do
let(:project) { create(:project) } let(:project) { create(:project, has_external_issue_tracker: nil) }
it 'stores true if there is any external_issue_tracker' do it 'stores true if there is any external_issue_tracker' do
services = double(:service, external_issue_trackers: [RedmineService.new]) services = double(:service, external_issue_trackers: [RedmineService.new])
......
...@@ -238,7 +238,7 @@ describe Service, models: true do ...@@ -238,7 +238,7 @@ describe Service, models: true do
it "updates the has_external_issue_tracker boolean" do it "updates the has_external_issue_tracker boolean" do
expect do expect do
service.save! service.save!
end.to change { service.project.has_external_issue_tracker }.from(nil).to(true) end.to change { service.project.has_external_issue_tracker }.from(false).to(true)
end end
end end
......
...@@ -46,6 +46,13 @@ describe Snippet, models: true do ...@@ -46,6 +46,13 @@ describe Snippet, models: true do
end end
end end
describe "#content_html_invalidated?" do
let(:snippet) { create(:snippet, content: "md", content_html: "html", file_name: "foo.md") }
it "invalidates the HTML cache of content when the filename changes" do
expect { snippet.file_name = "foo.rb" }.to change { snippet.content_html_invalidated? }.from(false).to(true)
end
end
describe '.search' do describe '.search' do
let(:snippet) { create(:snippet) } let(:snippet) { create(:snippet) }
......
...@@ -232,7 +232,7 @@ describe API::API, api: true do ...@@ -232,7 +232,7 @@ describe API::API, api: true do
post api('/projects', user), project post api('/projects', user), project
project.each_pair do |k, v| project.each_pair do |k, v|
next if %i{ issues_enabled merge_requests_enabled wiki_enabled }.include?(k) next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k)
expect(json_response[k.to_s]).to eq(v) expect(json_response[k.to_s]).to eq(v)
end end
...@@ -360,7 +360,7 @@ describe API::API, api: true do ...@@ -360,7 +360,7 @@ describe API::API, api: true do
post api("/projects/user/#{user.id}", admin), project post api("/projects/user/#{user.id}", admin), project
project.each_pair do |k, v| project.each_pair do |k, v|
next if k == :path next if %i[has_external_issue_tracker path].include?(k)
expect(json_response[k.to_s]).to eq(v) expect(json_response[k.to_s]).to eq(v)
end end
end end
......
...@@ -448,6 +448,8 @@ describe GitPushService, services: true do ...@@ -448,6 +448,8 @@ describe GitPushService, services: true do
let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? } let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? }
before do before do
# project.create_jira_service doesn't seem to invalidate the cache here
project.has_external_issue_tracker = true
jira_service_settings jira_service_settings
WebMock.stub_request(:post, jira_api_transition_url) WebMock.stub_request(:post, jira_api_transition_url)
......
...@@ -60,7 +60,10 @@ describe MergeRequests::MergeService, services: true do ...@@ -60,7 +60,10 @@ describe MergeRequests::MergeService, services: true do
let(:jira_tracker) { project.create_jira_service } let(:jira_tracker) { project.create_jira_service }
before { jira_service_settings } before do
project.update_attributes!(has_external_issue_tracker: true)
jira_service_settings
end
it 'closes issues on JIRA issue tracker' do it 'closes issues on JIRA issue tracker' do
jira_issue = ExternalIssue.new('JIRA-123', project) jira_issue = ExternalIssue.new('JIRA-123', project)
......
...@@ -531,12 +531,12 @@ describe SystemNoteService, services: true do ...@@ -531,12 +531,12 @@ describe SystemNoteService, services: true do
include JiraServiceHelper include JiraServiceHelper
describe 'JIRA integration' do describe 'JIRA integration' do
let(:project) { create(:project) } let(:project) { create(:jira_project) }
let(:author) { create(:user) } let(:author) { create(:user) }
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:mergereq) { create(:merge_request, :simple, target_project: project, source_project: project) } let(:mergereq) { create(:merge_request, :simple, target_project: project, source_project: project) }
let(:jira_issue) { ExternalIssue.new("JIRA-1", project)} let(:jira_issue) { ExternalIssue.new("JIRA-1", project)}
let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? } let(:jira_tracker) { project.jira_service }
let(:commit) { project.commit } let(:commit) { project.commit }
context 'in JIRA issue tracker' do context 'in JIRA issue tracker' do
...@@ -545,10 +545,6 @@ describe SystemNoteService, services: true do ...@@ -545,10 +545,6 @@ describe SystemNoteService, services: true do
WebMock.stub_request(:post, jira_api_comment_url) WebMock.stub_request(:post, jira_api_comment_url)
end end
after do
jira_tracker.destroy!
end
describe "new reference" do describe "new reference" do
before do before do
WebMock.stub_request(:get, jira_api_comment_url).to_return(body: jira_issue_comments) WebMock.stub_request(:get, jira_api_comment_url).to_return(body: jira_issue_comments)
...@@ -578,10 +574,6 @@ describe SystemNoteService, services: true do ...@@ -578,10 +574,6 @@ describe SystemNoteService, services: true do
WebMock.stub_request(:get, jira_api_comment_url).to_return(body: jira_issue_comments) WebMock.stub_request(:get, jira_api_comment_url).to_return(body: jira_issue_comments)
end end
after do
jira_tracker.destroy!
end
subject { described_class.cross_reference(jira_issue, issue, author) } subject { described_class.cross_reference(jira_issue, issue, author) }
it { is_expected.to eq(jira_status_message) } it { is_expected.to eq(jira_status_message) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment