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

[service] service is not being deleted measures #6

Merged
merged 2 commits into from
Oct 18, 2018

Conversation

macejmic
Copy link
Contributor

@macejmic macejmic commented Sep 19, 2018

@Martouta
Copy link
Contributor

I don't think this is the problem 🤔

It is normal to thing the events have not been created because they are not in the DB anymore but that may be because we have a Janitor that removes old events (we had to do this because we ended up with a table of 130GB of events before doing this 😂 ):

JanitorWorker.perform_async

PurgeOldEventsWorker.perform_async

# frozen_string_literal: true
class PurgeOldEventsWorker
include Sidekiq::Worker
def perform
batch = Sidekiq::Batch.new
batch.description = 'Purge old events'
batch.jobs do
EventStore::Event.stale.find_each(&DeletePlainObjectWorker.method(:perform_later))
end
end
end

TTL = 1.week
scope :stale, -> { where.has { created_at <= TTL.ago } }

But I don't see where it could have done this action (through a controller) without using the state machine 😄 I suspect the customer used the API to delete the service, since it was used for everything else. Specially the part where they said this after they had already deleted the service:

A delete of the service id 2555417758966 results in 404 error

And if you create a test/events/services/service_scheduled_for_deletion_event_test.rb containing:

require 'test_helper'

class Services::ServiceDeletedEventTest < ActiveSupport::TestCase
  def setup
    @service = FactoryGirl.create(:service)
  end

  attr_reader :service

  def test_destroy_service_publishes_event
    service.mark_as_deleted!
    event = EventStore::Event.where(event_type: 'Services::ServiceScheduledForDeletionEvent').last
    assert_equal service.id, event.data[:service_id]
  end

  def test_after_commit_when_published
    DeletePlainObjectWorker.expects(:perform_later).with do |param|
      param.kind_of?(Service) && param.id == service.id
    end

    event = Services::ServiceScheduledForDeletionEvent.create(service)
    Rails.application.config.event_store.publish_event(event)
  end
end

And btw, it should be DeleteObjectHierarchyWorker instead of DeletePlainObjectWorker for efficiency, which is not the problem here, but since we are on it... 😂

About those tests, you will see the 1st one passes and the 2nd one fails. So I think the problem is the after_commit is being ignored.

So I think we do it with a subscriber instead of the after_commit, like we did with the ServiceTokenDeletedEvent:

  • A normal Event, without after_commit:
    # frozen_string_literal: true
    class ServiceTokenDeletedEvent < ServiceTokenRelatedEvent
    def self.create(service_token)
    new(
    id: service_token.id,
    service_id: service_token.service_id,
    value: service_token.value,
    metadata: {
    provider_id: service_token.account_id || service_token.tenant_id
    }
    )
    end
    end
  • The subscriber listening:
    subscribe_event(ServiceTokenEventSubscriber.new, ServiceTokenDeletedEvent)
  • And the subscriber itself doing the job when receives the event:
    # frozen_string_literal: true
    class ServiceTokenEventSubscriber < AfterCommitSubscriber
    def after_commit(event)
    case event
    when ServiceTokenDeletedEvent then BackendDeleteServiceTokenWorker.enqueue(event)
    else raise "Unknown event type #{event.class}"
    end
    end
    end

@philipgough philipgough mentioned this pull request Oct 2, 2018
6 tasks
@macejmic macejmic force-pushed the service-not-deleted branch 3 times, most recently from 87495fb to 0457254 Compare October 17, 2018 18:31
@macejmic macejmic changed the title [service] report when service not deleted with SM [service] service is not being deleted measures Oct 17, 2018
Martouta
Martouta previously approved these changes Oct 17, 2018
Copy link
Contributor

@Martouta Martouta left a comment

Choose a reason for hiding this comment

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

Awesome! 😍 Thanks! 🥇 💪 💃

Copy link
Contributor

@Martouta Martouta left a comment

Choose a reason for hiding this comment

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

Great job! 🥇 💪

@macejmic macejmic merged commit 135541a into master Oct 18, 2018
@macejmic macejmic deleted the service-not-deleted branch October 18, 2018 10:29
jlledom added a commit to jlledom/porta that referenced this pull request May 16, 2023
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.

2 participants