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
Use a Class To Pass Around Repository Host Data #3338
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a05f64d
use a class to pass around repository host data
mikeyoung85 9cfed08
don't use private as var name
mikeyoung85 5d4dedc
remove some commented code
mikeyoung85 71337ed
add spec for github api call
mikeyoung85 6c312c2
change to struct and some fixes
mikeyoung85 b58405d
rename to create_from_data
mikeyoung85 6218ca5
rename to RawUpstreamData
mikeyoung85 5f85a01
move conversions into single factory
mikeyoung85 79ea005
add method to standardize full_name.downcase
mikeyoung85 f37b865
fix one more rename
mikeyoung85 7a6f86b
call method for var instead treating like hash
mikeyoung85 d60d572
use lower name and fix class rename
mikeyoung85 File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
# frozen_string_literal: true | ||
|
||
# This class is meant to be a facade over the raw upstream data coming | ||
# from the different repository hosts' repository data. It's main goal | ||
# is to standardize the output from each repository host into a concrete | ||
# set of data so we can make sure the raw data is being mapped to the same | ||
# schema within the Libraries.io models and code. | ||
RepositoryHost::RawUpstreamData = Struct.new( | ||
:archived, :default_branch, :description, :fork, :full_name, :has_issues, :has_wiki, :homepage, :host_type, | ||
:keywords, :language, :license, :name, :owner, :parent, :is_private, :repository_uuid, :scm, :repository_size, | ||
keyword_init: true | ||
) do | ||
def to_repository_attrs | ||
attrs = { | ||
default_branch: default_branch, | ||
description: description, | ||
full_name: full_name, | ||
has_issues: has_issues, | ||
has_wiki: has_wiki, | ||
homepage: homepage, | ||
host_type: host_type, | ||
keywords: keywords, | ||
language: language, | ||
license: formatted_license, | ||
name: name, | ||
private: is_private, | ||
scm: scm, | ||
size: repository_size, | ||
uuid: repository_uuid, | ||
} | ||
attrs[:source_name] = source_name if fork | ||
|
||
attrs | ||
end | ||
|
||
def formatted_license | ||
if license | ||
Project.format_license(license) | ||
end | ||
end | ||
|
||
def source_name | ||
parent[:full_name] if fork | ||
end | ||
|
||
def lower_name | ||
full_name&.downcase | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
# frozen_string_literal: true | ||
|
||
class RepositoryHost::RawUpstreamDataConverter | ||
def self.convert_from_github_api(upstream_repository_data_hash) | ||
RepositoryHost::RawUpstreamData.new( | ||
repository_uuid: upstream_repository_data_hash[:id], | ||
archived: upstream_repository_data_hash[:archived], | ||
default_branch: upstream_repository_data_hash[:default_branch], | ||
description: upstream_repository_data_hash[:description], | ||
fork: upstream_repository_data_hash[:fork], | ||
full_name: upstream_repository_data_hash[:full_name], | ||
has_issues: upstream_repository_data_hash[:has_issues], | ||
has_wiki: upstream_repository_data_hash[:has_wiki], | ||
homepage: upstream_repository_data_hash[:homepage], | ||
host_type: "GitHub", | ||
keywords: upstream_repository_data_hash[:topics], | ||
language: upstream_repository_data_hash[:language], | ||
license: upstream_repository_data_hash.dig(:license, :key), | ||
name: upstream_repository_data_hash[:name], | ||
owner: upstream_repository_data_hash[:owner], | ||
parent: upstream_repository_data_hash[:parent], | ||
is_private: upstream_repository_data_hash[:private], | ||
scm: "git", | ||
repository_size: upstream_repository_data_hash[:size] | ||
) | ||
end | ||
|
||
def self.convert_from_gitlab_api(api_project) | ||
RepositoryHost::RawUpstreamData.new( | ||
repository_uuid: api_project.id, | ||
description: api_project.description, | ||
name: api_project.name, | ||
default_branch: api_project.default_branch, | ||
archived: api_project.archived, | ||
host_type: "GitLab", | ||
full_name: api_project.path_with_namespace, | ||
owner: {}, | ||
fork: api_project.try(:forked_from_project).present?, | ||
has_issues: api_project.issues_enabled, | ||
has_wiki: api_project.wiki_enabled, | ||
scm: "git", | ||
is_private: api_project.visibility != "public", | ||
keywords: api_project.topics, | ||
parent: { | ||
full_name: api_project.try(:forked_from_project).try(:path_with_namespace), | ||
}, | ||
homepage: api_project.web_url, | ||
license: api_project.license.key, | ||
repository_size: 0, # locked to admins only?, | ||
language: nil # separate API endpoint that doesn't seem to be supported by the API gem we use | ||
) | ||
end | ||
|
||
def self.convert_from_bitbucket_api(api_project) | ||
RepositoryHost::RawUpstreamData.new( | ||
description: api_project.description, | ||
language: api_project.language, | ||
full_name: api_project.full_name, | ||
name: api_project.name, | ||
has_wiki: api_project.has_wiki, | ||
has_issues: api_project.has_issues, | ||
scm: api_project.scm, | ||
repository_uuid: api_project.uuid, | ||
host_type: "Bitbucket", | ||
owner: api_project.owner, | ||
homepage: api_project.website, | ||
fork: api_project.parent.present?, | ||
default_branch: api_project.fetch("mainbranch", {}).try(:fetch, "name", nil), | ||
is_private: api_project.is_private, | ||
repository_size: api_project[:size].to_f / 1000, | ||
parent: { | ||
full_name: api_project.fetch("parent", {}).try(:fetch, "full_name", nil), | ||
}, | ||
archived: false, | ||
keywords: [], | ||
license: nil | ||
) | ||
end | ||
end |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the error handling get standardized in here too? Saw a few
rescue *self::IGNORABLE_EXCEPTIONS nil
in the code.Alternatively, now that this is written out, it looks like there's some pretty tight coupling w/ the api return objects, so the convert_ methods could be moved to their respective methods in the repository_host/.rb files, as there doesn't seem to be a ton that's similar between the convert methods besides the object that's tying everything together, unless stuff like error handling is getting standardized. I was thinking there'd be more similarities between the objects being converted that there could be more extracted out and shared during the conversion process, like defaulting values/data validations/logging/etc.