Skip to content

Commit

Permalink
[F] Add soft deletion to projects and texts
Browse files Browse the repository at this point in the history
* This adds logic to background the deletion of models that can take
  a while to actually delete because of the complexity of their
  association hierarchies.
* When soft-deleting a project, its texts will also be soft-deleted
  while the project is being deleted in the background, so that any
  lookups on the text will 404 as we'd like.
* Update controllers, filterable concern to use soft-deletion scopes
  when applicable
* Add TimestampScopes helper
* Add some missing inverse associations, minor idiomatic cleanup

Resolves MNFLD-937
  • Loading branch information
scryptmouse committed Dec 12, 2024
1 parent 7db05e5 commit a662c24
Show file tree
Hide file tree
Showing 19 changed files with 445 additions and 80 deletions.
1 change: 1 addition & 0 deletions api/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ gem "active_record_upsert", "~> 0.11.1"
gem "activerecord-wrapped_transaction", "~> 0.9.0"
gem "acts_as_list", "~> 1.1.0"
gem "acts-as-taggable-on", "~> 9.0"
gem "after_commit_everywhere", "~> 1.4.0"
gem "ahoy_matey", "~> 4.2.1"
# Project is now unmaintained. Fixing to 3.3 for now.
gem "authority", "~>3.3.0"
Expand Down
4 changes: 4 additions & 0 deletions api/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ GEM
activerecord (>= 4.2)
addressable (2.8.5)
public_suffix (>= 2.0.2, < 6.0)
after_commit_everywhere (1.4.0)
activerecord (>= 4.2)
activesupport
ahoy_matey (4.2.1)
activesupport (>= 5.2)
device_detector
Expand Down Expand Up @@ -813,6 +816,7 @@ DEPENDENCIES
activerecord-wrapped_transaction (~> 0.9.0)
acts-as-taggable-on (~> 9.0)
acts_as_list (~> 1.1.0)
after_commit_everywhere (~> 1.4.0)
ahoy_matey (~> 4.2.1)
authority (~> 3.3.0)
bagit (~> 0.4.3)
Expand Down
4 changes: 3 additions & 1 deletion api/app/controllers/api/v1/me/relationships/collectable.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module API
module V1
module Me
Expand Down Expand Up @@ -56,7 +58,7 @@ def define_resourceful_scope_for!(klass)
scope: current_user.public_send(association_name)
)
else
current_user.public_send(association_name)
current_user.public_send(association_name).existing
end
end
end
Expand Down
11 changes: 6 additions & 5 deletions api/app/controllers/api/v1/projects_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module API
module V1
# Projects controller
Expand Down Expand Up @@ -39,7 +41,7 @@ def update

def destroy
@project = load_and_authorize_project
@project.destroy
@project.async_destroy
end

protected
Expand All @@ -52,7 +54,7 @@ def includes
end

def scope_for_projects
Project.friendly
Project.existing.friendly
end

def scope_visibility
Expand All @@ -63,11 +65,10 @@ def scope_visibility
# apply both the read and update scopes, but then we have to join roles on both
# which I suspect is less efficient. Note that the with_update_ability is applied
# automatically through the filterable concern. -ZD
return Project.with_read_ability current_user unless project_filter_params&.dig(:with_update_ability)
return Project.existing.with_read_ability current_user unless project_filter_params&.dig(:with_update_ability)

Project.all
Project.existing.all
end

end
end
end
10 changes: 5 additions & 5 deletions api/app/controllers/api/v1/texts_controller.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
# frozen_string_literal: true

module API
module V1
# Texts controller
class TextsController < ApplicationController

resourceful! Text, authorize_options: { actions: { toggle_export_epub_v3: :update }, except: [:index, :show] } do
Text.all
Text.existing
end

# GET /texts
Expand Down Expand Up @@ -37,7 +38,7 @@ def update

def destroy
@text = load_and_authorize_text
@text.destroy
@text.async_destroy
end

def toggle_export_epub_v3
Expand All @@ -56,9 +57,8 @@ def includes
end

