Skip to content

Commit

Permalink
Merge pull request #91 from eagletmt/fix-name-zeitwerk
Browse files Browse the repository at this point in the history
Rename SNSSubscription to SnsSubscription for Zeitwerk
  • Loading branch information
eagletmt authored Jan 6, 2022
2 parents 15e8621 + 2d2b774 commit 5de51f1
Show file tree
Hide file tree
Showing 15 changed files with 41 additions and 39 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,5 @@ jobs:
ruby-version: ${{ matrix.ruby }}
bundler-cache: true
- run: bin/rails db:setup
- run: bin/rails zeitwerk:check
- run: bundle exec rspec
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## v2.x.x (xxxx-xx-xx)
### Changes
- Update Rails to 6.1
- For Zeitwerk, Barbeque::SNSSubscription and Barbeque::SNSSubscriptionService are renamed to Barbeque::SnsSubscription and Barbeque::SnsSubscriptionService respectively
- Support Ruby 3.0

## v2.8.0 (2021-12-23)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/barbeque/job_definitions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def update
def destroy
@job_definition = Barbeque::JobDefinition.find(params[:id])
@job_definition.sns_subscriptions.each do |sns_subscription|
Barbeque::SNSSubscriptionService.new.unsubscribe(sns_subscription)
Barbeque::SnsSubscriptionService.new.unsubscribe(sns_subscription)
end
@job_definition.destroy
redirect_to job_definitions_url, notice: 'Job definition was successfully destroyed.'
Expand Down
22 changes: 11 additions & 11 deletions app/controllers/barbeque/sns_subscriptions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
class Barbeque::SnsSubscriptionsController < Barbeque::ApplicationController
def index
@sns_subscriptions = Barbeque::SNSSubscription.all
@sns_subscriptions = Barbeque::SnsSubscription.all
end

def show
@sns_subscription = Barbeque::SNSSubscription.find(params[:id])
@sns_subscription = Barbeque::SnsSubscription.find(params[:id])
end

def new
@sns_topic_arns = fetch_sns_topic_arns
@sns_subscription = Barbeque::SNSSubscription.new
@sns_subscription = Barbeque::SnsSubscription.new
end

def edit
@sns_subscription = Barbeque::SNSSubscription.find(params[:id])
@sns_subscription = Barbeque::SnsSubscription.find(params[:id])
end

def create
@sns_subscription = Barbeque::SNSSubscription.new(params.require(:sns_subscription).permit(:topic_arn, :job_queue_id, :job_definition_id))
if Barbeque::SNSSubscriptionService.new.subscribe(@sns_subscription)
@sns_subscription = Barbeque::SnsSubscription.new(params.require(:sns_subscription).permit(:topic_arn, :job_queue_id, :job_definition_id))
if Barbeque::SnsSubscriptionService.new.subscribe(@sns_subscription)
redirect_to @sns_subscription, notice: 'SNS subscription was successfully created.'
else
@sns_topic_arns = fetch_sns_topic_arns
Expand All @@ -27,7 +27,7 @@ def create
end

def update
@sns_subscription = Barbeque::SNSSubscription.find(params[:id])
@sns_subscription = Barbeque::SnsSubscription.find(params[:id])
if @sns_subscription.update(params.require(:sns_subscription).permit(:job_definition_id))
redirect_to @sns_subscription, notice: 'SNS subscription was successfully updated.'
else
Expand All @@ -36,14 +36,14 @@ def update
end

def destroy
sns_subscription = Barbeque::SNSSubscription.find(params[:id])
Barbeque::SNSSubscriptionService.new.unsubscribe(sns_subscription)
sns_subscription = Barbeque::SnsSubscription.find(params[:id])
Barbeque::SnsSubscriptionService.new.unsubscribe(sns_subscription)
redirect_to sns_subscriptions_path, notice: 'SNS subscription was successfully destroyed.'
end

private

def fetch_sns_topic_arns
Barbeque::SNSSubscriptionService.sns_client.list_topics.flat_map(&:topics).map(&:topic_arn)
Barbeque::SnsSubscriptionService.sns_client.list_topics.flat_map(&:topics).map(&:topic_arn)
end
end
2 changes: 1 addition & 1 deletion app/models/barbeque/job_definition.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class Barbeque::JobDefinition < Barbeque::ApplicationRecord
belongs_to :app
has_many :job_executions
has_many :sns_subscriptions, class_name: 'SNSSubscription'
has_many :sns_subscriptions
has_one :slack_notification, dependent: :destroy
has_one :retry_config, dependent: :destroy

