diff --git a/app/models/consumer_app.rb b/app/models/consumer_app.rb index ad16dfaa6..b883d69a1 100644 --- a/app/models/consumer_app.rb +++ b/app/models/consumer_app.rb @@ -49,11 +49,12 @@ class ConsumerApp < ApplicationRecord # [@forem/backend] `.where().first` is necessary because we use Redis data storage # https://github.com/rpush/rpush/wiki/Using-Redis#find_by_name-cannot-be-used-in-rpush-redis def clear_rpush_app + app_name = "#{app_bundle_was}.#{platform}" case ConsumerApp.platforms[platform_was] when Device::IOS - Rpush::Apns2::App.where(name: app_bundle_was).first&.destroy + Rpush::Apns2::App.where(name: app_name).first&.destroy when Device::ANDROID - Rpush::Gcm::App.where(name: app_bundle_was).first&.destroy + Rpush::Gcm::App.where(name: app_name).first&.destroy end # This prevents the `destroy` method to return true or false in a callback diff --git a/app/queries/consumer_apps/rpush_app_query.rb b/app/queries/consumer_apps/rpush_app_query.rb index 16414adfc..1607dbe2c 100644 --- a/app/queries/consumer_apps/rpush_app_query.rb +++ b/app/queries/consumer_apps/rpush_app_query.rb @@ -7,6 +7,7 @@ module ConsumerApps def initialize(app_bundle:, platform:) @app_bundle = app_bundle @platform = platform + @app_name = "#{app_bundle}.#{platform}" @consumer_app = ConsumerApps::FindOrCreateByQuery.call( app_bundle: @app_bundle, @@ -16,24 +17,24 @@ module ConsumerApps def call if consumer_app.android? - android_app = Rpush::Gcm::App.where(name: app_bundle).first + android_app = Rpush::Gcm::App.where(name: app_name).first android_app || recreate_android_app! elsif consumer_app.ios? - ios_app = Rpush::Apns2::App.where(name: app_bundle).first + ios_app = Rpush::Apns2::App.where(name: app_name).first ios_app || recreate_ios_app! end end private - attr_reader :app_bundle, :consumer_app + attr_reader :app_bundle, :app_name, :consumer_app, :platform def recreate_ios_app! # If the ConsumerApp doesn't have credentials there's no need to create it return if consumer_app.auth_credentials.blank? app = Rpush::Apns2::App.new - app.name = app_bundle + app.name = app_name app.certificate = consumer_app.auth_credentials.to_s.gsub("\\n", "\n") app.environment = Rails.env app.password = "" @@ -48,7 +49,7 @@ module ConsumerApps return if consumer_app.auth_credentials.blank? app = Rpush::Gcm::App.new - app.name = app_bundle + app.name = app_name app.auth_key = consumer_app.auth_credentials.to_s app.connections = 1 app.save! diff --git a/spec/queries/consumer_apps/rpush_app_query_spec.rb b/spec/queries/consumer_apps/rpush_app_query_spec.rb index 05aad1b7e..815b77e3e 100644 --- a/spec/queries/consumer_apps/rpush_app_query_spec.rb +++ b/spec/queries/consumer_apps/rpush_app_query_spec.rb @@ -1,19 +1,22 @@ require "rails_helper" RSpec.describe ConsumerApps::RpushAppQuery, type: :query do - let(:consumer_app) do + let(:ios_consumer_app) do ConsumerApps::FindOrCreateByQuery.call(app_bundle: ConsumerApp::FOREM_BUNDLE, platform: :ios) end + let(:android_consumer_app) do + ConsumerApps::FindOrCreateByQuery.call(app_bundle: ConsumerApp::FOREM_BUNDLE, platform: :android) + end describe "Rpush app" do it "is recreated after updating a ConsumerApp" do - mock_rpush(consumer_app) + mock_rpush(ios_consumer_app) # Fetch rpush app associated to the target - rpush_app = described_class.call(app_bundle: consumer_app.app_bundle, platform: :ios) + rpush_app = described_class.call(app_bundle: ios_consumer_app.app_bundle, platform: :ios) expect(rpush_app).to be_instance_of(Rpush::Apns2::App) - expect(rpush_app.name).to eq(consumer_app.app_bundle) + expect(rpush_app.name).to eq(ios_consumer_app.app_bundle) end it "returns nil if ConsumerApp is not operational" do @@ -26,5 +29,23 @@ RSpec.describe ConsumerApps::RpushAppQuery, type: :query do expect(bad_consumer_app.operational?).to be false expect(rpush_app).to be_nil end + + it "works when ConsumerApps have the same bundle but different platform" do + # This is a regression test to avoid conflicting `name` in either + # Rpush::Apns2::App or Rpush::Gcm::App to break the app + allow(ApplicationConfig).to receive(:[]).with("RPUSH_IOS_PEM").and_return("asdf123") + allow(ApplicationConfig).to receive(:[]).with("RPUSH_FCM_KEY").and_return("asdf123") + + mock_rpush(ios_consumer_app) + mock_rpush(android_consumer_app) + + # Fetch rpush app associated to the target + ios_rpush_app = described_class.call(app_bundle: ios_consumer_app.app_bundle, platform: :ios) + expect(ios_rpush_app).to be_instance_of(Rpush::Apns2::App) + + # Fetch rpush app associated to the target + android_rpush_app = described_class.call(app_bundle: android_consumer_app.app_bundle, platform: :android) + expect(android_rpush_app).to be_instance_of(Rpush::Gcm::App) + end end end