def scope_for_texts
Text.friendly
Text.existing.friendly
end

end
end
end
16 changes: 16 additions & 0 deletions api/app/jobs/soft_deletions/purge_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

module SoftDeletions
# @see SoftDeletable#async_destroy
class PurgeJob < ApplicationJob
queue_as :deletions

discard_on ActiveJob::DeserializationError, ActiveRecord::RecordNotFound, ActiveRecord::RecordNotDestroyed

# @param [SoftDeletable] record
# @return [void]
def perform(record)
record.destroy! if record.marked_for_purge?
end
end
end
2 changes: 1 addition & 1 deletion api/app/models/annotation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ def trigger_event_creation

class << self
def apply_filtering_loads
includes(:annotation_node, :creator, :membership_comments, :project, text_section: { text: %i[titles] })
super.includes(:annotation_node, :creator, :membership_comments, :project, text_section: { text: %i[titles] })
end

# @param [ReadingGroupMembership, String] rgm
Expand Down
6 changes: 6 additions & 0 deletions api/app/models/application_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ def merge_errors!(other)
end

class << self
# @see SoftDeletable
# @return [ActiveRecord::Relation]
def existing
all
end

def in_the_week_of(date)
where(created_at: date.to_week_range)
end
Expand Down
147 changes: 147 additions & 0 deletions api/app/models/concerns/soft_deletable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
# frozen_string_literal: true

module SoftDeletable
extend ActiveSupport::Concern

include Filterable

included do
define_model_callbacks :mark_for_purge, :soft_delete

scope :existing, -> { sans_deleted }
scope :only_deleted, -> { with_deleted.where.not(deleted_at: nil) }
scope :with_deleted, -> { unscope(where: :deleted_at) }
scope :sans_deleted, -> { with_deleted.where(deleted_at: nil) }

delegate :soft_deletable_associations, to: :class

before_mark_for_purge :validate_mark_for_purge!
after_mark_for_purge :enqueue_background_purge!
before_soft_delete :prevent_duplicate_soft_deletion!
after_soft_delete :scramble_deleted_slug!, if: :should_scramble_deleted_slug?
end

# @see SoftDeletions::PurgeJob
# @return [void]
def async_destroy
return if marked_for_purge?

soft_delete! unless soft_deleted?
mark_for_purge!
end

# @return [SoftDeletable, false]
def mark_for_purge
callbacks_result = with_lock do
run_callbacks :mark_for_purge do
mark_record_for_purge!
end
end

callbacks_result ? self : false
end

# @raise [ActiveRecord::RecordNotDestroyed]
# @return [SoftDeletable]
def mark_for_purge!
mark_for_purge or raise ActiveRecord::RecordNotDestroyed.new("Failed to mark the record for background purge", self)
end

def marked_for_purge?
marked_for_purge_at?
end

def should_scramble_deleted_slug?
has_attribute?(:slug) && slug.present?
end

def soft_deleted?
deleted_at?
end

# @return [SoftDeletable, false]
def soft_delete
callbacks_result = with_lock do
run_callbacks :soft_delete do
soft_destroy_record!
soft_destroy_dependent_records!
end
end

callbacks_result ? self : false
end

# @raise [ActiveRecord::RecordNotDestroyed]
# @return [SoftDeletable]
def soft_delete!
soft_delete or raise ActiveRecord::RecordNotDestroyed.new("Failed to destroy the record", self)
end

private

# @return [void]
def enqueue_background_purge!
AfterCommitEverywhere.after_commit do
SoftDeletions::PurgeJob.set(wait: 5.seconds).perform_later self
end
end

# @return [void]
def mark_record_for_purge!
update_column :marked_for_purge_at, Time.current
end

# @return [void]
def prevent_duplicate_soft_deletion!
throw :abort if soft_deleted?
end

# @return [void]
def scramble_deleted_slug!
new_slug = "#{slug}-deleted-#{SecureRandom.hex(6)}"

update_column :slug, new_slug
end

# @return [void]
def soft_destroy_dependent_records!
soft_deletable_associations.each do |association|
associated_records = public_send(association.name)

