Skip to content
Merged
34 changes: 17 additions & 17 deletions app/jobs/file_upload_job.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class FileUploadJob < ApplicationJob
# Consider removing concurrency limits due to SolidQueue blocking issues
# or use a more specific key to avoid blocking all jobs for a language
limits_concurrency to: 3, key: ->(_language_id, content_id, _content_type) { "hard-limit" }
limits_concurrency to: 3, key: ->(_language_id, _content_id, _content_type) { "hard-limit" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't we keep the param names in parallel with the perform params? _language_id, _file_id, _provider_id?

To be clear, I don't fully grasp how the concurrency keys work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

When I run locally I get this error.
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This param is unused. However, number of parameters here should match the number of parameters when we call the job. I am trying to replace this with just *args

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm pushing this branch to staging so Manoj can review the files.


retry_on AzureFileShares::Errors::ApiError, wait: :exponentially_longer, attempts: 3
retry_on Timeout::Error, wait: :exponentially_longer, attempts: 2
Expand All @@ -12,34 +12,34 @@ class FileUploadJob < ApplicationJob
Rails.logger.error "Suggestion: Check provider names for invalid characters if Azure API errors"
end

def perform(language_id, content_id, content_type, share = ENV["AZURE_STORAGE_SHARE_NAME"])
def perform(language_id, file_id, provider_id = nil, share = ENV["AZURE_STORAGE_SHARE_NAME"])
@language = Language.find(language_id)
@processor = LanguageContentProcessor.new(language)
@share = share
@file_id = file_id.to_sym
@processor = LanguageContentProcessor.new(language)

send_provider_content(content_id) if content_type == "provider"
send_language_content(content_id.to_sym) if content_type == "file"
send_provider_content(provider_id) if provider_id.present?
send_language_content if provider_id.blank?
end

private

attr_reader :language, :processor, :share
attr_reader :language, :file_id, :share, :processor

def send_provider_content(provider_id)
provider = language.providers.find(provider_id)
return unless provider

processor.provider_files.each do |file|
FileWorker.new(
share:,
name: file.name[provider],
path: file.path,
file: file.content[provider],
).send
end
file = processor.provider_files[file_id]
return unless provider && file

FileWorker.new(
share:,
name: file.name[provider],
path: file.path,
file: file.content[provider],
).send
end

def send_language_content(file_id)
def send_language_content
file = processor.language_files[file_id]
return unless file

Expand Down
19 changes: 19 additions & 0 deletions app/services/csv_generator/base.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
class CsvGenerator::Base
def initialize(source, **args)
@source = source
@args = args
end

def perform
CSV.generate(row_sep: "\n") do |csv|
csv << headers
Expand All @@ -7,4 +12,18 @@ def perform
end
end
end

private

attr_reader :source, :args

def topics_collection
return source.topics if provider?

source.topics
end

def language = language? ? source : args.fetch(:language)
def language? = source.is_a?(Language)
def provider? = source.is_a?(Provider)
end
9 changes: 1 addition & 8 deletions app/services/csv_generator/files.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
class CsvGenerator::Files < CsvGenerator::Base
def initialize(language, **args)
@language = language
@args = args
end

private

attr_reader :language, :args

def headers
%w[FileID TopicID FileName FileType FileSize]
end

def scope
language.topics.active
topics_collection.active
.flat_map do |topic|
topic.documents.map do |doc|
[
Expand Down
9 changes: 1 addition & 8 deletions app/services/csv_generator/tag_details.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
class CsvGenerator::TagDetails < CsvGenerator::Base
def initialize(language, **args)
@language = language
@args = args
end

private

attr_reader :language, :args

def headers
%w[TagID Tag]
end

def scope
language.topics.active.includes(:tags)
topics_collection.active.includes(:tags)
.flat_map { |topic| topic.tags_on(language.code.to_sym) }
.uniq
.map do |tag|
Expand Down
9 changes: 1 addition & 8 deletions app/services/csv_generator/topic_authors.rb
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
class CsvGenerator::TopicAuthors < CsvGenerator::Base
def initialize(language, **args)
@language = language
@args = args
end

private

attr_reader :language, :args

def headers
%w[TopicID AuthorID]
end

def scope
language.topics.active.map { |topic| [ topic.id, 0 ] }
topics_collection.active.map { |topic| [ topic.id, 0 ] }
end
end
9 changes: 1 addition & 8 deletions app/services/csv_generator/topic_tags.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
class CsvGenerator::TopicTags < CsvGenerator::Base
def initialize(language, **args)
@language = language
@args = args
end

private

attr_reader :language, :args

def headers
%w[TopicID TagID]
end

def scope
language.topics.active.includes(:tags)
topics_collection.active.includes(:tags)
.flat_map do |topic|
topic.tags_on(language.code.to_sym).map do |tag|
[
Expand Down
9 changes: 1 addition & 8 deletions app/services/csv_generator/topics.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
class CsvGenerator::Topics < CsvGenerator::Base
def initialize(language, **args)
@language = language
@args = args
end

private

attr_reader :language, :args

def headers
%w[TopicID TopicName TopicVolume TopicIssue TopicYear TopicMonth ContentProvider]
end

def scope
language.topics.active.includes(:provider)
topics_collection.active.includes(:provider)
.map do |topic|
[
topic.id,
Expand Down
39 changes: 33 additions & 6 deletions app/services/language_content_processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,38 @@ def perform
# this is needed to avoid loading all files into memory at once
# Field 'name' is a lambda to allow dynamic naming based on the provider
def provider_files
[
FileToUpload.new(
{
single_provider: FileToUpload.new(
content: ->(provider) { XmlGenerator::SingleProvider.new(provider).perform },
name: ->(provider) { "#{language.file_storage_prefix}#{provider.name.parameterize}.xml" },
path: "#{language.file_storage_prefix}CMES-Pi/assets/XML",
),
]
files: FileToUpload.new(
content: ->(provider) { CsvGenerator::Files.new(provider).perform },
name: ->(provider) { "#{language.file_storage_prefix}#{provider.name.parameterize}-file.csv" },
path: "#{language.file_storage_prefix}CMES-v2/assets/csv",
),
topics: FileToUpload.new(
content: ->(provider) { CsvGenerator::Topics.new(provider).perform },
name: ->(provider) { "#{language.file_storage_prefix}#{provider.name.parameterize}-topic.csv" },
path: "#{language.file_storage_prefix}CMES-v2/assets/csv",
),
tag_details: FileToUpload.new(
content: ->(provider) { CsvGenerator::TagDetails.new(provider, language:).perform },
name: ->(provider) { "#{language.file_storage_prefix}#{provider.name.parameterize}-tag.csv" },
path: "#{language.file_storage_prefix}CMES-v2/assets/csv",
),
topic_tags: FileToUpload.new(
content: ->(provider) { CsvGenerator::TopicTags.new(provider, language:).perform },
name: ->(provider) { "#{language.file_storage_prefix}#{provider.name.parameterize}-topic-tag.csv" },
path: "#{language.file_storage_prefix}CMES-v2/assets/csv",
),
topic_authors: FileToUpload.new(
content: ->(provider) { CsvGenerator::TopicAuthors.new(provider).perform },
name: ->(provider) { "#{language.file_storage_prefix}#{provider.name.parameterize}-topic-author.csv" },
path: "#{language.file_storage_prefix}CMES-v2/assets/csv",
),
}
end

# Field 'content' is a lambda to allow lazy evaluation
Expand Down Expand Up @@ -65,7 +90,7 @@ def language_files
name: "#{language.file_storage_prefix}TopicTag.csv",
path: "#{language.file_storage_prefix}CMES-v2/assets/csv",
),
topic_authors: FileToUpload.new(
topic_authors: FileToUpload.new(
content: ->(language) { CsvGenerator::TopicAuthors.new(language).perform },
name: "#{language.file_storage_prefix}TopicAuthor.csv",
path: "#{language.file_storage_prefix}CMES-v2/assets/csv",
Expand All @@ -79,11 +104,13 @@ def language_files

def process_language_content!
language_files.keys.each do |file_id|
FileUploadJob.perform_later(language.id, file_id.to_s, "file")
FileUploadJob.perform_later(language.id, file_id.to_s)
end

language.providers.distinct.find_each do |provider|
FileUploadJob.perform_later(language.id, provider.id, "provider")
provider_files.keys.each do |file_id|
FileUploadJob.perform_later(language.id, file_id.to_s, provider.id)
end
end
end
end
41 changes: 15 additions & 26 deletions spec/jobs/file_upload_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
file: file.content[language],
)

described_class.perform_now(language.id, file_id.to_s, "file")
described_class.perform_now(language.id, file_id.to_s)
end
end
end
Expand All @@ -30,42 +30,31 @@
before { create(:topic, :tagged, language:, provider:) }

it "processes specific file" do
expect(FileWorker).to receive(:new).with(
share: ENV["AZURE_STORAGE_SHARE_NAME"],
name: "#{language.file_storage_prefix}test-provider.xml",
path: "#{language.file_storage_prefix}CMES-Pi/assets/XML",
file: XmlGenerator::SingleProvider.new(provider).perform,
)

described_class.perform_now(language.id, provider.id, "provider")
end

context "when provider name contains /" do
let(:provider) { create(:provider, name: "Test/Provider") }

it "replaces / with - in the file name" do
processor.provider_files.each do |file_id, file|
expect(FileWorker).to receive(:new).with(
share: ENV["AZURE_STORAGE_SHARE_NAME"],
name: "#{language.file_storage_prefix}test-provider.xml",
path: "#{language.file_storage_prefix}CMES-Pi/assets/XML",
file: XmlGenerator::SingleProvider.new(provider).perform,
name: file.name[provider],
path: file.path,
file: file.content[provider],
)

described_class.perform_now(language.id, provider.id, "provider")
described_class.perform_now(language.id, file_id.to_s, provider.id)
end
end

context "when provider name contains /" do
let(:provider) { create(:provider, name: "WHO/Guidelines") }
context "when provider name contains /" do
let(:provider) { create(:provider, name: "Test/Provider") }

it "replaces / with - in the file name" do
it "replaces / with - in the file name" do
processor.provider_files.each do |file_id, file|
expect(FileWorker).to receive(:new).with(
share: ENV["AZURE_STORAGE_SHARE_NAME"],
name: "#{language.file_storage_prefix}who-guidelines.xml",
path: "#{language.file_storage_prefix}CMES-Pi/assets/XML",
file: XmlGenerator::SingleProvider.new(provider).perform,
name: file.name[provider],
path: file.path,
file: file.content[provider],
)

described_class.perform_now(language.id, provider.id, "provider")
described_class.perform_now(language.id, file_id.to_s, provider.id)
end
end
end
Expand Down
13 changes: 12 additions & 1 deletion spec/services/csv_generator/files_spec.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
require "rails_helper"

RSpec.describe CsvGenerator::Files do
subject { described_class.new(language) }
subject { described_class.new(source, **args) }

let(:language) { create(:language) }
let(:source) { language }
let(:args) { {} }
let(:header) { "FileID,TopicID,FileName,FileType,FileSize\n" }

it "generates empty csv" do
Expand Down Expand Up @@ -41,6 +43,15 @@
expect(subject.perform).to eq(data)
end
end

context "when generated for provider" do
let(:source) { topic.provider }
let(:args) { { language: } }

it "generates csv with documents info" do
expect(subject.perform).to eq(data)
end
end
end

context "when topic exists but archived" do
Expand Down
13 changes: 12 additions & 1 deletion spec/services/csv_generator/tag_details_spec.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
require "rails_helper"

RSpec.describe CsvGenerator::TagDetails do
subject { described_class.new(language) }
subject { described_class.new(source, **args) }

let(:language) { create(:language) }
let(:source) { language }
let(:args) { {} }
let(:header) { "TagID,Tag\n" }

it "generates empty csv" do
Expand Down Expand Up @@ -39,6 +41,15 @@
expect(subject.perform).to eq(data)
end
end

context "when generated for provider" do
let(:source) { topic.provider }
let(:args) { { language: } }

it "generates csv with documents info" do
expect(subject.perform).to eq(data)
end
end
end

context "when topic exists but archived" do
Expand Down
Loading