Make constructor and interface of parsers consistent
Problem
We have multiple parsers in the codebase, with differing interfaces in the constructor and the parse!
methods.
All the parsers are instantiated through Parsers.fabricate!
def self.fabricate!(file_type, *args, **kwargs)
parsers.fetch(file_type.to_sym).new(*args, **kwargs)
rescue KeyError
raise ParserNotFoundError, "Cannot find any parser matching file type '#{file_type}'"
end
However, these have differing signatures in the #initialize
and #parse!
methods. Some takes in additional parameters in the constructor, while others take additional parameters in #parse!
. They all currently work because the factory method fabricate!
passes all args and keyword args, but it makes it difficult to identify what each parser needs.
app/models/ci/build.rb:957: Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, test_reports, job: self)
app/models/ci/build.rb:965: Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, accessibility_report)
app/models/ci/build.rb:973: Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, codequality_report)
app/models/ci/build.rb:981: ::Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, terraform_reports, artifact: report_artifact)
ee/app/models/ee/ci/build.rb:129: ::Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, license_scanning_report)
ee/app/models/ee/ci/build.rb:163: ::Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, metrics_report)
ee/app/models/ee/ci/build.rb:173: ::Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, requirements_report)
ee/app/models/ee/ci/build.rb:182: ::Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, report)
ee/app/models/ee/ci/build.rb:237: ::Gitlab::Ci::Parsers.fabricate!(security_report.type, blob, security_report, signatures_enabled: signatures_enabled).parse!
ee/app/models/ee/ci/job_artifact.rb:121: ::Gitlab::Ci::Parsers.fabricate!(file_type, blob, report, signatures_enabled: signatures_enabled, validate: validate).parse!
ee/lib/ee/gitlab/background_migration/populate_uuids_for_security_findings.rb:55: ::Gitlab::Ci::Parsers.fabricate!(file_type, blob, report).parse!
lib/gitlab/ci/reports/coverage_report_generator.rb:24: Gitlab::Ci::Parsers.fabricate!(file_type).parse!(
Proposal
Move all the required arguments into the constructor so that all Parsers have a consistent interface:
def parse!(blob, report, **kwargs)
# parses @blob into @report, using kwargs where necessary
end
So that Ci::Parsers.fabricate!(file_type)
only needs to accept the file type.
List of parsers to be updated:
Parser | Status |
---|---|
::Gitlab::Ci::Parsers::Security::Common | TODO |
Edited by Albert