Repository: denoland/deno
Pull request: test: cleanup npm test registry code

Required for https://github.com/denoland/deno/pull/23560

Changed files:
filename: tests/util/server/src/npm.rs
patch: @@ -15,22 +15,34 @@ use tar::Builder;
 
 use crate::testdata_path;
 
-pub static CUSTOM_NPM_PACKAGE_CACHE: Lazy<CustomNpmPackageCache> =
+pub const DENOTEST_SCOPE_NAME: &str = "@denotest";
+
+pub static PUBLIC_TEST_NPM_REGISTRY: Lazy<TestNpmRegistry> = Lazy::new(|| {
+  TestNpmRegistry::new(
+    NpmRegistryKind::Public,
+    &format!("http://localhost:{}", crate::servers::PORT),
+    "/npm/registry",
+  )
+});
+
+// TODO: rewrite to use config
+pub static PRIVATE_TEST_NPM_REGISTRY_1: Lazy<TestNpmRegistry> =
   Lazy::new(|| {
-    CustomNpmPackageCache::new(format!(
-      "http://localhost:{}/npm/registry",
-      crate::servers::PORT,
-    ))
+    TestNpmRegistry::new(
+      NpmRegistryKind::Private,
+      &format!(
+        "http://localhost:{}",
+        crate::servers::PRIVATE_NPM_REGISTRY_1_PORT
+      ),
+      // TODO: change it
+      "/npm/registry",
+    )
   });
 