Expand Down
2 changes: 1 addition & 1 deletion app/models/barbeque/job_queue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class Barbeque::JobQueue < Barbeque::ApplicationRecord
SQS_NAME_MAX_LENGTH = 80

has_many :job_executions
has_many :sns_subscriptions, class_name: 'SNSSubscription', dependent: :destroy
has_many :sns_subscriptions, dependent: :destroy

# SQS queue allows [a-zA-Z0-9_-]+ as queue name. Its maximum length is 80.
validates :name, presence: true, uniqueness: true, format: /\A[a-zA-Z0-9_-]+\z/,
Expand Down
2 changes: 1 addition & 1 deletion app/models/barbeque/sns_subscription.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module Barbeque
class SNSSubscription < ApplicationRecord
class SnsSubscription < ApplicationRecord
belongs_to :job_queue
belongs_to :job_definition
has_one :app, through: :job_definition
Expand Down
16 changes: 8 additions & 8 deletions app/services/barbeque/sns_subscription_service.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'aws-sdk-sns'
require 'aws-sdk-sqs'

class Barbeque::SNSSubscriptionService
class Barbeque::SnsSubscriptionService
def self.sqs_client
@sqs_client ||= Aws::SQS::Client.new
end
Expand All @@ -10,7 +10,7 @@ def self.sns_client
@sns_client ||= Aws::SNS::Client.new
end

# @param [Barbeque::SNSSubscription] sns_subscription
# @param [Barbeque::SnsSubscription] sns_subscription
# @return [Boolean] `true` if succeeded to subscribe
def subscribe(sns_subscription)
if sns_subscription.valid?
Expand All @@ -31,7 +31,7 @@ def subscribe(sns_subscription)
end
end

# @param [Barbeque::SNSSubscription] sns_subscription
# @param [Barbeque::SnsSubscription] sns_subscription
def unsubscribe(sns_subscription)
sns_subscription.destroy
update_sqs_policy!(sns_subscription)
Expand All @@ -42,14 +42,14 @@ def unsubscribe(sns_subscription)
private

def sqs_client
Barbeque::SNSSubscriptionService.sqs_client
Barbeque::SnsSubscriptionService.sqs_client
end

def sns_client
Barbeque::SNSSubscriptionService.sns_client
Barbeque::SnsSubscriptionService.sns_client
end

# @param [Barbeque::SNSSubscription] sns_subscription
# @param [Barbeque::SnsSubscription] sns_subscription
def update_sqs_policy!(sns_subscription)
attrs = sqs_client.get_queue_attributes(
queue_url: sns_subscription.job_queue.queue_url,
Expand Down Expand Up @@ -90,7 +90,7 @@ def generate_policy(queue_arn:, topic_arns:)
}.to_json
end

# @param [Barbeque::SNSSubscription] sns_subscription
# @param [Barbeque::SnsSubscription] sns_subscription
def subscribe_topic!(sns_subscription)
sqs_attrs = sqs_client.get_queue_attributes(
queue_url: sns_subscription.job_queue.queue_url,
Expand All @@ -105,7 +105,7 @@ def subscribe_topic!(sns_subscription)
)
end

