diff --git a/Gemfile b/Gemfile index fcf77487e865a4f56791fb50b5da5ba960423bd1..938f5579d86c2f5125034170887fcdf8c6affc61 100644 --- a/Gemfile +++ b/Gemfile @@ -192,6 +192,7 @@ gem 'hamlit', '~> 3.0.0', feature_category: :shared # Files attachments gem 'carrierwave', '~> 1.3', feature_category: :shared gem 'mini_magick', '~> 4.12', feature_category: :shared +gem 'marcel', '~> 1.0.4', feature_category: :shared # PDF generation gem 'prawn', feature_category: :vulnerability_management diff --git a/Gemfile.checksum b/Gemfile.checksum index ac0264bf73c6f2577098bcc50fbdc1583b43d0d8..2764ad5baabe6dd7b9c27e17eb6c4e496fb97e94 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -386,7 +386,7 @@ {"name":"lru_redux","version":"1.1.0","platform":"ruby","checksum":"ee71d0ccab164c51de146c27b480a68b3631d5b4297b8ffe8eda1c72de87affb"}, {"name":"lumberjack","version":"1.2.7","platform":"ruby","checksum":"a5c6aae6b4234f1420dbcd80b23e3bca0817bd239440dde097ebe3fa63c63b1f"}, {"name":"mail","version":"2.8.1","platform":"ruby","checksum":"ec3b9fadcf2b3755c78785cb17bc9a0ca9ee9857108a64b6f5cfc9c0b5bfc9ad"}, -{"name":"marcel","version":"1.0.2","platform":"ruby","checksum":"a013b677ef46cbcb49fd5c59b3d35803d2ee04dd75d8bfdc43533fc5a31f7e4e"}, +{"name":"marcel","version":"1.0.4","platform":"ruby","checksum":"0d5649feb64b8f19f3d3468b96c680bae9746335d02194270287868a661516a4"}, {"name":"marginalia","version":"1.11.1","platform":"ruby","checksum":"cb63212ab63e42746e27595e912cb20408a1a28bcd0edde55d15b7c45fa289cf"}, {"name":"matrix","version":"0.4.2","platform":"ruby","checksum":"71083ccbd67a14a43bfa78d3e4dc0f4b503b9cc18e5b4b1d686dc0f9ef7c4cc0"}, {"name":"memory_profiler","version":"1.1.0","platform":"ruby","checksum":"79a17df7980a140c83c469785905409d3027ca614c42c086089d128b805aa8f8"}, diff --git a/Gemfile.lock b/Gemfile.lock index b987a09bb106d3a11c681aab7b97bc219aad34d4..aa462222cc3519e9153db8936d3f83e8b1ad4dbd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1172,7 +1172,7 @@ GEM net-imap net-pop net-smtp - marcel (1.0.2) + marcel (1.0.4) marginalia (1.11.1) actionpack (>= 5.2) activerecord (>= 5.2) @@ -2253,6 +2253,7 @@ DEPENDENCIES lru_redux mail (= 2.8.1) mail-smtp_pool (~> 0.1.0)! + marcel (~> 1.0.4) marginalia (~> 1.11.1) memory_profiler (~> 1.0) microsoft_graph_mailer (~> 0.1.0)! diff --git a/Gemfile.next.checksum b/Gemfile.next.checksum index 8f35ba49d0b27492ae0a9f4df7e2dfd109d16362..7ff229b6c3b81d6b41b936c7027d6d4ef1232fb6 100644 --- a/Gemfile.next.checksum +++ b/Gemfile.next.checksum @@ -2,17 +2,17 @@ {"name":"CFPropertyList","version":"3.0.7","platform":"ruby","checksum":"c45721614aca8d5eb6fa216f2ec28ec38de1a94505e9766a20e98745492c3c4c"}, {"name":"RedCloth","version":"4.3.4","platform":"ruby","checksum":"5231b2fdd91a933915cba330e5fd1a74025e77b56f57b7404c7191ebf2812297"}, {"name":"acme-client","version":"2.0.25","platform":"ruby","checksum":"e0bba7b9f785fd9ffe0933f8733ca81357ac46e4a979cb4f84806ab88fee0f31"}, -{"name":"actioncable","version":"7.2.2.1","platform":"ruby","checksum":"5b3b885075a80767d63cbf2b586cbf82466a241675b7985233f957abb01bffb4"}, -{"name":"actionmailbox","version":"7.2.2.1","platform":"ruby","checksum":"896a47c2520f4507c75dde67c6ea1f5eec3a041fe7bfbf3568c4e0149a080e25"}, -{"name":"actionmailer","version":"7.2.2.1","platform":"ruby","checksum":"b02ae523c32c8ad762d4db941e76f3c108c106030132247ee7a7b8c86bc7b21f"}, -{"name":"actionpack","version":"7.2.2.1","platform":"ruby","checksum":"17b2160a7bcbd5a569d06b1ae54a4bb5ccc7ba0815d73ff5768100a79dc1f734"}, -{"name":"actiontext","version":"7.2.2.1","platform":"ruby","checksum":"f369cee41a6674b697bf9257d917a3dce575a2c89935af437b432d6737a3f0d6"}, -{"name":"actionview","version":"7.2.2.1","platform":"ruby","checksum":"69fc880cf3d8b1baf21b048cf7bb68f1eef08760ff8104d7d60a6a1be8b359a5"}, -{"name":"activejob","version":"7.2.2.1","platform":"ruby","checksum":"f2f95a8573b394aa4f7c24843f0c4a6065c073a5c64d6f15ecd98d98c2c23e5b"}, -{"name":"activemodel","version":"7.2.2.1","platform":"ruby","checksum":"8398861f9ee2c4671a8357ab39e9b38a045fd656f6685a3dd5890c2419dbfdaf"}, -{"name":"activerecord","version":"7.2.2.1","platform":"ruby","checksum":"79a31f71c32d5138717c2104e0ff105f5d82922247c85bdca144f2720e67fab9"}, -{"name":"activestorage","version":"7.2.2.1","platform":"ruby","checksum":"b4ec35ff94d4d6656ee6952ce439c3f80e249552d49fd2d3996ee53880c5525f"}, -{"name":"activesupport","version":"7.2.2.1","platform":"ruby","checksum":"842bcbf8a92977f80fb4750661a237cf5dd4fdd442066b3c35e88afb488647f5"}, +{"name":"actioncable","version":"7.2.2.2","platform":"ruby","checksum":"3d957adc9d1d2ddb5ac8ed8791dc35b273c722f2dca2644f415bd24ba64c7425"}, +{"name":"actionmailbox","version":"7.2.2.2","platform":"ruby","checksum":"7f784a3685a877ca0937bf28f989bbafbffd0f3966d724e41bf9319f881487ef"}, +{"name":"actionmailer","version":"7.2.2.2","platform":"ruby","checksum":"d37c8ab094f3b73c3fbcbbf41d2e31fc15b607178569a58057ed878c2063a6e6"}, +{"name":"actionpack","version":"7.2.2.2","platform":"ruby","checksum":"ccd2f96ffb378060dd02e86318bca3faae1ecf483603a525fabfd84197c86a6e"}, +{"name":"actiontext","version":"7.2.2.2","platform":"ruby","checksum":"8e80623cf206f077f4b671846ba74b0cb154b2a306a6569d3c4b3deb22e2b701"}, +{"name":"actionview","version":"7.2.2.2","platform":"ruby","checksum":"5bf67e9716fbd159f09cbc8cf87f4813d3e8725f0197a7321910e9dc8c165b07"}, +{"name":"activejob","version":"7.2.2.2","platform":"ruby","checksum":"e706383862084022d531eee64f74ac4b5fd751f160a7138d3a3c1018b2facb55"}, +{"name":"activemodel","version":"7.2.2.2","platform":"ruby","checksum":"6898b91af028d725729f65d8e0f6ccfef5993e085ed70d5b93c42ba1bf7384dd"}, +{"name":"activerecord","version":"7.2.2.2","platform":"ruby","checksum":"e6b1e1499018f1c3ffd9f7828a8560588da1f5bd85dc2b7a95e49c5467cda800"}, +{"name":"activestorage","version":"7.2.2.2","platform":"ruby","checksum":"0b28d0c191b03162e83d3bf6875c3692ab48abd1e371bb0b428136dd8509ae66"}, +{"name":"activesupport","version":"7.2.2.2","platform":"ruby","checksum":"c54e84bb3d9027f1f372fb8f68203538fcfe0d5ff42801774c03974daa15bef0"}, {"name":"addressable","version":"2.8.7","platform":"ruby","checksum":"462986537cf3735ab5f3c0f557f14155d778f4b43ea4f485a9deb9c8f7c58232"}, {"name":"aes_key_wrap","version":"1.1.0","platform":"ruby","checksum":"b935f4756b37375895db45669e79dfcdc0f7901e12d4e08974d5540c8e0776a5"}, {"name":"akismet","version":"3.0.0","platform":"ruby","checksum":"74991b8e3d3257eeea996b47069abb8da2006c84a144255123e8dffd1c86b230"}, @@ -386,7 +386,7 @@ {"name":"lru_redux","version":"1.1.0","platform":"ruby","checksum":"ee71d0ccab164c51de146c27b480a68b3631d5b4297b8ffe8eda1c72de87affb"}, {"name":"lumberjack","version":"1.2.7","platform":"ruby","checksum":"a5c6aae6b4234f1420dbcd80b23e3bca0817bd239440dde097ebe3fa63c63b1f"}, {"name":"mail","version":"2.8.1","platform":"ruby","checksum":"ec3b9fadcf2b3755c78785cb17bc9a0ca9ee9857108a64b6f5cfc9c0b5bfc9ad"}, -{"name":"marcel","version":"1.0.2","platform":"ruby","checksum":"a013b677ef46cbcb49fd5c59b3d35803d2ee04dd75d8bfdc43533fc5a31f7e4e"}, +{"name":"marcel","version":"1.0.4","platform":"ruby","checksum":"0d5649feb64b8f19f3d3468b96c680bae9746335d02194270287868a661516a4"}, {"name":"marginalia","version":"1.11.1","platform":"ruby","checksum":"cb63212ab63e42746e27595e912cb20408a1a28bcd0edde55d15b7c45fa289cf"}, {"name":"matrix","version":"0.4.2","platform":"ruby","checksum":"71083ccbd67a14a43bfa78d3e4dc0f4b503b9cc18e5b4b1d686dc0f9ef7c4cc0"}, {"name":"memory_profiler","version":"1.1.0","platform":"ruby","checksum":"79a17df7980a140c83c469785905409d3027ca614c42c086089d128b805aa8f8"}, @@ -572,12 +572,12 @@ {"name":"rack-test","version":"2.1.0","platform":"ruby","checksum":"0c61fc61904049d691922ea4bb99e28004ed3f43aa5cfd495024cc345f125dfb"}, {"name":"rack-timeout","version":"0.7.0","platform":"ruby","checksum":"757337e9793cca999bb73a61fe2a7d4280aa9eefbaf787ce3b98d860749c87d9"}, {"name":"rackup","version":"1.0.1","platform":"ruby","checksum":"ba86604a28989fe1043bff20d819b360944ca08156406812dca6742b24b3c249"}, -{"name":"rails","version":"7.2.2.1","platform":"ruby","checksum":"aedb1604b40f4e43b5e8066e5a1aa34dae02c33aa9669b21fd4497d0f8c9bb40"}, +{"name":"rails","version":"7.2.2.2","platform":"ruby","checksum":"f38ff86c5e6905e5d30a762575f92ddad5b60346f5acfc282b0e22dbb36eca97"}, {"name":"rails-controller-testing","version":"1.0.5","platform":"ruby","checksum":"741448db59366073e86fc965ba403f881c636b79a2c39a48d0486f2607182e94"}, {"name":"rails-dom-testing","version":"2.2.0","platform":"ruby","checksum":"e515712e48df1f687a1d7c380fd7b07b8558faa26464474da64183a7426fa93b"}, {"name":"rails-html-sanitizer","version":"1.6.1","platform":"ruby","checksum":"e3d2fb10339f03b802e39c7f6cac28c54fd404d3f65ae39c31cca9d150c5cbf0"}, {"name":"rails-i18n","version":"7.0.10","platform":"ruby","checksum":"efae16e0ac28c0f42e98555c8db1327d69ab02058c8b535e0933cb106dd931ca"}, -{"name":"railties","version":"7.2.2.1","platform":"ruby","checksum":"e3f11bf116dd6d0d874522843ccc70ec0f89fbfed3e9c2ee48a4778cd042fe1f"}, +{"name":"railties","version":"7.2.2.2","platform":"ruby","checksum":"457506d29b7be2c5644b5bd4740edaf42bcefa52b50f9bed519a9b10ec9069e0"}, {"name":"rainbow","version":"3.1.1","platform":"ruby","checksum":"039491aa3a89f42efa1d6dec2fc4e62ede96eb6acd95e52f1ad581182b79bc6a"}, {"name":"rake","version":"13.0.6","platform":"ruby","checksum":"5ce4bf5037b4196c24ac62834d8db1ce175470391026bd9e557d669beeb19097"}, {"name":"rake-compiler-dock","version":"1.9.1","platform":"ruby","checksum":"e73720a29aba9c114728ce39cc0d8eef69ba61d88e7978c57bac171724cd4d53"}, diff --git a/Gemfile.next.lock b/Gemfile.next.lock index 3bdc10fe62b5115ac249ddf067aa0ea0425704a0..019fb36cbeeadcb62ca63c5e32385e7d3617d0e1 100644 --- a/Gemfile.next.lock +++ b/Gemfile.next.lock @@ -214,29 +214,29 @@ GEM base64 (~> 0.2) faraday (>= 1.0, < 3.0.0) faraday-retry (>= 1.0, < 3.0.0) - actioncable (7.2.2.1) - actionpack (= 7.2.2.1) - activesupport (= 7.2.2.1) + actioncable (7.2.2.2) + actionpack (= 7.2.2.2) + activesupport (= 7.2.2.2) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (7.2.2.1) - actionpack (= 7.2.2.1) - activejob (= 7.2.2.1) - activerecord (= 7.2.2.1) - activestorage (= 7.2.2.1) - activesupport (= 7.2.2.1) + actionmailbox (7.2.2.2) + actionpack (= 7.2.2.2) + activejob (= 7.2.2.2) + activerecord (= 7.2.2.2) + activestorage (= 7.2.2.2) + activesupport (= 7.2.2.2) mail (>= 2.8.0) - actionmailer (7.2.2.1) - actionpack (= 7.2.2.1) - actionview (= 7.2.2.1) - activejob (= 7.2.2.1) - activesupport (= 7.2.2.1) + actionmailer (7.2.2.2) + actionpack (= 7.2.2.2) + actionview (= 7.2.2.2) + activejob (= 7.2.2.2) + activesupport (= 7.2.2.2) mail (>= 2.8.0) rails-dom-testing (~> 2.2) - actionpack (7.2.2.1) - actionview (= 7.2.2.1) - activesupport (= 7.2.2.1) + actionpack (7.2.2.2) + actionview (= 7.2.2.2) + activesupport (= 7.2.2.2) nokogiri (>= 1.8.5) racc rack (>= 2.2.4, < 3.2) @@ -245,35 +245,35 @@ GEM rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actiontext (7.2.2.1) - actionpack (= 7.2.2.1) - activerecord (= 7.2.2.1) - activestorage (= 7.2.2.1) - activesupport (= 7.2.2.1) + actiontext (7.2.2.2) + actionpack (= 7.2.2.2) + activerecord (= 7.2.2.2) + activestorage (= 7.2.2.2) + activesupport (= 7.2.2.2) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.2.2.1) - activesupport (= 7.2.2.1) + actionview (7.2.2.2) + activesupport (= 7.2.2.2) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - activejob (7.2.2.1) - activesupport (= 7.2.2.1) + activejob (7.2.2.2) + activesupport (= 7.2.2.2) globalid (>= 0.3.6) - activemodel (7.2.2.1) - activesupport (= 7.2.2.1) - activerecord (7.2.2.1) - activemodel (= 7.2.2.1) - activesupport (= 7.2.2.1) + activemodel (7.2.2.2) + activesupport (= 7.2.2.2) + activerecord (7.2.2.2) + activemodel (= 7.2.2.2) + activesupport (= 7.2.2.2) timeout (>= 0.4.0) - activestorage (7.2.2.1) - actionpack (= 7.2.2.1) - activejob (= 7.2.2.1) - activerecord (= 7.2.2.1) - activesupport (= 7.2.2.1) + activestorage (7.2.2.2) + actionpack (= 7.2.2.2) + activejob (= 7.2.2.2) + activerecord (= 7.2.2.2) + activesupport (= 7.2.2.2) marcel (~> 1.0) - activesupport (7.2.2.1) + activesupport (7.2.2.2) base64 benchmark (>= 0.3) bigdecimal @@ -1166,7 +1166,7 @@ GEM net-imap net-pop net-smtp - marcel (1.0.2) + marcel (1.0.4) marginalia (1.11.1) actionpack (>= 5.2) activerecord (>= 5.2) @@ -1545,20 +1545,20 @@ GEM rackup (1.0.1) rack (< 3) webrick - rails (7.2.2.1) - actioncable (= 7.2.2.1) - actionmailbox (= 7.2.2.1) - actionmailer (= 7.2.2.1) - actionpack (= 7.2.2.1) - actiontext (= 7.2.2.1) - actionview (= 7.2.2.1) - activejob (= 7.2.2.1) - activemodel (= 7.2.2.1) - activerecord (= 7.2.2.1) - activestorage (= 7.2.2.1) - activesupport (= 7.2.2.1) + rails (7.2.2.2) + actioncable (= 7.2.2.2) + actionmailbox (= 7.2.2.2) + actionmailer (= 7.2.2.2) + actionpack (= 7.2.2.2) + actiontext (= 7.2.2.2) + actionview (= 7.2.2.2) + activejob (= 7.2.2.2) + activemodel (= 7.2.2.2) + activerecord (= 7.2.2.2) + activestorage (= 7.2.2.2) + activesupport (= 7.2.2.2) bundler (>= 1.15.0) - railties (= 7.2.2.1) + railties (= 7.2.2.2) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) actionview (>= 5.0.1.rc1) @@ -1573,9 +1573,9 @@ GEM rails-i18n (7.0.10) i18n (>= 0.7, < 2) railties (>= 6.0.0, < 8) - railties (7.2.2.1) - actionpack (= 7.2.2.1) - activesupport (= 7.2.2.1) + railties (7.2.2.2) + actionpack (= 7.2.2.2) + activesupport (= 7.2.2.2) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) @@ -2248,6 +2248,7 @@ DEPENDENCIES lru_redux mail (= 2.8.1) mail-smtp_pool (~> 0.1.0)! + marcel (~> 1.0.4) marginalia (~> 1.11.1) memory_profiler (~> 1.0) microsoft_graph_mailer (~> 0.1.0)! diff --git a/doc/user/project/import/github.md b/doc/user/project/import/github.md index 69207e06d3e1a2679d554ebe41d4c87eede2f003..183b0aa145eb24c8bd21debb948b001f826c60b1 100644 --- a/doc/user/project/import/github.md +++ b/doc/user/project/import/github.md @@ -88,8 +88,11 @@ GitHub Enterprise does not require a public email address, so you might have to - GitHub pull request comments (known as diff notes in GitLab) created before 2017 are imported in separate threads. This occurs because of a limitation of the GitHub API that doesn't include `in_reply_to_id` for comments before 2017. -- Because of a [known issue](https://gitlab.com/gitlab-org/gitlab/-/issues/424400), Markdown attachments from - repositories on GitHub Enterprise Server instances aren't imported. +- [In GitLab 18.3 and earlier](https://gitlab.com/gitlab-org/gitlab/-/issues/424400), Markdown attachments + from repositories on GitHub Enterprise Server instances are not imported. + [In GitLab 18.4 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/553386): + - Only video and image files in Markdown attachments are imported. + - Other file attachments are not imported. - Because of a [known issue](https://gitlab.com/gitlab-org/gitlab/-/issues/418800), when importing projects that used [GitHub auto-merge](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/automatically-merging-a-pull-request), the imported project in GitLab can have merge commits labeled `unverified` if the commit was signed with the GitHub internal GPG key. - GitLab [can't import](https://gitlab.com/gitlab-org/gitlab/-/issues/424046) GitHub Markdown image attachments that diff --git a/lib/gitlab/github_import/attachments_downloader.rb b/lib/gitlab/github_import/attachments_downloader.rb index 3f17f3fbe94201e617c2761500b89da17288e143..ae03ee276304ebc3f4d27760618456fa32e6a485 100644 --- a/lib/gitlab/github_import/attachments_downloader.rb +++ b/lib/gitlab/github_import/attachments_downloader.rb @@ -14,12 +14,15 @@ class AttachmentsDownloader TMP_DIR = File.join(Dir.tmpdir, 'github_attachments').freeze SUPPORTED_VIDEO_MEDIA_TYPES = %w[mov mp4 webm].freeze - attr_reader :file_url, :filename, :file_size_limit, :options + attr_reader :file_url, :filename, :file_size_limit, :options, :web_endpoint - def initialize(file_url, options: {}, file_size_limit: DEFAULT_FILE_SIZE_LIMIT) + def initialize( + file_url, options: {}, file_size_limit: DEFAULT_FILE_SIZE_LIMIT, + web_endpoint: ::Octokit::Default.web_endpoint) @file_url = file_url @options = options @file_size_limit = file_size_limit + @web_endpoint = web_endpoint filename = URI(file_url).path.split('/').last @filename = ensure_filename_size(filename) @@ -30,9 +33,12 @@ def perform download_url = get_assets_download_redirection_url + # skip download for GHE server urls that redirect to a login url + return file_url if download_url.include?("login?return_to=") + parsed_file_name = File.basename(URI.parse(download_url).path) - # if the file is a media type, we update both the @filename and @filepath with the filetype extension + # if the file has a video filetype extension, we update both the @filename and @filepath with the filetype ext. if parsed_file_name.end_with?(*SUPPORTED_VIDEO_MEDIA_TYPES.map { |ext| ".#{ext}" }) @filename = ensure_filename_size(parsed_file_name) add_extension_to_file_path(filename) @@ -72,7 +78,7 @@ def get_assets_download_redirection_url end def github_assets_url_regex - %r{#{Regexp.escape(::Gitlab::GithubImport::MarkdownText.github_url)}/.*/(assets|files)/} + %r{#{Regexp.escape(web_endpoint)}/.*/(assets|files)/} end def download_from(url) diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb index 2bfc9ba49caebef812dd5bd41ba4a0c28b05ac54..2928d0f30f7385014055081ad1b31e9d785a6ebc 100644 --- a/lib/gitlab/github_import/client.rb +++ b/lib/gitlab/github_import/client.rb @@ -243,17 +243,25 @@ def rate_limiting_enabled? end def api_endpoint - formatted_host || custom_api_endpoint || default_api_endpoint + formatted_api_endpoint || custom_api_endpoint || default_api_endpoint end def web_endpoint - formatted_host || custom_api_endpoint || ::Octokit::Default.web_endpoint + formatted_web_endpoint || custom_web_endpoint || ::Octokit::Default.web_endpoint end def custom_api_endpoint github_omniauth_provider.dig('args', 'client_options', 'site') end + def custom_web_endpoint + return unless custom_api_endpoint + + uri = URI.parse(custom_api_endpoint) + uri.path = '' + uri.to_s.chomp('/') + end + def default_api_endpoint OmniAuth::Strategies::GitHub.default_options[:client_options][:site] || ::Octokit::Default.api_endpoint end @@ -282,8 +290,8 @@ def request_count_counter private - def formatted_host - strong_memoize(:formatted_host) do + def formatted_api_endpoint + strong_memoize(:formatted_api_endpoint) do next if @host.nil? uri = URI.parse(@host) @@ -292,6 +300,16 @@ def formatted_host end end + def formatted_web_endpoint + strong_memoize(:formatted_web_endpoint) do + next if @host.nil? + + uri = URI.parse(@host) + uri.path = '' + uri.to_s + end + end + def api_endpoint_host strong_memoize(:api_endpoint_host) do URI.parse(api_endpoint).host diff --git a/lib/gitlab/github_import/importer/attachments/base_importer.rb b/lib/gitlab/github_import/importer/attachments/base_importer.rb index 9dfa6bb2ff6fa58276bbd4d75d29b8e781a9719d..7fa6b6c8e1de7820364ec4485dba5d5b0b49309b 100644 --- a/lib/gitlab/github_import/importer/attachments/base_importer.rb +++ b/lib/gitlab/github_import/importer/attachments/base_importer.rb @@ -52,7 +52,10 @@ def object_representation(object) end def has_attachments?(object) - object_representation(object).has_attachments? + note_text = object_representation(object) + return false if note_text.text.blank? + + Gitlab::GithubImport::MarkdownText.fetch_attachments(note_text.text, client.web_endpoint).present? end end end diff --git a/lib/gitlab/github_import/importer/diff_note_importer.rb b/lib/gitlab/github_import/importer/diff_note_importer.rb index 223a286774825a29cff0a937c6a4b0c7da6f4910..4c09ce675d8594072d1f1ee4cdcc5330e000af16 100644 --- a/lib/gitlab/github_import/importer/diff_note_importer.rb +++ b/lib/gitlab/github_import/importer/diff_note_importer.rb @@ -110,7 +110,7 @@ def import_with_diff_note end def note_body - @note_body ||= MarkdownText.format(note.note, note.author, author_found) + @note_body ||= MarkdownText.format(note.note, note.author, author_found, project: project, client: client) end def author diff --git a/lib/gitlab/github_import/importer/issue_importer.rb b/lib/gitlab/github_import/importer/issue_importer.rb index f68ee9e916ffc74776d4044afa558ae1b4ee13f0..53518b868bdbbf9d7466267058c01f382ac5b578 100644 --- a/lib/gitlab/github_import/importer/issue_importer.rb +++ b/lib/gitlab/github_import/importer/issue_importer.rb @@ -51,7 +51,8 @@ def issue_assignee_map def create_issue author_id, author_found = user_finder.author_id_for(issue) - description = MarkdownText.format(issue.description, issue.author, author_found, project: project) + description = MarkdownText.format(issue.description, issue.author, author_found, project: project, + client: client) assignee_ids = issue_assignee_map.keys attributes = { diff --git a/lib/gitlab/github_import/importer/milestones_importer.rb b/lib/gitlab/github_import/importer/milestones_importer.rb index 79e6a848a5dc6f74c54c176664a9fff46546100a..9471526fe91a036865a2dbdc0d37cbf035592fd1 100644 --- a/lib/gitlab/github_import/importer/milestones_importer.rb +++ b/lib/gitlab/github_import/importer/milestones_importer.rb @@ -46,7 +46,7 @@ def build_attributes(milestone) end def description_for(milestone) - MarkdownText.format(milestone[:description], project: project) + MarkdownText.format(milestone[:description], project: project, client: client) end def state_for(milestone) diff --git a/lib/gitlab/github_import/importer/note_attachments_importer.rb b/lib/gitlab/github_import/importer/note_attachments_importer.rb index 8f829634ed74d14ef4e3a89849b10db49ce92045..c7139c0abf9b4895eb85f111251e5d7fefa81207 100644 --- a/lib/gitlab/github_import/importer/note_attachments_importer.rb +++ b/lib/gitlab/github_import/importer/note_attachments_importer.rb @@ -4,7 +4,7 @@ module Gitlab module GithubImport module Importer class NoteAttachmentsImporter - attr_reader :note_text, :project, :client + attr_reader :note_text, :project, :client, :web_endpoint SUPPORTED_RECORD_TYPES = [::Release.name, ::Issue.name, ::MergeRequest.name, ::Note.name].freeze @@ -15,12 +15,14 @@ def initialize(note_text, project, client) @note_text = note_text @project = project @client = client + @web_endpoint = client.web_endpoint end def execute - return unless note_text.has_attachments? + attachments = Gitlab::GithubImport::MarkdownText.fetch_attachments(note_text.text, web_endpoint) + return if attachments.blank? - new_text = note_text.attachments.reduce(note_text.text) do |text, attachment| + new_text = attachments.reduce(note_text.text) do |text, attachment| new_url = gitlab_attachment_link(attachment) # we need to update video media file links with the correct markdown format @@ -57,7 +59,7 @@ def supported_video_media_types # From: https://github.com/login/test-import-attachments-source/blob/main/example.md # To: https://gitlab.com/login/test-import-attachments-target/-/blob/main/example.md def convert_project_content_link(attachment_url, import_source) - path_without_domain = attachment_url.gsub(::Gitlab::GithubImport::MarkdownText.github_url, '') + path_without_domain = attachment_url.gsub(web_endpoint, '') path_without_import_source = path_without_domain.gsub(import_source, '').delete_prefix('/') path_with_blob_prefix = "/-#{path_without_import_source}" @@ -67,9 +69,23 @@ def convert_project_content_link(attachment_url, import_source) # in: an instance of Gitlab::GithubImport::Markdown::Attachment # out: gitlab attachment markdown url def download_attachment(attachment) - downloader = ::Gitlab::GithubImport::AttachmentsDownloader.new(attachment.url, options: options) + downloader = ::Gitlab::GithubImport::AttachmentsDownloader.new(attachment.url, options: options, + web_endpoint: web_endpoint) + file = downloader.perform + + # for ghe imports skip file attachments + # in these cases the AttachmentsDownloader returns the redirect url + # so we return the original attachment.url + if web_endpoint != ::Octokit::Default.web_endpoint && file.is_a?(String) && file.starts_with?(github_file_url_regex) # rubocop:disable Layout/LineLength,Lint/RedundantCopDisableDirective -- minor infraction + return attachment.url + end + + # for ghe imports check on filetype to add ext to video attachments + file = update_ghe_video_path(file) unless web_endpoint == ::Octokit::Default.web_endpoint + uploader = UploadService.new(project, file, FileUploader).execute + uploader.to_h[:url] end @@ -108,6 +124,30 @@ def record_column_for_type(record_type) :note end end + + def update_ghe_video_path(file) + filepath = file.path + return file if File.extname(filepath).present? + + mime_type = Marcel::MimeType.for(Pathname.new(filepath)) + return file unless mime_type.start_with?('video/') + + extension = case mime_type + when 'video/quicktime' then '.mov' + when 'video/mp4' then '.mp4' + when 'video/webm' then '.webm' + end + + return file if extension.blank? + + new_path = "#{filepath}.#{extension}" + FileUtils.mv(filepath, new_path) + File.open(new_path, 'rb') + end + + def github_file_url_regex + %r{#{Regexp.escape(web_endpoint)}/.*/files/} + end end end end diff --git a/lib/gitlab/github_import/importer/note_importer.rb b/lib/gitlab/github_import/importer/note_importer.rb index d8858b5bd61aa95be3528bc7e3bb1d1afdd5d979..875eae18d4e146ea213e2b89fc22c01cf364f63c 100644 --- a/lib/gitlab/github_import/importer/note_importer.rb +++ b/lib/gitlab/github_import/importer/note_importer.rb @@ -61,7 +61,7 @@ def find_noteable_id private def note_body(author_found) - MarkdownText.format(note.note, note.author, author_found, project: project) + MarkdownText.format(note.note, note.author, author_found, project: project, client: client) end end end diff --git a/lib/gitlab/github_import/importer/pull_request_importer.rb b/lib/gitlab/github_import/importer/pull_request_importer.rb index 4aba28729756bbb9c8a436e47b541eaa00543fad..0cff758b0e5a22be6fe3959d8da9826560dec129 100644 --- a/lib/gitlab/github_import/importer/pull_request_importer.rb +++ b/lib/gitlab/github_import/importer/pull_request_importer.rb @@ -54,7 +54,7 @@ def execute def create_merge_request author_id, author_found = user_finder.author_id_for(pull_request) - description = MarkdownText.format(pull_request.description, pull_request.author, author_found, project: project) + description = MarkdownText.format(pull_request.description, pull_request.author, author_found, project: project, client: client) attributes = { iid: pull_request.iid, diff --git a/lib/gitlab/github_import/importer/pull_requests/review_importer.rb b/lib/gitlab/github_import/importer/pull_requests/review_importer.rb index 9ac29dd8b91fae16e0c235710ff40e3c48cbac10..508b98efaa2d29b5c25c377fa56cd67cb5613aa6 100644 --- a/lib/gitlab/github_import/importer/pull_requests/review_importer.rb +++ b/lib/gitlab/github_import/importer/pull_requests/review_importer.rb @@ -52,7 +52,9 @@ def add_complementary_review_note!(author_id) note_body = MarkdownText.format( review_note_content, - review.author + review.author, + project: project, + client: client ) add_note!(author_id, note_body) diff --git a/lib/gitlab/github_import/importer/releases_importer.rb b/lib/gitlab/github_import/importer/releases_importer.rb index 2b93c691e2b4d48f940035a8ec81fa3fcdb054bb..6608445884d5fbee00a55b2ad305f35e8dbbd4d4 100644 --- a/lib/gitlab/github_import/importer/releases_importer.rb +++ b/lib/gitlab/github_import/importer/releases_importer.rb @@ -75,7 +75,7 @@ def description_for(release) description = release[:body].presence || "Release for tag #{release[:tag_name]}" user = map_github_user_info(release) - MarkdownText.format(description, user, user[:id], project: project) + MarkdownText.format(description, user, user[:id], project: project, client: client) end def object_type diff --git a/lib/gitlab/github_import/markdown/attachment.rb b/lib/gitlab/github_import/markdown/attachment.rb index 3b1af797f380009ff4aa6a763879d6f6fb1783a3..7bb9a1cdd16783e6a7cbe570fcd3b7b885ebfd46 100644 --- a/lib/gitlab/github_import/markdown/attachment.rb +++ b/lib/gitlab/github_import/markdown/attachment.rb @@ -12,16 +12,16 @@ class Attachment class << self # markdown_node - CommonMarker::Node - def from_markdown(markdown_node) + def from_markdown(markdown_node, web_endpoint) case markdown_node.type when :html, :inline_html - from_inline_html(markdown_node) + from_inline_html(markdown_node, web_endpoint) when :image - from_markdown_image(markdown_node) + from_markdown_image(markdown_node, web_endpoint) when :link - from_markdown_link(markdown_node) + from_markdown_link(markdown_node, web_endpoint) when :text, :paragraph - from_markdown_text(markdown_node) + from_markdown_text(markdown_node, web_endpoint) end end @@ -31,62 +31,63 @@ def from_markdown(markdown_node) # a filetype suffix e.g. "https://github.com/user-attachments/assets/75334fd4" # each markdown_node will only ever have a single url as embedded media on # GitHub is always on its own line - def from_markdown_text(markdown_node) + def from_markdown_text(markdown_node, web_endpoint) text = markdown_node.to_plaintext.strip url = URI.extract(text, %w[http https]).first return if url.nil? - return unless github_url?(url, media: true) - return unless whitelisted_type?(url, media: true) + return unless github_url?(url, web_endpoint, media: true) + return unless whitelisted_type?(url, web_endpoint, media: true) # we don't have the :alt or :name so we use a default name - new("media_attachment", url) + new("media_attachment", url, web_endpoint) end - def from_markdown_image(markdown_node) + def from_markdown_image(markdown_node, web_endpoint) url = markdown_node.url return unless url - return unless github_url?(url, media: true) - return unless whitelisted_type?(url, media: true) + return unless github_url?(url, web_endpoint, media: true) + return unless whitelisted_type?(url, web_endpoint, media: true) - new(markdown_node.to_plaintext.strip, url) + new(markdown_node.to_plaintext.strip, url, web_endpoint) end - def from_markdown_link(markdown_node) + def from_markdown_link(markdown_node, web_endpoint) url = markdown_node.url return unless url - return unless github_url?(url, docs: true) - return unless whitelisted_type?(url, docs: true) + return unless github_url?(url, web_endpoint, docs: true) + return unless whitelisted_type?(url, web_endpoint, docs: true) - new(markdown_node.to_plaintext.strip, url) + new(markdown_node.to_plaintext.strip, url, web_endpoint) end - def from_inline_html(markdown_node) + def from_inline_html(markdown_node, web_endpoint) img = Nokogiri::HTML.parse(markdown_node.string_content).xpath('//img')[0] return if img.nil? || img[:src].blank? - return unless github_url?(img[:src], media: true) - return unless whitelisted_type?(img[:src], media: true) + return unless github_url?(img[:src], web_endpoint, media: true) + return unless whitelisted_type?(img[:src], web_endpoint, media: true) - new(img[:alt], img[:src]) + new(img[:alt], img[:src], web_endpoint) end - def github_url?(url, docs: false, media: false) + def github_url?(url, web_endpoint, docs: false, media: false) if media - url.start_with?(::Gitlab::GithubImport::MarkdownText.github_url, - ::Gitlab::GithubImport::MarkdownText::GITHUB_MEDIA_CDN) + url.start_with?(web_endpoint, + ::Gitlab::GithubImport::MarkdownText::GITHUB_MEDIA_CDN + ) elsif docs - url.start_with?(::Gitlab::GithubImport::MarkdownText.github_url) + url.start_with?(web_endpoint) end end - def whitelisted_type?(url, docs: false, media: false) + def whitelisted_type?(url, web_endpoint, docs: false, media: false) if media # We do not know the file extension type from the /assets markdown - return true if url.start_with?(::Gitlab::GithubImport::MarkdownText.github_url) + return true if url.start_with?(web_endpoint) MEDIA_TYPES.any? { |type| url.end_with?(type) } elsif docs @@ -95,38 +96,39 @@ def whitelisted_type?(url, docs: false, media: false) end end - attr_reader :name, :url + attr_reader :name, :url, :web_endpoint - def initialize(name, url) + def initialize(name, url, web_endpoint) @name = name @url = url + @web_endpoint = web_endpoint end def part_of_project_blob?(import_source) url.start_with?( - "#{::Gitlab::GithubImport::MarkdownText.github_url}/#{import_source}/blob" + "#{web_endpoint}/#{import_source}/blob" ) end def doc_belongs_to_project?(import_source) url.start_with?( - "#{::Gitlab::GithubImport::MarkdownText.github_url}/#{import_source}/files" + "#{web_endpoint}/#{import_source}/files" ) end def media?(import_source) url.start_with?( - "#{::Gitlab::GithubImport::MarkdownText.github_url}/#{import_source}/assets", + "#{web_endpoint}/#{import_source}/assets", ::Gitlab::GithubImport::MarkdownText::GITHUB_MEDIA_CDN ) end def user_attachment? - url.start_with?("#{::Gitlab::GithubImport::MarkdownText.github_url}/user-attachments/") + url.start_with?("#{web_endpoint}/user-attachments/") end def inspect - "<#{self.class.name}: { name: #{name}, url: #{url} }>" + "<#{self.class.name}: { name: #{name}, url: #{url}, web_endpoint: #{web_endpoint} }>" end end end diff --git a/lib/gitlab/github_import/markdown_text.rb b/lib/gitlab/github_import/markdown_text.rb index 3d22552269c6632e0b027ba3512c418572419357..c97bd1fd056a2c4b83bffa07d09ca1123120123b 100644 --- a/lib/gitlab/github_import/markdown_text.rb +++ b/lib/gitlab/github_import/markdown_text.rb @@ -20,31 +20,23 @@ def format(...) new(...).perform end - def fetch_attachments(text) + def fetch_attachments(text, web_endpoint) attachments = [] return attachments if text.nil? doc = CommonMarker.render_doc(text) doc.walk do |node| - attachment = extract_attachment(node) + attachment = extract_attachment(node, web_endpoint) attachments << attachment if attachment end attachments end - # Returns github domain without slash in the end - def github_url - oauth_config = Gitlab::Auth::OAuth::Provider.config_for('github') || {} - url = oauth_config['url'].presence || 'https://github.com' - url = url.chop if url.end_with?('/') - url - end - private - def extract_attachment(node) - ::Gitlab::GithubImport::Markdown::Attachment.from_markdown(node) + def extract_attachment(node, web_endpoint) + ::Gitlab::GithubImport::Markdown::Attachment.from_markdown(node, web_endpoint) end end @@ -52,11 +44,12 @@ def extract_attachment(node) # author - An instance of `Gitlab::GithubImport::Representation::User` # exists - Boolean that indicates the user exists in the GitLab database. # project - An instance of `Project`. - def initialize(text, author = nil, exists = false, project: nil) + def initialize(text, author = nil, exists = false, project: nil, client: nil) @text = text @author = author @exists = exists @project = project + @web_endpoint = client&.web_endpoint || ::Octokit::Default.web_endpoint end def perform @@ -64,13 +57,13 @@ def perform # Gitlab::EncodingHelper#clean remove `null` chars from the string text = clean(formatted_text) - text = convert_ref_links(text, project) if project.present? + text = convert_ref_links(text, project, web_endpoint) if project.present? wrap_mentions_in_backticks(text) end private - attr_reader :text, :author, :exists, :project + attr_reader :text, :author, :exists, :project, :web_endpoint def formatted_text login = author.respond_to?(:fetch) ? author.fetch(:login, nil) : author.try(:login) @@ -80,8 +73,9 @@ def formatted_text end # Links like `https://domain.github.com///pull/` needs to be converted - def convert_ref_links(text, project) - matcher_options = { github_url: self.class.github_url, import_source: project.import_source } + def convert_ref_links(text, project, web_endpoint) + web_endpoint = web_endpoint.chop if web_endpoint.end_with?('/') + matcher_options = { github_url: web_endpoint, import_source: project.import_source } issue_ref_matcher = ISSUE_REF_MATCHER % matcher_options pull_ref_matcher = PULL_REF_MATCHER % matcher_options diff --git a/lib/gitlab/github_import/representation/diff_note.rb b/lib/gitlab/github_import/representation/diff_note.rb index e8f56adcf8882238f1fbb02713b97c8c11ee7ecf..d2f6325819f264a4b887604831c945211e8a7e01 100644 --- a/lib/gitlab/github_import/representation/diff_note.rb +++ b/lib/gitlab/github_import/representation/diff_note.rb @@ -34,7 +34,7 @@ def self.from_api_response(note, additional_data = {}) original_commit_id: note[:original_commit_id], diff_hunk: note[:diff_hunk], author: user, - note: GithubImport::MarkdownText.format(note[:body]), + note: note[:body], created_at: note[:created_at], updated_at: note[:updated_at], note_id: note[:id], @@ -54,7 +54,6 @@ def self.from_json_hash(raw_hash) hash = Representation.symbolize_hash(raw_hash) hash[:author] &&= Representation::User.from_json_hash(hash[:author]) - hash[:note] &&= GithubImport::MarkdownText.format(hash[:note]) new(hash) end diff --git a/lib/gitlab/github_import/representation/note_text.rb b/lib/gitlab/github_import/representation/note_text.rb index 79bef4ec3634700544b5e95155c20c03e59a66e5..43e18a923d603dcb4c7ea3b9dcd721cef230a305 100644 --- a/lib/gitlab/github_import/representation/note_text.rb +++ b/lib/gitlab/github_import/representation/note_text.rb @@ -55,14 +55,6 @@ def github_identifiers }.merge(record_type_specific_attribute) end - def has_attachments? - attachments.present? - end - - def attachments - @attachments ||= MarkdownText.fetch_attachments(text) - end - private def record_type_specific_attribute diff --git a/spec/lib/gitlab/github_import/attachments_downloader_spec.rb b/spec/lib/gitlab/github_import/attachments_downloader_spec.rb index ce792f673a6f428e47ef2be3b3f033e3f34a6f42..27e0ce6127c486761c00bece739e2f5050a19b83 100644 --- a/spec/lib/gitlab/github_import/attachments_downloader_spec.rb +++ b/spec/lib/gitlab/github_import/attachments_downloader_spec.rb @@ -125,6 +125,18 @@ expect(File.exist?(file.path)).to eq(true) end + context 'when filename includes login redirect' do + let(:redirect_url) { 'https://example.com/some-text/login?return_to=gpre366' } + + it 'returns the original file_url' do + expect(::Import::Clients::HTTP).to receive(:get).with(file_url, { follow_redirects: false }) + .and_return sample_response + + file = downloader.perform + expect(file).to eq(file_url) + end + end + context 'when attachment is a video media file' do let(:file_url) { "https://github.com/user-attachments/assets/73433gh3" } let(:redirect_url) { "https://github-production-user-asset-6210df.s3.amazonaws.com/73433gh3.mov" } @@ -186,4 +198,20 @@ expect(Dir.exist?(tmp_dir_path)).to eq false end end + + describe '#github_assets_url_regex' do + context 'with GHE domain' do + let(:web_endpoint) { 'https://github.enterprise.com' } + let(:file_url) { "#{web_endpoint}/project/assets/142635249/4b9f9c90-f060-4845-97cf-b24c558bcb11" } + + subject(:downloader) { described_class.new(file_url, web_endpoint: web_endpoint) } + + it 'matches GHE assets URLs' do + allow(Gitlab::Auth::OAuth::Provider).to receive(:config_for).with('github').and_return(nil) + + regex = downloader.send(:github_assets_url_regex) + expect(file_url).to match(regex) + end + end + end end diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb index cccd97fed75a039d134cdac8a1557711ef8c8c11..3a67722215c12722d426590f228a5aa9f4c90db9 100644 --- a/spec/lib/gitlab/github_import/client_spec.rb +++ b/spec/lib/gitlab/github_import/client_spec.rb @@ -632,7 +632,7 @@ endpoint = 'https://github.kittens.com' expect(client) - .to receive(:custom_api_endpoint) + .to receive(:custom_api_endpoint).twice .and_return(endpoint) expect(client.web_endpoint).to eq(endpoint) @@ -640,6 +640,30 @@ end end + describe '#custom_web_endpoint' do + context 'with custom API endpoint' do + it 'returns the web endpoint derived from API endpoint' do + endpoint = 'https://github.enterprise.com' + + expect(client) + .to receive(:custom_api_endpoint).twice + .and_return('https://github.enterprise.com/api/v3') + + expect(client.custom_web_endpoint).to eq(endpoint) + end + end + + context 'without custom API endpoint' do + it 'returns nil' do + expect(client) + .to receive(:custom_api_endpoint) + .and_return(nil) + + expect(client.custom_web_endpoint).to be_nil + end + end + end + describe '#custom_api_endpoint' do context 'without a custom endpoint' do it 'returns nil' do diff --git a/spec/lib/gitlab/github_import/importer/attachments/issues_importer_spec.rb b/spec/lib/gitlab/github_import/importer/attachments/issues_importer_spec.rb index 6ee6a943484e52286bac835c72d65c3a074e2f2b..b761939c91ff0de76f3e8512f710651882bcd3bd 100644 --- a/spec/lib/gitlab/github_import/importer/attachments/issues_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/attachments/issues_importer_spec.rb @@ -7,7 +7,7 @@ let_it_be(:project) { create(:project) } - let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: "https://github.com") } describe '#sequential_import', :clean_gitlab_redis_shared_state do let_it_be(:issue) { create(:issue, project: project) } diff --git a/spec/lib/gitlab/github_import/importer/attachments/merge_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/attachments/merge_requests_importer_spec.rb index 4329337dc9fa7234e9f2c4c71c1f3d31b8f9af68..e4ee7fd415aa4b0d53f7839b9d6e3353f7b5b4c4 100644 --- a/spec/lib/gitlab/github_import/importer/attachments/merge_requests_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/attachments/merge_requests_importer_spec.rb @@ -7,7 +7,7 @@ let_it_be(:project) { create(:project) } - let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: "https://github.com") } describe '#sequential_import', :clean_gitlab_redis_shared_state do let_it_be(:mr) { create(:merge_request, source_project: project, target_branch: 'feature1') } diff --git a/spec/lib/gitlab/github_import/importer/attachments/notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/attachments/notes_importer_spec.rb index 65433b482c2f878a7cc24695ea511dcca3e6a772..74b754957cdd0978d8a3adb6c1f0559d7d428b04 100644 --- a/spec/lib/gitlab/github_import/importer/attachments/notes_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/attachments/notes_importer_spec.rb @@ -7,7 +7,7 @@ let_it_be(:project) { create(:project) } - let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: "https://github.com") } describe '#sequential_import', :clean_gitlab_redis_shared_state do let_it_be(:note) { create(:note, project: project) } diff --git a/spec/lib/gitlab/github_import/importer/attachments/releases_importer_spec.rb b/spec/lib/gitlab/github_import/importer/attachments/releases_importer_spec.rb index 8f1b6debcd6aa0968a392baa8c0d88b27144b3ee..f0ed34f48f974152415897622b71f44e2c209d67 100644 --- a/spec/lib/gitlab/github_import/importer/attachments/releases_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/attachments/releases_importer_spec.rb @@ -7,7 +7,7 @@ let_it_be(:project) { create(:project) } - let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: "https://github.com") } describe '#sequential_import', :clean_gitlab_redis_shared_state do let_it_be(:release) { create(:release, project: project) } diff --git a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb index f511e2a1055871017c6fb9599fc40ea1f2a478f0..3a1a552ab0ba67d1fd7e84db5f3263f86baccac8 100644 --- a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb @@ -14,7 +14,7 @@ let_it_be(:user) { create(:user) } - let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: 'https://github.com') } let(:discussion_id) { 'b0fa404393eeebb4e82becb8104f238812bb1fe6' } let(:created_at) { Time.new(2017, 1, 1, 12, 00).utc } let(:updated_at) { Time.new(2017, 1, 1, 12, 15).utc } @@ -111,6 +111,18 @@ expect(note.updated_at).to eq(updated_at) end + context 'when the note body includes @ mentions' do + let(:note_body) { "I said to @sam_allen\0 the code should follow @bob's\0 advice. @.ali-ce/group#9?\0" } + let(:expected_note_body) { "I said to `@sam_allen` the code should follow `@bob`'s advice. `@.ali-ce/group#9`?" } + + it 'formats the text correctly' do + subject.execute + + note = project.notes.diff_notes.take + expect(note.note).to eq(expected_note_body) + end + end + context 'when the note has suggestions' do let(:note_body) do <<~EOB diff --git a/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb index 5eb87a7f89fb7f223809a5289953e9a3677d7628..f1273aa37294f481a35c4cef6f46b3c1057a9d59 100644 --- a/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNotesImporter, feature_category: :importers do let(:project) { build(:project, id: 4, import_source: 'foo/bar') } - let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: 'https://github.com') } let(:github_comment) do { diff --git a/spec/lib/gitlab/github_import/importer/events/commented_spec.rb b/spec/lib/gitlab/github_import/importer/events/commented_spec.rb index 41958dd40bc93e6a8dcadcfc45290614fa37b728..22639e7e40d7e1697eca11169f3fb5259a53c981 100644 --- a/spec/lib/gitlab/github_import/importer/events/commented_spec.rb +++ b/spec/lib/gitlab/github_import/importer/events/commented_spec.rb @@ -16,7 +16,7 @@ let_it_be(:source_user) { generate_source_user(project, 1000) } - let(:client) { instance_double('Gitlab::GithubImport::Client') } + let(:client) { instance_double('Gitlab::GithubImport::Client', web_endpoint: 'https://github.com') } let(:issuable) { create(:issue, project: project) } let(:issue_event) do diff --git a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb index 99f23fe8ec43ba4ae51b3f8dcc405d863616af01..550eb24bc41f3bd7f378e7b34d42041b89cb8462 100644 --- a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb @@ -18,7 +18,7 @@ let_it_be(:milestone) { create(:milestone, project: project) } - let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: 'https://github.com') } let(:created_at) { Time.new(2017, 1, 1, 12, 00) } let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } let(:description) { 'This is my issue' } diff --git a/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb b/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb index aacc2e9586b163c8965c4e0f3d6f99b3df359dcf..e3d9d07f9453a8cb2b876974910ee62bb5b204a4 100644 --- a/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Gitlab::GithubImport::Importer::MilestonesImporter, :clean_gitlab_redis_shared_state, feature_category: :importers do let(:project) { create(:project, import_source: 'foo/bar') } - let(:client) { double(:client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: 'https://github.com') } let(:importer) { described_class.new(project, client) } let(:due_on) { Time.new(2017, 2, 1, 12, 00) } let(:created_at) { Time.new(2017, 1, 1, 12, 00) } diff --git a/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb index ef3f35b26ff099dc22edd0ed787035fb5ea29739..fe6bb0a4ce98a95b902f48db960ec1da7c4627f5 100644 --- a/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb @@ -8,7 +8,12 @@ let_it_be(:project) { create(:project, import_source: 'nickname/public-test-repo') } let(:note_text) { Gitlab::GithubImport::Representation::NoteText.from_db_record(record) } - let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:web_endpoint) { 'https://github.com' } + let(:client) do + instance_double(Gitlab::GithubImport::Client).tap do |double| + allow(double).to receive(:web_endpoint).and_return(web_endpoint) + end + end let(:doc_url) { 'https://github.com/nickname/public-test-repo/files/9020437/git-cheat-sheet.txt' } let(:image_url) { 'https://user-images.githubusercontent.com/6833842/0cf366b61ef2.jpeg' } @@ -82,13 +87,17 @@ let(:options) { { headers: { 'Authorization' => "Bearer #{access_token}" } } } before do - allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new).with(doc_url, options: options) + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(doc_url, options: options, web_endpoint: web_endpoint) .and_return(downloader_stub) - allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new).with(image_url, options: options) + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(image_url, options: options, web_endpoint: web_endpoint) .and_return(downloader_stub) - allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new).with(image_tag_url, options: options) + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(image_tag_url, options: options, web_endpoint: web_endpoint) .and_return(downloader_stub) - allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new).with(user_attachment_url, options: options) + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(user_attachment_url, options: options, web_endpoint: web_endpoint) .and_return(downloader_stub) allow(downloader_stub).to receive(:perform).and_return(tmp_stub_doc, tmp_stub_image, tmp_stub_image_tag, tmp_stub_user_attachment) @@ -142,7 +151,7 @@ expect(record.note).to include("[link to other project blob file](#{other_project_blob_url})") end - context 'when the attachment is video media' do + context 'when the attachment is supported video media' do let(:media_attachment_url) { 'https://github.com/user-attachments/assets/media-attachment' } let(:text) { media_attachment_url } let(:record) { create(:note, project: project, note: text) } @@ -155,7 +164,7 @@ before do allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new).with(media_attachment_url, - options: options) + options: options, web_endpoint: web_endpoint) .and_return(downloader_stub) allow(downloader_stub).to receive(:perform).and_return(tmp_stub_media_attachment) @@ -174,6 +183,285 @@ expect(record.note).to include("![media_attachment](/uploads/test-secret/video.mp4)") end end + + context 'with GHE domain' do + let(:web_endpoint) { 'https://github.enterprise.com' } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: web_endpoint) } + let(:ghe_doc_url) { "#{web_endpoint}/nickname/public-test-repo/files/9020437/git-cheat-sheet.txt" } + let(:ghe_image_url) { "#{web_endpoint}/user-attachments/assets/73433gh3" } + + let(:text) do + <<-TEXT.split("\n").map(&:strip).join("\n") + Some text... + [ghe-doc](#{ghe_doc_url}) + ![ghe-image](#{ghe_image_url}) + TEXT + end + + let(:record) { create(:release, project: project, description: text) } + + before do + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(ghe_doc_url, options: options, web_endpoint: web_endpoint) + .and_return(downloader_stub) + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(ghe_image_url, options: options, web_endpoint: web_endpoint) + .and_return(downloader_stub) + allow(downloader_stub).to receive(:perform).and_return(tmp_stub_doc, tmp_stub_image, tmp_stub_image_tag, + tmp_stub_user_attachment) + allow(downloader_stub).to receive(:delete).twice + allow(client).to receive_message_chain(:octokit, :access_token).and_return(access_token) + allow(Gitlab::Auth::OAuth::Provider).to receive(:config_for).with('github').and_return(nil) + end + + it 'changes attachment links' do + importer.execute + + record.reload + expect(record.description).to start_with("Some text...\n[ghe-doc](/uploads/") + expect(record.description).to include("![ghe-image](/uploads/") + end + + context 'when the attachment is file' do + let(:pdf_file_url) { "#{web_endpoint}/user-attachments/files/3/pdf-attachment.pdf" } + let(:zip_file_url) { "#{web_endpoint}/user-attachments/files/4/zip-attachment.zip" } + + let(:text) do + <<-TEXT.split("\n").map(&:strip).join("\n") + Some text... + [pdf-file](#{pdf_file_url}) + [zip-file](#{zip_file_url}) + TEXT + end + + let(:record) { create(:note, project: project, note: text) } + + before do + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(pdf_file_url, options: options, web_endpoint: web_endpoint) + .and_return(downloader_stub) + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(zip_file_url, options: options, web_endpoint: web_endpoint) + .and_return(downloader_stub) + allow(downloader_stub).to receive(:perform).and_return(pdf_file_url, zip_file_url) + allow(client).to receive_message_chain(:octokit, :access_token).and_return(access_token) + end + + it 'does not change attachment link' do + importer.execute + + record.reload + expect(record.note).to include(pdf_file_url) + expect(record.note).to include(zip_file_url) + end + end + + # rubocop:disable RSpec/MultipleMemoizedHelpers -- required + context 'when the attachment is a mov file' do + let(:ghe_video_url) { "#{web_endpoint}/user-attachments/assets/video-attachment" } + let(:text) { ghe_video_url } + let(:record) { create(:note, project: project, note: text) } + let(:tmp_stub_video) { Tempfile.create('video-attachment') } + let(:tmp_stub_path) { Pathname.new(tmp_stub_video.path) } + let(:tmp_stub_video_with_ext) { Tempfile.create('video-attachment.mov') } + let(:uploader_hash) do + { alt: nil, + url: '/uploads/test-secret/video.mov', + markdown: nil } + end + + before do + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(ghe_video_url, options: options, web_endpoint: web_endpoint) + .and_return(downloader_stub) + allow(downloader_stub).to receive(:perform).and_return(tmp_stub_video) + allow(downloader_stub).to receive(:delete) + + allow(Marcel::MimeType).to receive(:for).with(tmp_stub_path).and_return('video/quicktime') + + allow(File).to receive(:extname).and_call_original + allow(File).to receive(:extname).with(tmp_stub_video.path).and_return('') + + mime_type_double = instance_double(Mime::Type) + allow(Mime::Type).to receive(:lookup).with('video/quicktime').and_return(mime_type_double) + allow(FileUtils).to receive(:mv).and_return(true) + + allow(File).to receive(:open).with( + a_string_matching(/\.mov/), 'rb').and_return(tmp_stub_video_with_ext) + + allow(UploadService).to receive_message_chain(:new, :execute).and_return(uploader_hash) + + allow(client).to receive_message_chain(:octokit, :access_token).and_return(access_token) + allow(Gitlab::Auth::OAuth::Provider).to receive(:config_for).with('github').and_return(nil) + end + + it 'calls update_ghe_video_path for video files on GHE' do + expect(importer).to receive(:update_ghe_video_path).with(tmp_stub_video).at_least(:once).and_call_original + + importer.execute + end + + it 'changes standalone attachment link to correct markdown' do + importer.execute + record.reload + expect(record.note).to include("![media_attachment](/uploads/test-secret/video.mov)") + end + end + + context 'when the attachment is a mp4 file' do + let(:ghe_video_url) { "#{web_endpoint}/user-attachments/assets/video-attachment" } + let(:text) { ghe_video_url } + let(:record) { create(:note, project: project, note: text) } + let(:tmp_stub_video) { Tempfile.create('video-attachment') } + let(:tmp_stub_path) { Pathname.new(tmp_stub_video.path) } + let(:tmp_stub_video_with_ext) { Tempfile.create('video-attachment.mp4') } + let(:uploader_hash) do + { alt: nil, + url: '/uploads/test-secret/video.mp4', + markdown: nil } + end + + before do + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(ghe_video_url, options: options, web_endpoint: web_endpoint) + .and_return(downloader_stub) + allow(downloader_stub).to receive(:perform).and_return(tmp_stub_video) + allow(downloader_stub).to receive(:delete) + + allow(Marcel::MimeType).to receive(:for).with(tmp_stub_path).and_return('video/mp4') + + allow(File).to receive(:extname).and_call_original + allow(File).to receive(:extname).with(tmp_stub_video.path).and_return('') + + mime_type_double = instance_double(Mime::Type) + allow(Mime::Type).to receive(:lookup).with('video/mp4').and_return(mime_type_double) + allow(FileUtils).to receive(:mv).and_return(true) + + allow(File).to receive(:open).with( + a_string_matching(/\.mp4/), 'rb').and_return(tmp_stub_video_with_ext) + + allow(UploadService).to receive_message_chain(:new, :execute).and_return(uploader_hash) + + allow(client).to receive_message_chain(:octokit, :access_token).and_return(access_token) + allow(Gitlab::Auth::OAuth::Provider).to receive(:config_for).with('github').and_return(nil) + end + + it 'calls update_ghe_video_path for video files on GHE' do + expect(importer).to receive(:update_ghe_video_path).with(tmp_stub_video).at_least(:once).and_call_original + + importer.execute + end + + it 'changes standalone attachment link to correct markdown' do + importer.execute + record.reload + expect(record.note).to include("![media_attachment](/uploads/test-secret/video.mp4)") + end + end + + context 'when the attachment is a webm file' do + let(:ghe_video_url) { "#{web_endpoint}/user-attachments/assets/video-attachment" } + let(:text) { ghe_video_url } + let(:record) { create(:note, project: project, note: text) } + let(:tmp_stub_video) { Tempfile.create('video-attachment') } + let(:tmp_stub_path) { Pathname.new(tmp_stub_video.path) } + let(:tmp_stub_video_with_ext) { Tempfile.create('video-attachment.webm') } + let(:uploader_hash) do + { alt: nil, + url: '/uploads/test-secret/video.webm', + markdown: nil } + end + + before do + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(ghe_video_url, options: options, web_endpoint: web_endpoint) + .and_return(downloader_stub) + allow(downloader_stub).to receive(:perform).and_return(tmp_stub_video) + allow(downloader_stub).to receive(:delete) + + allow(Marcel::MimeType).to receive(:for).with(tmp_stub_path).and_return('video/webm') + + allow(File).to receive(:extname).and_call_original + allow(File).to receive(:extname).with(tmp_stub_video.path).and_return('') + + mime_type_double = instance_double(Mime::Type) + allow(Mime::Type).to receive(:lookup).with('video/webm').and_return(mime_type_double) + allow(FileUtils).to receive(:mv).and_return(true) + + allow(File).to receive(:open).with( + a_string_matching(/\.webm/), 'rb').and_return(tmp_stub_video_with_ext) + + allow(UploadService).to receive_message_chain(:new, :execute).and_return(uploader_hash) + + allow(client).to receive_message_chain(:octokit, :access_token).and_return(access_token) + allow(Gitlab::Auth::OAuth::Provider).to receive(:config_for).with('github').and_return(nil) + end + + it 'calls update_ghe_video_path for video files on GHE' do + expect(importer).to receive(:update_ghe_video_path).with(tmp_stub_video).at_least(:once).and_call_original + + importer.execute + end + + it 'changes standalone attachment link to correct markdown' do + importer.execute + record.reload + expect(record.note).to include("![media_attachment](/uploads/test-secret/video.webm)") + end + end + + context 'when the attachment is an unsupported video format' do + let(:ghe_video_url) { "#{web_endpoint}/user-attachments/assets/video-attachment" } + let(:text) { ghe_video_url } + let(:record) { create(:note, project: project, note: text) } + let(:tmp_stub_video) { Tempfile.create('video-attachment') } + let(:tmp_stub_path) { Pathname.new(tmp_stub_video.path) } + let(:tmp_stub_video_with_ext) { Tempfile.create('video-attachment.avi') } + let(:uploader_hash) do + { alt: nil, + url: '/uploads/test-secret/video.avi', + markdown: nil } + end + + before do + allow(Gitlab::GithubImport::AttachmentsDownloader).to receive(:new) + .with(ghe_video_url, options: options, web_endpoint: web_endpoint) + .and_return(downloader_stub) + allow(downloader_stub).to receive(:perform).and_return(tmp_stub_video) + allow(downloader_stub).to receive(:delete) + + allow(Marcel::MimeType).to receive(:for).with(tmp_stub_path).and_return('video/avi') + + allow(File).to receive(:extname).and_call_original + allow(File).to receive(:extname).with(tmp_stub_video.path).and_return('') + + mime_type_double = instance_double(Mime::Type) + allow(Mime::Type).to receive(:lookup).with('video/avi').and_return(mime_type_double) + allow(FileUtils).to receive(:mv).and_return(true) + + allow(File).to receive(:open).with( + a_string_matching(/\.avi/), 'rb').and_return(tmp_stub_video_with_ext) + + allow(UploadService).to receive_message_chain(:new, :execute).and_return(uploader_hash) + + allow(client).to receive_message_chain(:octokit, :access_token).and_return(access_token) + allow(Gitlab::Auth::OAuth::Provider).to receive(:config_for).with('github').and_return(nil) + end + + it 'calls update_ghe_video_path for video files on GHE' do + expect(importer).to receive(:update_ghe_video_path).with(tmp_stub_video).at_least(:once).and_call_original + + importer.execute + end + + it 'does not update the link with a file extension' do + importer.execute + record.reload + expect(record.note).to include("/uploads/test-secret/video") + end + end + # rubocop:enable RSpec/MultipleMemoizedHelpers + end end end end diff --git a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb index 7a4c985f8a28dfd8e569ea01286231fea0808efd..c767e14d5fc0ae509194addefda1fb2e222e86c3 100644 --- a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb @@ -16,7 +16,7 @@ let_it_be(:user) { create(:user) } let_it_be(:source_user) { generate_source_user(project, '4') } - let(:client) { double(:client) } + let(:client) { double(:client, web_endpoint: 'https://github.com') } let(:created_at) { Time.new(2017, 1, 1, 12, 00) } let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } let(:note_body) { 'This is my note' } diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb index 505219350a45d7b1334dc0391f01ca807310cea9..7d6d381f31ec34f33ced10da95375377a046769f 100644 --- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb @@ -18,7 +18,7 @@ let_it_be(:source_user_2) { generate_source_user(project, user_representation_2.id) } let_it_be(:user) { create(:user) } - let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: 'https://github.com') } let(:created_at) { DateTime.strptime('2024-11-05T20:10:15Z') } let(:updated_at) { DateTime.strptime('2024-11-06T20:10:15Z') } let(:merged_at) { DateTime.strptime('2024-11-07T20:10:15Z') } diff --git a/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb index 2f7e8f037ba48a5cbd8957f2dfce35fe765d813c..dbe2107f96efc18613a74c66edb717913f8557e3 100644 --- a/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb @@ -20,7 +20,8 @@ let(:client_double) do instance_double( 'Gitlab::GithubImport::Client', - user: { id: 999, login: 'author', email: 'author@email.com' } + user: { id: 999, login: 'author', email: 'author@email.com' }, + web_endpoint: 'https://github.com' ) end diff --git a/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb index 1f77031043c343aefa4cef851c67f25e268a91d8..7be41b75ee200b158cb8682ef99f8d49c628ef3a 100644 --- a/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb @@ -20,7 +20,7 @@ } end - let(:client) { double(:client) } + let(:client) { instance_double(Gitlab::GithubImport::Client, web_endpoint: 'https://github.com') } let(:github_release_name) { 'Initial Release' } let(:created_at) { Time.new(2017, 1, 1, 12, 00) } let(:released_at) { Time.new(2017, 1, 1, 12, 00) } diff --git a/spec/lib/gitlab/github_import/markdown/attachment_spec.rb b/spec/lib/gitlab/github_import/markdown/attachment_spec.rb index 4869db6a35400d3a8bf57a52176b007f0f47264d..12824c3f7f83e77487b4a93ed5dac0ecc7337d0e 100644 --- a/spec/lib/gitlab/github_import/markdown/attachment_spec.rb +++ b/spec/lib/gitlab/github_import/markdown/attachment_spec.rb @@ -6,6 +6,7 @@ let(:name) { FFaker::Lorem.word } let(:url) { FFaker::Internet.uri('https') } let(:import_source) { 'nickname/public-test-repo' } + let(:web_endpoint) { 'https://github.com' } describe '.from_markdown' do context "when it's a doc attachment" do @@ -17,16 +18,32 @@ end it 'returns instance with attachment info' do - attachment = described_class.from_markdown(markdown_node) + attachment = described_class.from_markdown(markdown_node, web_endpoint) expect(attachment.name).to eq name expect(attachment.url).to eq url end + context "when it's a doc attachment from GHE" do + let(:web_endpoint) { 'https://gce.kitty.com' } + let(:url) { "#{web_endpoint}/nickname/public-test-repo/files/3/git-cheat-sheet.pdf" } + let(:name) { FFaker::Lorem.word } + let(:markdown_node) do + instance_double(CommonMarker::Node, url: url, to_plaintext: name, type: :link) + end + + it 'returns instance with attachment info' do + attachment = described_class.from_markdown(markdown_node, web_endpoint) + + expect(attachment.name).to eq name + expect(attachment.url).to eq url + end + end + context "when type is not in whitelist" do let(:doc_extension) { 'exe' } - it { expect(described_class.from_markdown(markdown_node)).to eq nil } + it { expect(described_class.from_markdown(markdown_node, web_endpoint)).to eq nil } end context 'when domain name is unknown' do @@ -34,13 +51,13 @@ "https://bitbucket.com/nickname/public-test-repo/files/3/git-cheat-sheet.#{doc_extension}" end - it { expect(described_class.from_markdown(markdown_node)).to eq nil } + it { expect(described_class.from_markdown(markdown_node, web_endpoint)).to eq nil } end context 'when URL is blank' do let(:url) { nil } - it { expect(described_class.from_markdown(markdown_node)).to eq nil } + it { expect(described_class.from_markdown(markdown_node, web_endpoint)).to eq nil } end end @@ -53,7 +70,7 @@ end it 'returns instance with attachment info' do - attachment = described_class.from_markdown(markdown_node) + attachment = described_class.from_markdown(markdown_node, web_endpoint) expect(attachment.name).to eq name expect(attachment.url).to eq url @@ -62,32 +79,44 @@ context "when type is not in whitelist" do let(:image_extension) { 'mkv' } - it { expect(described_class.from_markdown(markdown_node)).to eq nil } + it { expect(described_class.from_markdown(markdown_node, web_endpoint)).to eq nil } end context 'when domain name is unknown' do let(:url) { "https://user-images.github.com/1/uuid-1.#{image_extension}" } - it { expect(described_class.from_markdown(markdown_node)).to eq nil } + it { expect(described_class.from_markdown(markdown_node, web_endpoint)).to eq nil } end context 'with allowed domain as subdomain' do let(:url) { "https://user-images.githubusercontent.com.attacker.controlled.domain/1/uuid-1.#{image_extension}" } - it { expect(described_class.from_markdown(markdown_node)).to eq nil } + it { expect(described_class.from_markdown(markdown_node, web_endpoint)).to eq nil } end context 'when URL is blank' do let(:url) { nil } - it { expect(described_class.from_markdown(markdown_node)).to eq nil } + it { expect(described_class.from_markdown(markdown_node, web_endpoint)).to eq nil } end context 'when image attachment is in the new format' do let(:url) { "https://github.com/#{import_source}/assets/142635249/4b9f9c90-f060-4845-97cf-b24c558bcb11" } it 'returns instance with attachment info' do - attachment = described_class.from_markdown(markdown_node) + attachment = described_class.from_markdown(markdown_node, web_endpoint) + + expect(attachment.name).to eq name + expect(attachment.url).to eq url + end + end + + context 'when the instance is a ghe instance' do + let(:web_endpoint) { "https://ghe.doggo.com" } + let(:url) { "#{web_endpoint}/user-attachments/assets/142635249/4b9f9c90-f060-4845-97cf-b24c558bcb11" } + + it 'returns instance with attachment info' do + attachment = described_class.from_markdown(markdown_node, web_endpoint) expect(attachment.name).to eq name expect(attachment.url).to eq url @@ -105,7 +134,7 @@ end it 'returns instance with attachment info' do - attachment = described_class.from_markdown(markdown_node) + attachment = described_class.from_markdown(markdown_node, web_endpoint) expect(attachment.name).to eq name expect(attachment.url).to eq url @@ -114,7 +143,7 @@ context 'when image src is not present' do let(:img) { "\"#{name}\"" } - it { expect(described_class.from_markdown(markdown_node)).to eq nil } + it { expect(described_class.from_markdown(markdown_node, web_endpoint)).to eq nil } end end @@ -125,7 +154,7 @@ end it 'returns an attachment object with the download url and default name' do - attachment = described_class.from_markdown(markdown_node) + attachment = described_class.from_markdown(markdown_node, web_endpoint) expect(attachment.name).to be("media_attachment") expect(attachment.url).to eq media_attachment_url @@ -134,7 +163,7 @@ end describe '#part_of_project_blob?' do - let(:attachment) { described_class.new('test', url) } + let(:attachment) { described_class.new('test', url, web_endpoint) } context 'when url is a part of project blob' do let(:url) { "https://github.com/#{import_source}/blob/main/example.md" } @@ -150,7 +179,7 @@ end describe '#doc_belongs_to_project?' do - let(:attachment) { described_class.new('test', url) } + let(:attachment) { described_class.new('test', url, web_endpoint) } context 'when url relates to this project' do let(:url) { "https://github.com/#{import_source}/files/9020437/git-cheat-sheet.txt" } @@ -172,7 +201,7 @@ end describe '#media?' do - let(:attachment) { described_class.new('test', url) } + let(:attachment) { described_class.new('test', url, web_endpoint) } context 'when it is a media link' do let(:url) { 'https://user-images.githubusercontent.com/6833842/0cf366b61ef2.jpeg' } @@ -195,9 +224,9 @@ describe '#inspect' do it 'returns attachment basic info' do - attachment = described_class.new(name, url) + attachment = described_class.new(name, url, web_endpoint) - expect(attachment.inspect).to eq "" + expect(attachment.inspect).to eq "" # rubocop:disable Layout/LineLength, Lint/RedundantCopDisableDirective -- Needs to be on one line end end end diff --git a/spec/lib/gitlab/github_import/markdown_text_spec.rb b/spec/lib/gitlab/github_import/markdown_text_spec.rb index 4143875fee17947e1fcf0a6def809454750be6e9..53f822c1f6591f612ef60627ec8e76ae50fbbada 100644 --- a/spec/lib/gitlab/github_import/markdown_text_spec.rb +++ b/spec/lib/gitlab/github_import/markdown_text_spec.rb @@ -42,6 +42,7 @@ context 'when Github EE with custom domain name' do let(:github_domain) { 'https://custom.github.com/' } + let(:client) { double(:client, web_endpoint: github_domain) } let(:text_in) do <<-TEXT #{paragraph} @@ -51,12 +52,7 @@ TEXT end - before do - allow(Gitlab::Auth::OAuth::Provider) - .to receive(:config_for).with('github').and_return({ 'url' => github_domain }) - end - - it { expect(described_class.format(text_in, project: project)).to eq text_out } + it { expect(described_class.format(text_in, project: project, client: client)).to eq text_out } end end @@ -94,7 +90,7 @@ end it 'fetches attachments' do - attachments = described_class.fetch_attachments(text) + attachments = described_class.fetch_attachments(text, "https://github.com") expect(attachments.map(&:name)).to contain_exactly('special-image', 'tag-image', 'some-doc') expect(attachments.map(&:url)).to contain_exactly( @@ -105,7 +101,31 @@ end it 'returns an empty array when passed nil' do - expect(described_class.fetch_attachments(nil)).to be_empty + expect(described_class.fetch_attachments(nil, nil)).to be_empty + end + + context 'with GHE web endpoint' do + let(:web_endpoint) { 'https://github.enterprise.com' } + let(:image_extension) { Gitlab::GithubImport::Markdown::Attachment::MEDIA_TYPES.sample } + let(:ghe_image_attachment) do + "![ghe-image](#{web_endpoint}/user-attachments/assets/uuid-1.#{image_extension})" + end + + let(:text) do + <<-TEXT.split("\n").map(&:strip).join("\n") + Comment with GHE attachment + #{ghe_image_attachment} + TEXT + end + + it 'fetches attachments from GHE domain' do + attachments = described_class.fetch_attachments(text, web_endpoint) + + expect(attachments.map(&:name)).to contain_exactly('ghe-image') + expect(attachments.map(&:url)).to contain_exactly( + "#{web_endpoint}/user-attachments/assets/uuid-1.#{image_extension}" + ) + end end end diff --git a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb index f8f34d7bf92bffe9ebe13d4546f289ea960491a0..4295627ff4f1659fd532a8e4258b360e9f38985e 100644 --- a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb +++ b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb @@ -35,18 +35,8 @@ shared_examples 'a DiffNote representation' do context 'when note body is present' do - let(:note_body) { "I said to @sam_allen\0 the code should follow @bob's\0 advice. @.ali-ce/group#9?\0" } - let(:expected_note_body) { "I said to `@sam_allen` the code should follow `@bob`'s advice. `@.ali-ce/group#9`?" } - - before do - allow(Gitlab::GithubImport::MarkdownText).to receive(:format).and_call_original - - note - end - - it 'verify that the formatted description using MarkdownText equals the expected description' do - expect(Gitlab::GithubImport::MarkdownText).to have_received(:format) - expect(note.note).to eq(expected_note_body) + it 'includes the note body' do + expect(note.note).to eq(note_body) end end diff --git a/spec/lib/gitlab/github_import/representation/note_text_spec.rb b/spec/lib/gitlab/github_import/representation/note_text_spec.rb index b1ca15128557a8847a837e419c608fd1826c9806..f81f82fb36bf6d7a83f899d5b4af2ac5146c8f5f 100644 --- a/spec/lib/gitlab/github_import/representation/note_text_spec.rb +++ b/spec/lib/gitlab/github_import/representation/note_text_spec.rb @@ -153,36 +153,4 @@ end end end - - describe '#has_attachments?' do - subject { described_class.new({ text: text }).has_attachments? } - - context 'when text has attachments' do - let(:text) { 'See ![image](https://user-images.githubusercontent.com/1/uuid-1.png) for details' } - - it { is_expected.to eq(true) } - end - - context 'when text does not have attachments' do - let(:text) { 'Some text here' } - - it { is_expected.to eq(false) } - end - end - - describe '#attachments' do - subject { described_class.new({ text: text }).attachments } - - context 'when text has attachments' do - let(:text) { 'See ![image](https://user-images.githubusercontent.com/1/uuid-1.png) for details' } - - it { is_expected.to contain_exactly(instance_of(Gitlab::GithubImport::Markdown::Attachment)) } - end - - context 'when text does not have attachments' do - let(:text) { 'Some text here' } - - it { is_expected.to be_empty } - end - end end diff --git a/spec/workers/gitlab/github_import/attachments/import_issue_worker_spec.rb b/spec/workers/gitlab/github_import/attachments/import_issue_worker_spec.rb index e0db440232cf25f1714fcd6ee9fc1197373c9772..0300937cbb13d4610474bcf36e73f2058c00983c 100644 --- a/spec/workers/gitlab/github_import/attachments/import_issue_worker_spec.rb +++ b/spec/workers/gitlab/github_import/attachments/import_issue_worker_spec.rb @@ -12,7 +12,7 @@ instance_double('Project', full_path: 'foo/bar', id: 1, import_state: import_state) end - let(:client) { instance_double('Gitlab::GithubImport::Client') } + let(:client) { instance_double('Gitlab::GithubImport::Client', web_endpoint: "https://github.com") } let(:issue_hash) do { diff --git a/spec/workers/gitlab/github_import/attachments/import_merge_request_worker_spec.rb b/spec/workers/gitlab/github_import/attachments/import_merge_request_worker_spec.rb index b4be229af2a0be54430f1d0a40db8393d61ce66d..cb7d04b69de3d7ebcd5f0aaa7dd587733be63a33 100644 --- a/spec/workers/gitlab/github_import/attachments/import_merge_request_worker_spec.rb +++ b/spec/workers/gitlab/github_import/attachments/import_merge_request_worker_spec.rb @@ -12,7 +12,7 @@ instance_double('Project', full_path: 'foo/bar', id: 1, import_state: import_state) end - let(:client) { instance_double('Gitlab::GithubImport::Client') } + let(:client) { instance_double('Gitlab::GithubImport::Client', web_endpoint: "https://github.com") } let(:mr_hash) do {