From 7f5daa3c0e1e5d0884d0960207e87ff8bad8575d Mon Sep 17 00:00:00 2001
From: Adrian Heine <mail@adrianheine.de>
Date: Mon, 3 Aug 2020 19:28:05 +0200
Subject: [PATCH] Hide Path implementation

---
 src/artifacts/mod.rs | 31 ++++++++++++++++-
 src/builder.rs       | 82 ++++++++++++++++++++------------------------
 src/locator.rs       | 57 +++++++++++++++++-------------
 tests/setup.rs       | 15 ++++++--
 4 files changed, 113 insertions(+), 72 deletions(-)

diff --git a/src/artifacts/mod.rs b/src/artifacts/mod.rs
index c02538b..20ed4fa 100644
--- a/src/artifacts/mod.rs
+++ b/src/artifacts/mod.rs
@@ -1,7 +1,36 @@
 use std::path::{Path as ActualPath, PathBuf};
 
 #[derive(Clone, Debug)]
-pub struct Path(pub PathBuf);
+pub struct Path(PathBuf);
+
+// FIXME: This is a specialization since with Path: Into<PathBuf>
+// it would overwrite impl<T> From <T> for T
+//impl<T: Into<PathBuf>> From<T> for Path {
+//  fn from(v: T) -> Self {
+//    Path(v.into())
+//  }
+//}
+
+macro_rules! path_from {
+  ( $t:ty ) => {
+    impl From<$t> for Path {
+      fn from(v: $t) -> Self {
+        Self(v.into())
+      }
+    }
+  };
+}
+
+path_from!(String);
+path_from!(&str);
+path_from!(PathBuf);
+
+impl From<Path> for PathBuf {
+  fn from(v: Path) -> Self {
+    v.0
+  }
+}
+
 impl AsRef<ActualPath> for Path {
   fn as_ref(&self) -> &ActualPath {
     &self.0
diff --git a/src/builder.rs b/src/builder.rs
index c9bd89e..6bea5c4 100644
--- a/src/builder.rs
+++ b/src/builder.rs
@@ -65,7 +65,7 @@ impl<D> SymbolBuilder<Key<D>> for DefaultBuilder {
     target: &<Key<D> as Resource>::Artifact,
     (): <Self::Prerequisites as ToArtifact>::Artifact,
   ) -> Self::Symbol {
-    KeySymbol::new(StdCommandRunner, target.0.clone())
+    KeySymbol::new(StdCommandRunner, target.clone().into())
   }
 }
 
@@ -84,8 +84,8 @@ impl<D: Clone> SymbolBuilder<Csr<D>> for DefaultBuilder {
     CsrSymbol::new(
       StdCommandRunner,
       resource.0.clone(),
-      key.0,
-      target.0.clone(),
+      key.into(),
+      target.clone().into(),
     )
   }
 }
@@ -124,11 +124,11 @@ impl<D: Clone> SymbolBuilder<Cert<D>> for DefaultBuilder {
     CertSymbol::new(
       resource.0.clone(),
       SetuidCommandRunner::new(user_name.0, &StdCommandRunner),
-      root_cert.0,
-      account_key.0,
-      challenges_dir.0,
-      csr.0,
-      target.0.clone(),
+      root_cert.into(),
+      account_key.into(),
+      challenges_dir.into(),
+      csr.into(),
+      target.clone().into(),
     )
   }
 }
@@ -145,7 +145,7 @@ impl<D: Clone> SymbolBuilder<CertChain<D>> for DefaultBuilder {
     target: &<CertChain<D> as Resource>::Artifact,
     (cert, root_cert): <Self::Prerequisites as ToArtifact>::Artifact,
   ) -> Self::Symbol {
-    ConcatSymbol::new([cert.0, root_cert.0], target.0.clone())
+    ConcatSymbol::new([cert.into(), root_cert.into()], target.clone().into())
   }
 }
 