case association.options[:dependent]
when :destroy
associated_records.find_each(&:soft_destroy)
when :delete_all
associated_records.sans_deleted.update_all(deleted_at: Time.current)
end
end
end

# @return [void]
def soft_destroy_record!
update_column :deleted_at, Time.current
end

# @return [void]
def validate_mark_for_purge!
throw :abort if !soft_deleted? || marked_for_purge?
end

module ClassMethods
# @return [ActiveRecord::Relation]
def apply_filtering_loads
existing
end

def soft_deletable?
self < SoftDeletable
end

# @return [<ActiveRecord::Reflection::AssociationReflection>]
def soft_deletable_associations
reflect_on_all_associations.select do |assoc|
assoc.options[:dependent].present? && assoc.klass.try(:soft_deletable?)
end
end
end
end
28 changes: 28 additions & 0 deletions api/app/models/concerns/timestamp_scopes.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true

module TimestampScopes
extend ActiveSupport::Concern

included do
scope :by_recently_created, -> { order(created_at: :desc) }
scope :by_recently_updated, -> { order(updated_at: :desc) }

scope :in_recent_order, -> { by_recently_created }

scope :created_more_than, ->(time) { where(arel_table[:created_at].lteq(time)) }
scope :updated_more_than, ->(time) { where(arel_table[:updated_at].lteq(time)) }

scope :created_since, ->(time) { where(arel_table[:created_at].gteq(time)) }
scope :updated_since, ->(time) { where(arel_table[:updated_at].gteq(time)) }

scope :created_in_the_last, ->(duration) { created_since(duration.ago) }
scope :updated_in_the_last, ->(duration) { updated_since(duration.ago) }
end

module ClassMethods
# @return [ApplicationRecord, nil]
def latest
in_recent_order.first
end
end
end
14 changes: 7 additions & 7 deletions api/app/models/ingestion_source.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# frozen_string_literal: true

# Connects texts to resources that were sources for text sections during ingestion
#
# @see IngestionSourceUploader
class IngestionSource < ApplicationRecord
# Constants
TYPEAHEAD_ATTRIBUTES = [:display_name, :source_identifier].freeze

# Authorization
include Authority::Abilities
include Filterable
include SerializedAbilitiesFor
Expand All @@ -19,10 +19,10 @@ class IngestionSource < ApplicationRecord
manifold_has_attached_file :attachment, :resource

# Constants
KIND_COVER_IMAGE = "cover_image".freeze
KIND_NAVIGATION = "navigation".freeze
KIND_SECTION = "section".freeze
KIND_PUBLICATION_RESOURCE = "publication_resource".freeze
KIND_COVER_IMAGE = "cover_image"
KIND_NAVIGATION = "navigation"
KIND_SECTION = "section"
KIND_PUBLICATION_RESOURCE = "publication_resource"
ALLOWED_KINDS = [
KIND_COVER_IMAGE,
KIND_NAVIGATION,
Expand Down Expand Up @@ -58,7 +58,7 @@ class IngestionSource < ApplicationRecord
batch_size: 500)

# Associations
belongs_to :text
belongs_to :text, inverse_of: :ingestion_sources

# Delegations
delegate :project, to: :text
Expand Down
8 changes: 4 additions & 4 deletions api/app/models/project.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# frozen_string_literal: true

# The project model is the primary unit of Manifold.
class Project < ApplicationRecord
# Constants
TYPEAHEAD_ATTRIBUTES = [:title, :maker_names].freeze

# Concerns
include Authority::Abilities
include Collectable
include Entitleable
Expand All @@ -24,10 +24,10 @@ class Project < ApplicationRecord
include WithPermittedUsers
include Sluggable
include SearchIndexable
include SoftDeletable
include TimestampScopes
include WithConfigurableAvatar

# Magic

has_formatted_attributes :description, :subtitle, :image_credits
has_formatted_attributes :restricted_access_body, include_wrap: false
has_formatted_attributes :title, include_wrap: false
Expand Down
Loading

0 comments on commit a662c24

Please sign in to comment.