-pub static CUSTOM_NPM_PACKAGE_CACHE_FOR_PRIVATE_REGISTRY: Lazy<
-  CustomNpmPackageCache,
-> = Lazy::new(|| {
-  CustomNpmPackageCache::new(format!(
-    "http://localhost:{}/npm/registry",
-    crate::servers::PRIVATE_NPM_REGISTRY_1_PORT
-  ))
-});
+pub enum NpmRegistryKind {
+  Public,
+  Private,
+}
 
 struct CustomNpmPackage {
   pub registry_file: String,
@@ -39,19 +51,32 @@ struct CustomNpmPackage {
 
 /// Creates tarballs and a registry json file for npm packages
 /// in the `testdata/npm/registry/@denotest` directory.
-pub struct CustomNpmPackageCache {
-  registry_url: String,
+pub struct TestNpmRegistry {
+  #[allow(unused)]
+  kind: NpmRegistryKind,
+  // Eg. http://localhost:4544/
+  hostname: String,
+  // Eg. /registry/npm/
+  path: String,
+
   cache: Mutex<HashMap<String, CustomNpmPackage>>,
 }
 
-impl CustomNpmPackageCache {
-  pub fn new(registry_url: String) -> Self {
-    let registry_url = registry_url
-      .strip_suffix('/')
-      .unwrap_or(&registry_url)
-      .to_string();
+impl TestNpmRegistry {
+  pub fn new(kind: NpmRegistryKind, hostname: &str, path: &str) -> Self {
+    let hostname = hostname.strip_suffix('/').unwrap_or(hostname).to_string();
+    assert!(
+      !path.is_empty(),
+      "npm test registry must have a non-empty path"
+    );
+    let stripped = path.strip_prefix('/').unwrap_or(path);
+    let stripped = path.strip_suffix('/').unwrap_or(stripped);
+    let path = format!("/{}/", stripped);
+
     Self {
-      registry_url,
+      hostname,
+      path,
+      kind,
       cache: Default::default(),
     }
   }
@@ -79,7 +104,7 @@ impl CustomNpmPackageCache {
   ) -> Result<Option<TResult>> {
     // it's ok if multiple threads race here as they will do the same work twice
     if !self.cache.lock().contains_key(package_name) {
-      match get_npm_package(package_name, &self.registry_url)? {
+      match get_npm_package(&self.hostname, &self.path, package_name)? {
         Some(package) => {
           self.cache.lock().insert(package_name.to_string(), package);
         }
@@ -88,11 +113,35 @@ impl CustomNpmPackageCache {
     }
     Ok(self.cache.lock().get(package_name).map(func))
   }
+
+  pub fn strip_registry_path_prefix_from_uri_path<'s>(
+    &self,
+    uri_path: &'s str,
+  ) -> Option<&'s str> {
+    uri_path.strip_prefix(&self.path)
+  }
+
+  pub fn strip_denotest_prefix_from_uri_path<'s>(
+    &self,
+    uri_path: &'s str,
+  ) -> Option<&'s str> {
+    let prefix1 = format!("{}{}/", self.path, DENOTEST_SCOPE_NAME);
+    let prefix2 = format!("{}{}%2f", self.path, DENOTEST_SCOPE_NAME);
+
+    uri_path
+      .strip_prefix(&prefix1)
+      .or_else(|| uri_path.strip_prefix(&prefix2))
+  }
+
+  pub fn uri_path_starts_with_registry_path(&self, uri_path: &str) -> bool {
+    uri_path.starts_with(&self.path)
+  }
 }
 
 fn get_npm_package(
+  registry_hostname: &str,
+  registry_path: &str,
   package_name: &str,
-  registry_url: &str,
 ) -> Result<Option<CustomNpmPackage>> {
   let package_folder = testdata_path().join("npm/registry").join(package_name);
   if !package_folder.exists() {
@@ -141,7 +190,8 @@ fn get_npm_package(
     dist.insert("shasum".to_string(), "dummy-value".into());
     dist.insert(
       "tarball".to_string(),
-      format!("{registry_url}/{package_name}/{version}.tgz").into(),
+      format!("{registry_hostname}{registry_path}{package_name}/{version}.tgz")
+        .into(),
     );
 
     tarballs.insert(version.clone(), tarball_bytes);

filename: tests/util/server/src/servers/mod.rs
patch: @@ -50,8 +50,7 @@ use hyper_utils::ServerOptions;
 
 use super::https::get_tls_listener_stream;
 use super::https::SupportedHttpVersions;
-use super::npm::CUSTOM_NPM_PACKAGE_CACHE;
-use super::npm::CUSTOM_NPM_PACKAGE_CACHE_FOR_PRIVATE_REGISTRY;
+use super::npm;
 use super::std_path;
 use super::testdata_path;
 
@@ -1084,26 +1083,30 @@ async fn main_server(
       );
     }
     _ => {
+      let uri_path = req.uri().path();
       let mut file_path = testdata_path().to_path_buf();
-      file_path.push(&req.uri().path()[1..].replace("%2f", "/"));
+      file_path.push(&uri_path[1..].replace("%2f", "/"));
       if let Ok(file) = tokio::fs::read(&file_path).await {
-        let file_resp = custom_headers(req.uri().path(), file);
+        let file_resp = custom_headers(uri_path, file);
         return Ok(file_resp);
       }
 
       // serve npm registry files
-      if let Some(resp) =
-        try_serve_npm_registry(&req, file_path.clone(), NpmRegistryKind::Public)
-          .await
+      if let Some(resp) = try_serve_npm_registry(
+        uri_path,
+        file_path.clone(),
+        &npm::PUBLIC_TEST_NPM_REGISTRY,
+      )
+      .await
       {
         return resp;
-      } else if let Some(suffix) = req.uri().path().strip_prefix("/deno_std/") {
+      } else if let Some(suffix) = uri_path.strip_prefix("/deno_std/") {
         let file_path = std_path().join(suffix);
         if let Ok(file) = tokio::fs::read(&file_path).await {
-          let file_resp = custom_headers(req.uri().path(), file);
+          let file_resp = custom_headers(uri_path, file);
           return Ok(file_resp);
         }
-      } else if let Some(suffix) = req.uri().path().strip_prefix("/sleep/") {
+      } else if let Some(suffix) = uri_path.strip_prefix("/sleep/") {
         let duration = suffix.parse::<u64>().unwrap();
         tokio::time::sleep(Duration::from_millis(duration)).await;
         return Response::builder()
@@ -1123,11 +1126,6 @@ async fn main_server(
 
 const PRIVATE_NPM_REGISTRY_AUTH_TOKEN: &str = "private-reg-token";
 
-enum NpmRegistryKind {
-  Public,
-  Private,
-}
-
 async fn wrap_private_npm_registry1(port: u16) {
   let npm_registry_addr = SocketAddr::from(([127, 0, 0, 1], port));
   run_server(
@@ -1158,11 +1156,16 @@ async fn private_npm_registry1(
     );
   }
 
-  let mut file_path = testdata_path().to_path_buf();
-  file_path.push(&req.uri().path()[1..].replace("%2f", "/"));
+  let uri_path = req.uri().path();
+  let mut testdata_file_path = testdata_path().to_path_buf();
+  testdata_file_path.push(&uri_path[1..].replace("%2f", "/"));
 
-  if let Some(resp) =
-    try_serve_npm_registry(&req, file_path, NpmRegistryKind::Private).await
+  if let Some(resp) = try_serve_npm_registry(
+    uri_path,
+    testdata_file_path,
+    &npm::PRIVATE_TEST_NPM_REGISTRY_1,
+  )
+  .await
   {
     return resp;
   }
@@ -1175,26 +1178,25 @@ async fn private_npm_registry1(
 
 fn handle_custom_npm_registry_path(
   path: &str,
-  registry_kind: NpmRegistryKind,
+  test_npm_registry: &npm::TestNpmRegistry,
 ) -> Result<Option<Response<UnsyncBoxBody<Bytes, Infallible>>>, anyhow::Error> {
   let parts = path
     .split('/')
     .filter(|p| !p.is_empty())
     .collect::<Vec<_>>();
-  let cache = match registry_kind {
-    NpmRegistryKind::Public => &CUSTOM_NPM_PACKAGE_CACHE,
-    NpmRegistryKind::Private => &CUSTOM_NPM_PACKAGE_CACHE_FOR_PRIVATE_REGISTRY,
-  };
+
   let package_name = format!("@denotest/{}", parts[0]);
   if parts.len() == 2 {
-    if let Some(file_bytes) =
-      cache.tarball_bytes(&package_name, parts[1].trim_end_matches(".tgz"))?
+    if let Some(file_bytes) = test_npm_registry
+      .tarball_bytes(&package_name, parts[1].trim_end_matches(".tgz"))?
     {
       let file_resp = custom_headers("file.tgz", file_bytes);
       return Ok(Some(file_resp));
     }
   } else if parts.len() == 1 {
-    if let Some(registry_file) = cache.registry_file(&package_name)? {
+    if let Some(registry_file) =
+      test_npm_registry.registry_file(&package_name)?
+    {
       let file_resp = custom_headers("registry.json", registry_file);
       return Ok(Some(file_resp));
     }
@@ -1210,19 +1212,16 @@ fn should_download_npm_packages() -> bool {
 }
 
 async fn try_serve_npm_registry(
-  req: &Request<hyper::body::Incoming>,
-  mut file_path: PathBuf,
-  registry_kind: NpmRegistryKind,
+  uri_path: &str,
+  mut testdata_file_path: PathBuf,
+  test_npm_registry: &npm::TestNpmRegistry,
 ) -> Option<Result<Response<UnsyncBoxBody<Bytes, Infallible>>, anyhow::Error>> {
-  if let Some(suffix) = req
-    .uri()
-    .path()
-    .strip_prefix("/npm/registry/@denotest/")
-    .or_else(|| req.uri().path().strip_prefix("/npm/registry/@denotest%2f"))
+  if let Some(suffix) =
+    test_npm_registry.strip_denotest_prefix_from_uri_path(uri_path)
   {
-    // serve all requests to /npm/registry/@deno using the file system
+    // serve all requests to the `DENOTEST_SCOPE_NAME` using the file system
     // at that path
-    match handle_custom_npm_registry_path(suffix, registry_kind) {
+    match handle_custom_npm_registry_path(suffix, test_npm_registry) {
       Ok(Some(response)) => return Some(Ok(response)),
       Ok(None) => {} // ignore, not found
       Err(err) => {
@@ -1234,18 +1233,23 @@ async fn try_serve_npm_registry(
         );
       }
     }
-  } else if req.uri().path().starts_with("/npm/registry/") {
+  } else if test_npm_registry.uri_path_starts_with_registry_path(uri_path) {
     // otherwise, serve based on registry.json and tgz files
-    let is_tarball = req.uri().path().ends_with(".tgz");
+    let is_tarball = uri_path.ends_with(".tgz");
     if !is_tarball {
-      file_path.push("registry.json");
+      testdata_file_path.push("registry.json");
     }
-    if let Ok(file) = tokio::fs::read(&file_path).await {
-      let file_resp = custom_headers(req.uri().path(), file);
+    if let Ok(file) = tokio::fs::read(&testdata_file_path).await {
+      let file_resp = custom_headers(uri_path, file);
       return Some(Ok(file_resp));
     } else if should_download_npm_packages() {
-      if let Err(err) =
-        download_npm_registry_file(req.uri(), &file_path, is_tarball).await
+      if let Err(err) = download_npm_registry_file(
+        test_npm_registry,
+        uri_path,
+        &testdata_file_path,
+        is_tarball,
+      )
+      .await
       {
         return Some(
           Response::builder()
@@ -1256,8 +1260,8 @@ async fn try_serve_npm_registry(
       };
 
       // serve the file
-      if let Ok(file) = tokio::fs::read(&file_path).await {
-        let file_resp = custom_headers(req.uri().path(), file);
+      if let Ok(file) = tokio::fs::read(&testdata_file_path).await {
+        let file_resp = custom_headers(uri_path, file);
         return Some(Ok(file_resp));
       }
     }
@@ -1266,14 +1270,32 @@ async fn try_serve_npm_registry(
   None
 }
 
+// Replaces URL of public npm registry (`https://registry.npmjs.org/`) with
+// the test registry (`http://localhost:4545/npm/registry/`).
+//
+// These strings end up in `registry.json` files for each downloaded package
+// that are stored in `tests/testdata/` directory.
+//
+// If another npm test registry wants to use them, it should replace
+// these values with appropriate URL when serving.
+fn replace_default_npm_registry_url_with_test_npm_registry_url(
+  str_: String,
+  package_name: &str,
+) -> String {
+  str_.replace(
+    &format!("https://registry.npmjs.org/{package_name}/-/"),
+    &format!("http://localhost:4545/npm/registry/{package_name}/"),
+  )
+}
+
 async fn download_npm_registry_file(
-  uri: &hyper::Uri,
-  file_path: &PathBuf,
+  test_npm_registry: &npm::TestNpmRegistry,
+  uri_path: &str,
+  testdata_file_path: &PathBuf,
   is_tarball: bool,
 ) -> Result<(), anyhow::Error> {
-  let url_parts = uri
-    .path()
-    .strip_prefix("/npm/registry/")
+  let url_parts = test_npm_registry
+    .strip_registry_path_prefix_from_uri_path(uri_path)
     .unwrap()
     .split('/')
     .collect::<Vec<_>>();
@@ -1283,7 +1305,7 @@ async fn download_npm_registry_file(
     url_parts.into_iter().take(1).collect::<Vec<_>>().join("/")
   };
   let url = if is_tarball {
-    let file_name = file_path.file_name().unwrap().to_string_lossy();
+    let file_name = testdata_file_path.file_name().unwrap().to_string_lossy();
     format!("https://registry.npmjs.org/{package_name}/-/{file_name}")
   } else {
     format!("https://registry.npmjs.org/{package_name}")
@@ -1294,16 +1316,14 @@ async fn download_npm_registry_file(
   let bytes = if is_tarball {
     bytes.to_vec()
   } else {
-    String::from_utf8(bytes.to_vec())
-      .unwrap()
-      .replace(
-        &format!("https://registry.npmjs.org/{package_name}/-/"),
-        &format!("http://localhost:4545/npm/registry/{package_name}/"),
-      )
-      .into_bytes()
+    replace_default_npm_registry_url_with_test_npm_registry_url(
+      String::from_utf8(bytes.to_vec()).unwrap(),
+      &package_name,
+    )
+    .into_bytes()
   };
-  std::fs::create_dir_all(file_path.parent().unwrap())?;
-  std::fs::write(file_path, bytes)?;
+  std::fs::create_dir_all(testdata_file_path.parent().unwrap())?;
+  std::fs::write(testdata_file_path, bytes)?;
   Ok(())
 }