@@ -161,7 +161,7 @@ impl<D: Clone> SymbolBuilder<KeyAndCertBundle<D>> for DefaultBuilder {
     target: &<KeyAndCertBundle<D> as Resource>::Artifact,
     (cert_chain, key): <Self::Prerequisites as ToArtifact>::Artifact,
   ) -> Self::Symbol {
-    ConcatSymbol::new([key.0, cert_chain.0], target.0.clone())
+    ConcatSymbol::new([key.into(), cert_chain.into()], target.clone().into())
   }
 }
 
@@ -210,7 +210,7 @@ impl SymbolBuilder<DefaultServer> for DefaultBuilder {
   ) -> Self::Symbol {
     (
       FileSymbol::new(
-        target.0.clone(),
+        target.clone().into(),
         nginx::default_server(challenges_snippet_path),
       ),
       ReloadServiceSymbol::new(StdCommandRunner, "nginx"),
@@ -239,14 +239,8 @@ impl<D: AsRef<str> + Clone + Display> SymbolBuilder<ServeCustom<D>> for DefaultB
   ) -> Self::Symbol {
     (
       FileSymbol::new(
-        target.0.clone(),
-        nginx::server_config(
-          &resource.0,
-          cert.0,
-          key.0,
-          &resource.1,
-          challenges_snippet_path,
-        ),
+        target.clone().into(),
+        nginx::server_config(&resource.0, cert, key, &resource.1, challenges_snippet_path),
       ),
       ReloadServiceSymbol::new(StdCommandRunner, "nginx"),
     )
@@ -280,11 +274,11 @@ impl<D: Clone + Display, P: AsRef<Path>> SymbolBuilder<ServePhp<D, P>> for Defau
   ) -> Self::Symbol {
     (
       FileSymbol::new(
-        target.0.clone(),
+        target.clone().into(),
         nginx::server_config(
           &resource.0,
-          cert.0,
-          key.0,
+          cert,
+          key,
           nginx::php_snippet(resource.2, &pool.0, &resource.1) + &resource.3,
           challenges_snippet_path,
         ),
@@ -329,11 +323,11 @@ impl<D: Clone + Display, P: Clone + AsRef<Path>> SymbolBuilder<ServeService<D, P
   ) -> Self::Symbol {
     (
       FileSymbol::new(
-        target.0.clone(),
+        target.clone().into(),
         nginx::server_config(
           &resource.0,
-          cert.0,
-          key.0,
+          cert,
+          key,
           nginx::proxy_snippet(&socket.0, &resource.3),
           challenges_snippet_path,
         ),
@@ -364,11 +358,11 @@ impl<D: AsRef<str> + Clone + Display> SymbolBuilder<ServeRedir<D>> for DefaultBu
   ) -> Self::Symbol {
     (
       FileSymbol::new(
-        target.0.clone(),
+        target.clone().into(),
         nginx::server_config(
           &resource.0,
-          cert.0,
-          key.0,
+          cert,
+          key,
           nginx::redir_snippet(resource.1.as_ref()),
           challenges_snippet_path,
         ),
@@ -401,11 +395,11 @@ impl<D: AsRef<str> + Clone + Display, P: AsRef<Path>> SymbolBuilder<ServeStatic<
   ) -> Self::Symbol {
     (
       FileSymbol::new(
-        target.0.clone(),
+        target.clone().into(),
         nginx::server_config(
           &resource.0,
-          cert.0,
-          key.0,
+          cert,
+          key,
           nginx::static_snippet(resource.1.as_ref()),
           challenges_snippet_path,
         ),
@@ -430,8 +424,8 @@ impl<D: Clone> SymbolBuilder<PhpFpmPool<D>> for DefaultBuilder {
   ) -> Self::Symbol {
     (
       FileSymbol::new(
-        conf_path.0.clone(),
-        php_fpm_pool_config(&user_name.0, &socket_path.0, resource.1),
+        conf_path.clone().into(),
+        php_fpm_pool_config(&user_name.0, &socket_path, resource.1),
       ),
       ReloadServiceSymbol::new(StdCommandRunner, service_name.0.clone()),
     )
@@ -454,7 +448,7 @@ impl<D, P: AsRef<Path>> SymbolBuilder<SystemdSocketService<D, P>> for DefaultBui
   ) -> Self::Symbol {
     (
       FileSymbol::new(
-        conf_path.0.clone(),
+        conf_path.clone().into(),
         if resource.4 {
           systemd_nodejs_service(&resource.2, socket_path, &resource.3)
         } else {
@@ -468,7 +462,7 @@ impl<D, P: AsRef<Path>> SymbolBuilder<SystemdSocketService<D, P>> for DefaultBui
       ),
       SystemdUserSessionSymbol::new(user_name.0.clone(), &StdCommandRunner),
       UserServiceSymbol::new(
-        socket_path.0.clone(),
+        socket_path.clone().into(),
         user_name.0.clone(),
         resource.1,
         &StdCommandRunner,
@@ -517,7 +511,7 @@ impl<P: Clone + AsRef<Path>> SymbolBuilder<StoredDirectory<P>> for DefaultBuilde
   ) -> Self::Symbol {
     SavedDirectorySymbol::new(
       resource.1.clone(),
-      SimpleStorage::new(target.0.clone()),
+      SimpleStorage::new(target.clone().into()),
       StorageDirection::Store,
       StdCommandRunner,
     )
@@ -536,7 +530,7 @@ impl<P: Clone + AsRef<Path>> SymbolBuilder<LoadedDirectory<P>> for DefaultBuilde
   ) -> Self::Symbol {
     SavedDirectorySymbol::new(
       resource.1.clone(),
-      SimpleStorage::new(target.0.clone()),
+      SimpleStorage::new(target.clone().into()),
       StorageDirection::Load,
       StdCommandRunner,
     )
@@ -615,8 +609,8 @@ impl SymbolBuilder<AcmeChallengesDir> for DefaultBuilder {
     user_name: <Self::Prerequisites as ToArtifact>::Artifact,
   ) -> Self::Symbol {
     (
-      DirSymbol::new(target.0.clone()),
-      OwnerSymbol::new(target.0.clone(), user_name.0, StdCommandRunner),
+      DirSymbol::new(target.clone().into()),
+      OwnerSymbol::new(target.clone().into(), user_name.0, StdCommandRunner),
     )
   }
 }
@@ -634,7 +628,7 @@ impl SymbolBuilder<AcmeChallengesNginxSnippet> for DefaultBuilder {
     challenges_dir: <Self::Prerequisites as ToArtifact>::Artifact,
   ) -> Self::Symbol {
     FileSymbol::new(
-      target.0.clone(),
+      target.clone().into(),
       nginx::acme_challenges_snippet(challenges_dir),
     )
   }
@@ -656,8 +650,8 @@ impl SymbolBuilder<AcmeAccountKey> for DefaultBuilder {
     user_name: <Self::Prerequisites as ToArtifact>::Artifact,
   ) -> Self::Symbol {
     (
-      KeySymbol::new(StdCommandRunner, target.0.clone()),
-      OwnerSymbol::new(target.0.clone(), user_name.0, StdCommandRunner),
+      KeySymbol::new(StdCommandRunner, target.clone().into()),
+      OwnerSymbol::new(target.clone().into(), user_name.0, StdCommandRunner),
     )
   }
 }
@@ -672,7 +666,7 @@ impl SymbolBuilder<AcmeRootCert> for DefaultBuilder {
     target: &<AcmeRootCert as Resource>::Artifact,
     (): <Self::Prerequisites as ToArtifact>::Artifact,
   ) -> Self::Symbol {
-    FileSymbol::new(target.0.clone(), LETS_ENCRYPT_X3_CROSS_SIGNED)
+    FileSymbol::new(target.clone().into(), LETS_ENCRYPT_X3_CROSS_SIGNED)
   }
 }
 
@@ -705,7 +699,7 @@ impl<D: Clone> SymbolBuilder<MariaDbDatabase<D>> for DefaultBuilder {
     (db_name, _, data_path): &<MariaDbDatabase<D> as Resource>::Artifact,
     _: <Self::Prerequisites as ToArtifact>::Artifact,
   ) -> Self::Symbol {
-    let db_dump = SimpleStorage::new(data_path.0.clone());
+    let db_dump = SimpleStorage::new(data_path.clone().into());
     (
       MariaDbDatabaseSymbol::new(db_name.0.clone(), db_dump.clone(), &StdCommandRunner),
       MariaDbDumpSymbol::new(db_name.0.clone(), db_dump, &StdCommandRunner),
diff --git a/src/locator.rs b/src/locator.rs
index 4340cdb..eaaddeb 100644
--- a/src/locator.rs
+++ b/src/locator.rs
@@ -54,7 +54,7 @@ impl<P, D: AsRef<str>> ResourceLocator<Key<D>> for DefaultLocator<P> {
   type Prerequisites = Dir<PathBuf>;
   fn locate(resource: &Key<D>) -> (<Key<D> as Resource>::Artifact, Self::Prerequisites) {
     (
-      PathArtifact(format!("/etc/ssl/private/{}.key", resource.0.as_ref()).into()),
+      PathArtifact::from(format!("/etc/ssl/private/{}.key", resource.0.as_ref())),
       Dir("/etc/ssl/private".into()),
     )
   }
@@ -64,7 +64,7 @@ impl<P, D: AsRef<str>> ResourceLocator<Csr<D>> for DefaultLocator<P> {
   type Prerequisites = Dir<PathBuf>;
   fn locate(resource: &Csr<D>) -> (<Csr<D> as Resource>::Artifact, Self::Prerequisites) {
     (
-      PathArtifact(format!("/etc/ssl/local_certs/{}.csr", resource.0.as_ref()).into()),
+      PathArtifact::from(format!("/etc/ssl/local_certs/{}.csr", resource.0.as_ref())),
       Dir("/etc/ssl/local_certs".into()),
     )
   }
@@ -74,7 +74,7 @@ impl<P, D: AsRef<str>> ResourceLocator<Cert<D>> for DefaultLocator<P> {
   type Prerequisites = Dir<PathBuf>;
   fn locate(resource: &Cert<D>) -> (<Cert<D> as Resource>::Artifact, Self::Prerequisites) {
     (
-      PathArtifact(format!("/etc/ssl/local_certs/{}.crt", resource.0.as_ref()).into()),
+      PathArtifact::from(format!("/etc/ssl/local_certs/{}.crt", resource.0.as_ref())),
       Dir("/etc/ssl/local_certs".into()),
     )
   }
@@ -86,7 +86,10 @@ impl<P, D: AsRef<str>> ResourceLocator<CertChain<D>> for DefaultLocator<P> {
     resource: &CertChain<D>,
   ) -> (<CertChain<D> as Resource>::Artifact, Self::Prerequisites) {
     (
-      PathArtifact(format!("/etc/ssl/local_certs/{}.chained.crt", resource.0.as_ref()).into()),
+      PathArtifact::from(format!(
+        "/etc/ssl/local_certs/{}.chained.crt",
+        resource.0.as_ref()
+      )),
       Dir("/etc/ssl/local_certs".into()),
     )
   }
@@ -101,7 +104,10 @@ impl<P, D: AsRef<str>> ResourceLocator<KeyAndCertBundle<D>> for DefaultLocator<P
     Self::Prerequisites,
   ) {
     (
-      PathArtifact(format!("/etc/ssl/private/{}.with_key.crt", resource.0.as_ref()).into()),
+      PathArtifact::from(format!(
+        "/etc/ssl/private/{}.with_key.crt",
+        resource.0.as_ref()
+      )),
       Dir("/etc/ssl/private".into()),
     )
   }
@@ -152,7 +158,7 @@ impl<POLICY: Policy, P: AsRef<Path>> ResourceLocator<StoredDirectory<P>>
     <StoredDirectory<P> as Resource>::Artifact,
     Self::Prerequisites,
   ) {
-    (PathArtifact(POLICY::path_for_data(resource.0)), ())
+    (PathArtifact::from(POLICY::path_for_data(resource.0)), ())
   }
 }
 
@@ -167,7 +173,7 @@ impl<POLICY: Policy, P: AsRef<Path>> ResourceLocator<LoadedDirectory<P>>
     Self::Prerequisites,
   ) {
     (
-      PathArtifact(POLICY::path_for_data(resource.0)),
+      PathArtifact::from(POLICY::path_for_data(resource.0)),
       Dir(resource.1.as_ref().parent().unwrap().into()),
     )
   }
@@ -180,7 +186,7 @@ impl<P: Policy> ResourceLocator<AcmeAccountKey> for DefaultLocator<P> {
   ) -> (<AcmeAccountKey as Resource>::Artifact, Self::Prerequisites) {
     let acme_user = P::acme_user();
     let home = P::user_home(acme_user);
-    (PathArtifact(home.join("account.key")), Dir(home))
+    (PathArtifact::from(home.join("account.key")), Dir(home))
   }
 }
 
@@ -202,7 +208,7 @@ impl<P: Policy> ResourceLocator<AcmeChallengesDir> for DefaultLocator<P> {
   ) {
     let acme_user = P::acme_user();
     let home = P::user_home(acme_user);
-    (PathArtifact(home.join("challenges")), Dir(home))
+    (PathArtifact::from(home.join("challenges")), Dir(home))
   }
 }
 
@@ -215,7 +221,7 @@ impl<P: Policy> ResourceLocator<AcmeChallengesNginxSnippet> for DefaultLocator<P
     Self::Prerequisites,
   ) {
     (
-      PathArtifact("/etc/nginx/snippets/acme-challenge.conf".into()),
+      PathArtifact::from("/etc/nginx/snippets/acme-challenge.conf"),
       (),
     )
   }
@@ -229,7 +235,7 @@ impl<P: Policy> ResourceLocator<AcmeRootCert> for DefaultLocator<P> {
     let acme_user = P::acme_user();
     let home = P::user_home(acme_user);
     (
-      PathArtifact(home.join("lets_encrypt_x3_cross_signed.pem")),
+      PathArtifact::from(home.join("lets_encrypt_x3_cross_signed.pem")),
       Dir(home),
     )
   }
@@ -245,7 +251,7 @@ impl<P: Policy, D: AsRef<str>> ResourceLocator<UserForDomain<D>> for DefaultLoca
   ) {
     let user_name = P::user_name_for_domain(resource.0.as_ref());
     let home = P::user_home(&user_name);
-    ((UserNameArtifact(user_name), PathArtifact(home)), ())
+    ((UserNameArtifact(user_name), PathArtifact::from(home)), ())
   }
 }
 
@@ -268,7 +274,7 @@ impl<P> ResourceLocator<DefaultServer> for DefaultLocator<P> {
   fn locate(
     _resource: &DefaultServer,
   ) -> (<DefaultServer as Resource>::Artifact, Self::Prerequisites) {
-    (PathArtifact("/etc/nginx/sites-enabled/default".into()), ())
+    (PathArtifact::from("/etc/nginx/sites-enabled/default"), ())
   }
 }
 
@@ -278,7 +284,7 @@ impl<D: AsRef<Path>, POLICY> ResourceLocator<ServeCustom<D>> for DefaultLocator<
     resource: &ServeCustom<D>,
   ) -> (<ServeCustom<D> as Resource>::Artifact, Self::Prerequisites) {
     (
-      PathArtifact(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
+      PathArtifact::from(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
       (),
     )
   }
@@ -290,7 +296,7 @@ impl<D: AsRef<Path>, P, POLICY> ResourceLocator<ServePhp<D, P>> for DefaultLocat
     resource: &ServePhp<D, P>,
   ) -> (<ServePhp<D, P> as Resource>::Artifact, Self::Prerequisites) {
     (
-      PathArtifact(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
+      PathArtifact::from(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
       (),
     )
   }
@@ -305,7 +311,7 @@ impl<D: AsRef<Path>, P, POLICY> ResourceLocator<ServeService<D, P>> for DefaultL
     Self::Prerequisites,
   ) {
     (
-      PathArtifact(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
+      PathArtifact::from(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
       (),
     )
   }
@@ -317,7 +323,7 @@ impl<D: AsRef<Path>, POLICY> ResourceLocator<ServeRedir<D>> for DefaultLocator<P
     resource: &ServeRedir<D>,
   ) -> (<ServeRedir<D> as Resource>::Artifact, Self::Prerequisites) {
     (
-      PathArtifact(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
+      PathArtifact::from(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
       (),
     )
   }
@@ -332,7 +338,7 @@ impl<D: AsRef<Path>, P, POLICY> ResourceLocator<ServeStatic<D, P>> for DefaultLo
     Self::Prerequisites,
   ) {
     (
-      PathArtifact(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
+      PathArtifact::from(("/etc/nginx/sites-enabled/".as_ref() as &Path).join(&resource.0)),
       (),
     )
   }
@@ -347,8 +353,11 @@ impl<D: Clone + AsRef<str>, P: Policy> ResourceLocator<PhpFpmPool<D>> for Defaul
     let php_version = P::php_version();
     (
       (
-        PathArtifact(format!("/run/php/{}.sock", user.0).into()),
-        PathArtifact(format!("/etc/php/{}/fpm/pool.d/{}.conf", php_version, user.0).into()),
+        PathArtifact::from(format!("/run/php/{}.sock", user.0)),
+        PathArtifact::from(format!(
+          "/etc/php/{}/fpm/pool.d/{}.conf",
+          php_version, user.0
+        )),
         user,
         ServiceNameArtifact(format!("php{}-fpm", php_version)),
       ),
@@ -368,12 +377,12 @@ impl<D: Clone + AsRef<str>, P, POLICY: Policy> ResourceLocator<SystemdSocketServ
     Self::Prerequisites,
   ) {
     let ((user_name, home_path), ()) = Self::locate(&UserForDomain(&resource.0));
-    let config = home_path.0.join(".config");
+    let config = home_path.as_ref().join(".config");
     let service_dir_path = config.join("systemd/user");
     (
       (
-        PathArtifact(format!("/var/tmp/{}-{}.socket", user_name.0, resource.1).into()),
-        PathArtifact(service_dir_path.join(format!("{}.service", resource.1))),
+        PathArtifact::from(format!("/var/tmp/{}-{}.socket", user_name.0, resource.1)),
+        PathArtifact::from(service_dir_path.join(format!("{}.service", resource.1))),
         user_name.clone(),
       ),
       (Dir(service_dir_path), Owner(user_name.0, config)),
@@ -394,7 +403,7 @@ impl<D: AsRef<str>, P: Policy> ResourceLocator<MariaDbDatabase<D>> for DefaultLo
       (
         DatabaseNameArtifact(user_name.0.clone()),
         user_name.clone(),
-        PathArtifact(P::path_for_data(format!("{}.sql", user_name.0))),
+        PathArtifact::from(P::path_for_data(format!("{}.sql", user_name.0))),
       ),
       (),
     )
diff --git a/tests/setup.rs b/tests/setup.rs
index b8aadf7..52e7aaf 100644
--- a/tests/setup.rs
+++ b/tests/setup.rs
@@ -30,11 +30,17 @@ fn runs_only_once() {
   };
   let mut setup = Setup::new(runner);
   assert_eq!(
-    (setup.add(Csr("somehost")).unwrap().0).0.to_str().unwrap(),
+    (setup.add(Csr("somehost")).unwrap().0)
+      .as_ref()
+      .to_str()
+      .unwrap(),
     "/etc/ssl/local_certs/somehost.csr",
   );
   assert_eq!(
-    (setup.add(Csr("somehost")).unwrap().0).0.to_str().unwrap(),
+    (setup.add(Csr("somehost")).unwrap().0)
+      .as_ref()
+      .to_str()
+      .unwrap(),
     "/etc/ssl/local_certs/somehost.csr",
   );
   assert_eq!(*count.borrow(), 2 + 5); // Key and CSR + 5 dirs
@@ -48,7 +54,10 @@ fn can_create_an_acme_cert() {
   };
   let mut setup = Setup::new(runner);
   assert_eq!(
-    (setup.add(Cert("somehost")).unwrap().0).0.to_str().unwrap(),
+    (setup.add(Cert("somehost")).unwrap().0)
+      .as_ref()
+      .to_str()
+      .unwrap(),
     "/etc/ssl/local_certs/somehost.crt",
   );
   assert_eq!(*count.borrow(), 15);