# @param [Barbeque::SNSSubscription] sns_subscription
# @param [Barbeque::SnsSubscription] sns_subscription
def unsubscribe_topic!(sns_subscription)
sqs_attrs = sqs_client.get_queue_attributes(
queue_url: sns_subscription.job_queue.queue_url,
Expand Down
2 changes: 1 addition & 1 deletion lib/barbeque/message/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class Notification < Base
attr_reader :application # [String] To specify `job_definitions.app.name`
attr_reader :job # [String] To specify `job_definitions.job`

# @param [Barneque::SNSSubscription] subscription
# @param [Barneque::SnsSubscription] subscription
# @return [Barbeque::Message::Notification]
def set_params_from_subscription(subscription)
@application = subscription.app.name
Expand Down
2 changes: 1 addition & 1 deletion lib/barbeque/message_handler/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def initialize(message:, message_queue:)
@message = message
@message_queue = message_queue

subscription = SNSSubscription.find_by!(topic_arn: @message.topic_arn, job_queue_id: @message_queue.job_queue.id)
subscription = SnsSubscription.find_by!(topic_arn: @message.topic_arn, job_queue_id: @message_queue.job_queue.id)
@message.set_params_from_subscription(subscription)
end
end
Expand Down
6 changes: 3 additions & 3 deletions spec/controllers/barbeque/job_definitions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,8 @@
let(:subscription_arn) { 'arn:aws:sns:ap-northeast-1:012345678912:barbeque-spec:01234567-89ab-cdef-0123-456789abcdef' }

before do
allow(Barbeque::SNSSubscriptionService).to receive(:sns_client).and_return(sns_client)
allow(Barbeque::SNSSubscriptionService).to receive(:sqs_client).and_return(sqs_client)
allow(Barbeque::SnsSubscriptionService).to receive(:sns_client).and_return(sns_client)
allow(Barbeque::SnsSubscriptionService).to receive(:sqs_client).and_return(sqs_client)

allow(sqs_client).to receive(:get_queue_attributes).
with(queue_url: sns_subscription.job_queue.queue_url, attribute_names: ['QueueArn']).
Expand All @@ -216,7 +216,7 @@
expect(sqs_client).to receive(:set_queue_attributes).with(queue_url: sns_subscription.job_queue.queue_url, attributes: { 'Policy' => '' })
expect(sns_client).to receive(:unsubscribe).with(subscription_arn: subscription_arn)
delete :destroy, params: { id: job_definition.id }
expect(Barbeque::SNSSubscription.all).to be_empty
expect(Barbeque::SnsSubscription.all).to be_empty
end
end
end
Expand Down
18 changes: 9 additions & 9 deletions spec/controllers/barbeque/sns_subscriptions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@
routes { Barbeque::Engine.routes }

before do
allow(Barbeque::SNSSubscriptionService).to receive(:sqs_client).and_return(sqs_client)
allow(Barbeque::SNSSubscriptionService).to receive(:sns_client).and_return(sns_client)
allow(Barbeque::SnsSubscriptionService).to receive(:sqs_client).and_return(sqs_client)
allow(Barbeque::SnsSubscriptionService).to receive(:sns_client).and_return(sns_client)
end

describe '#create' do
let(:job_queue) { create(:job_queue) }
let(:job_definition) { create(:job_definition) }
let(:job_definition) { create(:job_definition) }
let(:sqs_client) do
double(
'Aws::SQS::Client',
get_queue_attributes: get_queue_attrs_response,
set_queue_attributes: set_queue_attrs_response,
)
)
end
let(:get_queue_attrs_response) do
double(
Expand All @@ -40,7 +40,7 @@
double(
'Aws::SNS::Client',
subscribe: subscribe_response,
)
)
end
let(:subscribe_response) { double('Aws::SNS::Types::SubscribeResult') }

Expand All @@ -55,7 +55,7 @@
)
expect(sns_client).to receive(:subscribe).with(topic_arn: topic_arn, protocol: 'sqs', endpoint: queue_arn)
expect { post :create , params: { sns_subscription: attributes } }.
to change { Barbeque::SNSSubscription.count }.by(1)
to change { Barbeque::SnsSubscription.count }.by(1)
end

context 'with NotFound error' do
Expand Down Expand Up @@ -89,7 +89,7 @@
'Aws::SQS::Client',
get_queue_attributes: get_queue_attrs_response,
set_queue_attributes: set_queue_attrs_response,
)
)
end
let(:queue_arn) { "arn:aws:sqs:ap-northeast-1:123456789012:#{sns_subscription.job_queue.name}" }
let(:get_queue_attrs_response) { double('Aws::SQS::Types::GetQueueAttributesResult', attributes: { 'QueueArn' => queue_arn }) }
Expand All @@ -100,7 +100,7 @@
'Aws::SNS::Client',
list_subscriptions_by_topic: list_subscriptions_by_topic_response,
unsubscribe: unsubscribe_response
)
)
end
let(:list_subscriptions_by_topic_response) do
double(
Expand Down Expand Up @@ -128,7 +128,7 @@
}
)
expect { delete :destroy, params: { id: sns_subscription.id } }.
to change { Barbeque::SNSSubscription.count }.by(-1)
to change { Barbeque::SnsSubscription.count }.by(-1)
end
end
end
2 changes: 1 addition & 1 deletion spec/dummy/config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
module Dummy
class Application < Rails::Application
# Initialize configuration defaults for originally generated Rails version.
config.load_defaults 5.2
config.load_defaults 6.1

# Settings in config/environments/* take precedence over those specified here.
# Application configuration can go into files in config/initializers
Expand Down
Empty file added spec/dummy/config/storage.yml
Empty file.
2 changes: 1 addition & 1 deletion spec/factories/sns_subscription.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
FactoryBot.define do
factory :sns_subscription, class: Barbeque::SNSSubscription do
factory :sns_subscription, class: Barbeque::SnsSubscription do
sequence(:topic_arn) { |n| "arn:aws:sns:ap-northest-1:123456789012/Topic-#{n}" }
job_queue
job_definition
Expand Down

0 comments on commit 5de51f1

Please sign in to comment.