refactor entity get and 'expansion'

3 files changed, 64 insertions, 81 deletions

diff --git a/rust/src/api_entity_crud.rs b/rust/src/api_entity_crud.rs
index 6bbfcdc4..401b9835 100644
--- a/rust/src/api_entity_crud.rs
+++ b/rust/src/api_entity_crud.rs
@@ -1,4 +1,5 @@
 use api_helpers::*;
+use api_server::get_release_files;
 use chrono;
 use database_models::*;
 use database_schema::*;
@@ -45,6 +46,7 @@ where
     // Generic Methods
     fn db_get(conn: &DbConn, ident: FatCatId) -> Result<Self>;
     fn db_get_rev(conn: &DbConn, rev_id: Uuid) -> Result<Self>;
+    fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()>;
     fn db_create(&self, conn: &DbConn, edit_context: &EditContext) -> Result<Self::EditRow>;
     fn db_create_batch(
         conn: &DbConn,
@@ -114,6 +116,14 @@ macro_rules! generic_db_get_rev {
     };
 }
 
+macro_rules! generic_db_expand {
+    () => {
+        fn db_expand(&mut self, _conn: &DbConn, _expand: ExpandFlags) -> Result<()> {
+            Ok(())
+        }
+    };
+}
+
 macro_rules! generic_db_create {
     // TODO: this path should call generic_db_create_batch
     ($ident_table: ident, $edit_table: ident) => {
@@ -387,13 +397,13 @@ impl EntityCrud for ContainerEntity {
     generic_parse_editgroup_id!();
     generic_db_get!(container_ident, container_rev);
     generic_db_get_rev!(container_rev);
+    generic_db_expand!();
     generic_db_create!(container_ident, container_edit);
     generic_db_create_batch!(container_ident, container_edit);
     generic_db_update!(container_ident, container_edit);
     generic_db_delete!(container_ident, container_edit);
     generic_db_get_history!(container_edit);
     generic_db_accept_edits_batch!("container");
-    //generic_db_accept_edits_each!(container_ident, container_edit);
     generic_db_insert_rev!();
 
     fn db_from_row(
@@ -468,13 +478,13 @@ impl EntityCrud for CreatorEntity {
     generic_parse_editgroup_id!();
     generic_db_get!(creator_ident, creator_rev);
     generic_db_get_rev!(creator_rev);
+    generic_db_expand!();
     generic_db_create!(creator_ident, creator_edit);
     generic_db_create_batch!(creator_ident, creator_edit);
     generic_db_update!(creator_ident, creator_edit);
     generic_db_delete!(creator_ident, creator_edit);
     generic_db_get_history!(creator_edit);
     generic_db_accept_edits_batch!("creator");
-    //generic_db_accept_edits_each!(creator_ident, creator_edit);
     generic_db_insert_rev!();
 
     fn db_from_row(
@@ -546,13 +556,13 @@ impl EntityCrud for FileEntity {
     generic_parse_editgroup_id!();
     generic_db_get!(file_ident, file_rev);
     generic_db_get_rev!(file_rev);
+    generic_db_expand!();
     generic_db_create!(file_ident, file_edit);
     generic_db_create_batch!(file_ident, file_edit);
     generic_db_update!(file_ident, file_edit);
     generic_db_delete!(file_ident, file_edit);
     generic_db_get_history!(file_edit);
     generic_db_accept_edits_batch!("file");
-    //generic_db_accept_edits_each!(file_ident, file_edit);
     generic_db_insert_rev!();
 
     fn db_from_row(
@@ -690,9 +700,26 @@ impl EntityCrud for ReleaseEntity {
     generic_db_delete!(release_ident, release_edit);
     generic_db_get_history!(release_edit);
     generic_db_accept_edits_batch!("release");
-    //generic_db_accept_edits_each!(release_ident, release_edit);
     generic_db_insert_rev!();
 
+    fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()> {
+
+        let ident = match &self.ident {
+            None => bail!("Can't expand a non-concrete entity"),
+            Some(s) => FatCatId::from_str(&s)?
+        };
+        if expand.files {
+            self.files = Some(get_release_files(ident, conn)?);
+        }
+        if expand.container {
+            if let Some(ref cid) = self.container_id {
+                self.container =
+                    Some(ContainerEntity::db_get(conn, FatCatId::from_str(&cid)?)?);
+            }
+        }
+        Ok(())
+    }
+
     fn db_create(&self, conn: &DbConn, edit_context: &EditContext) -> Result<Self::EditRow> {
         let mut edits = Self::db_create_batch(conn, edit_context, &[self])?;
         // probably a more elegant way to destroy the vec and take first element
@@ -1062,13 +1089,13 @@ impl EntityCrud for WorkEntity {
     generic_parse_editgroup_id!();
     generic_db_get!(work_ident, work_rev);
     generic_db_get_rev!(work_rev);
+    generic_db_expand!();
     generic_db_create!(work_ident, work_edit);
     generic_db_create_batch!(work_ident, work_edit);
     generic_db_update!(work_ident, work_edit);
     generic_db_delete!(work_ident, work_edit);
     generic_db_get_history!(work_edit);
     generic_db_accept_edits_batch!("work");
-    //generic_db_accept_edits_each!(work_ident, work_edit);
     generic_db_insert_rev!();
 
     fn db_from_row(
diff --git a/rust/src/api_server.rs b/rust/src/api_server.rs
index dd35b330..f25e6169 100644
--- a/rust/src/api_server.rs
+++ b/rust/src/api_server.rs
@@ -57,15 +57,24 @@ pub struct Server {
     pub db_pool: ConnectionPool,
 }
 
+pub fn get_release_files (
+    id: FatCatId,
+    conn: &DbConn,
+) -> Result<Vec<FileEntity>> {
+    let rows: Vec<(FileRevRow, FileIdentRow, FileReleaseRow)> = file_rev::table
+        .inner_join(file_ident::table)
+        .inner_join(file_release::table)
+        .filter(file_release::target_release_ident_id.eq(&id.to_uuid()))
+        .filter(file_ident::is_live.eq(true))
+        .filter(file_ident::redirect_id.is_null())
+        .load(conn)?;
+
+    rows.into_iter()
+        .map(|(rev, ident, _)| FileEntity::db_from_row(conn, rev, Some(ident)))
+        .collect()
+}
+
 impl Server {
-    pub fn get_container_handler(
-        &self,
-        id: FatCatId,
-        _expand: &Option<String>,
-        conn: &DbConn,
-    ) -> Result<ContainerEntity> {
-        ContainerEntity::db_get(conn, id)
-    }
 
     pub fn lookup_container_handler(&self, issnl: &str, conn: &DbConn) -> Result<ContainerEntity> {
         check_issn(issnl)?;
@@ -82,15 +91,6 @@ impl Server {
         ContainerEntity::db_from_row(conn, rev, Some(ident))
     }
 
-    pub fn get_creator_handler(
-        &self,
-        id: FatCatId,
-        _expand: &Option<String>,
-        conn: &DbConn,
-    ) -> Result<CreatorEntity> {
-        CreatorEntity::db_get(conn, id)
-    }
-
     pub fn lookup_creator_handler(&self, orcid: &str, conn: &DbConn) -> Result<CreatorEntity> {
         check_orcid(orcid)?;
         let (ident, rev): (CreatorIdentRow, CreatorRevRow) = creator_ident::table
@@ -126,15 +126,6 @@ impl Server {
             .collect()
     }
 
-    pub fn get_file_handler(
-        &self,
-        id: FatCatId,
-        _expand: &Option<String>,
-        conn: &DbConn,
-    ) -> Result<FileEntity> {
-        FileEntity::db_get(conn, id)
-    }
-
     pub fn lookup_file_handler(&self, sha1: &str, conn: &DbConn) -> Result<FileEntity> {
         let (ident, rev): (FileIdentRow, FileRevRow) = file_ident::table
             .inner_join(file_rev::table)
@@ -149,25 +140,6 @@ impl Server {
         FileEntity::db_from_row(conn, rev, Some(ident))
     }
 
-    pub fn get_release_handler(
-        &self,
-        id: FatCatId,
-        expand: &Option<String>,
-        conn: &DbConn,
-    ) -> Result<ReleaseEntity> {
-        let mut release = ReleaseEntity::db_get(conn, id)?;
-
-        // For now, if there is any expand param we do them all
-        if expand.is_some() {
-            release.files = Some(self.get_release_files_handler(id, conn)?);
-            if let Some(ref cid) = release.container_id {
-                release.container =
-                    Some(self.get_container_handler(FatCatId::from_str(&cid)?, &None, conn)?);
-            }
-        }
-        Ok(release)
-    }
-
     pub fn lookup_release_handler(&self, doi: &str, conn: &DbConn) -> Result<ReleaseEntity> {
         check_doi(doi)?;
         let (ident, rev): (ReleaseIdentRow, ReleaseRevRow) = release_ident::table
@@ -188,26 +160,7 @@ impl Server {
         id: FatCatId,
         conn: &DbConn,
     ) -> Result<Vec<FileEntity>> {
-        let rows: Vec<(FileRevRow, FileIdentRow, FileReleaseRow)> = file_rev::table
-            .inner_join(file_ident::table)
-            .inner_join(file_release::table)
-            .filter(file_release::target_release_ident_id.eq(&id.to_uuid()))
-            .filter(file_ident::is_live.eq(true))
-            .filter(file_ident::redirect_id.is_null())
-            .load(conn)?;
-
-        rows.into_iter()
-            .map(|(rev, ident, _)| FileEntity::db_from_row(conn, rev, Some(ident)))
-            .collect()
-    }
-
-    pub fn get_work_handler(
-        &self,
-        id: FatCatId,
-        _expand: &Option<String>,
-        conn: &DbConn,
-    ) -> Result<WorkEntity> {
-        WorkEntity::db_get(conn, id)
+        get_release_files(id, conn)
     }
 
     pub fn get_work_releases_handler(
diff --git a/rust/src/api_wrappers.rs b/rust/src/api_wrappers.rs
index 3f070593..ca4081da 100644
--- a/rust/src/api_wrappers.rs
+++ b/rust/src/api_wrappers.rs
@@ -20,7 +20,7 @@ macro_rules! wrap_entity_handlers {
     // stable doesn't have a mechanism to "concat" or generate new identifiers in macros, at least
     // in the context of defining new functions.
     // The only stable approach I know of would be: https://github.com/dtolnay/mashup
-    ($get_fn:ident, $get_handler:ident, $get_resp:ident, $post_fn:ident,
+    ($get_fn:ident, $get_resp:ident, $post_fn:ident,
             $post_resp:ident, $post_batch_fn:ident, $post_batch_handler:ident,
             $post_batch_resp:ident, $update_fn:ident, $update_resp:ident,
             $delete_fn:ident, $delete_resp:ident, $get_history_fn:ident,
@@ -36,7 +36,15 @@ macro_rules! wrap_entity_handlers {
             // No transaction for GET
             let ret = match conn.transaction(|| {
                 let entity_id = FatCatId::from_str(&id)?;
-                self.$get_handler(entity_id, &expand, &conn)
+                match expand {
+                    None => $model::db_get(&conn, entity_id),
+                    Some(param) => {
+                        let expand_flags = ExpandFlags::from_string(&param);
+                        let mut entity = $model::db_get(&conn, entity_id)?;
+                        entity.db_expand(&conn, expand_flags)?;
+                        Ok(entity)
+                    },
+                }
             }) {
                 Ok(entity) =>
                     $get_resp::FoundEntity(entity),
@@ -65,7 +73,7 @@ macro_rules! wrap_entity_handlers {
             let conn = self.db_pool.get().expect("db_pool error");
             let ret = match conn.transaction(|| {
                 let edit_context = make_edit_context(&conn, entity.parse_editgroup_id()?, false)?;
-                Ok(entity.db_create(&conn, &edit_context)?.into_model()?)
+                entity.db_create(&conn, &edit_context)?.into_model()
             }) {
                 Ok(edit) =>
                     $post_resp::CreatedEntity(edit),
@@ -124,7 +132,7 @@ macro_rules! wrap_entity_handlers {
             let ret = match conn.transaction(|| {
                 let entity_id = FatCatId::from_str(&id)?;
                 let edit_context = make_edit_context(&conn, entity.parse_editgroup_id()?, false)?;
-                Ok(entity.db_update(&conn, &edit_context, entity_id)?.into_model()?)
+                entity.db_update(&conn, &edit_context, entity_id)?.into_model()
             }) {
                 Ok(edit) =>
                     $update_resp::UpdatedEntity(edit),
@@ -161,7 +169,7 @@ macro_rules! wrap_entity_handlers {
                     None => None,
                 };
                 let edit_context = make_edit_context(&conn, editgroup_id, false)?;
-                Ok($model::db_delete(&conn, &edit_context, entity_id)?.into_model()?)
+                $model::db_delete(&conn, &edit_context, entity_id)?.into_model()
             }) {
                 Ok(edit) =>
                     $delete_resp::DeletedEntity(edit),
@@ -194,7 +202,7 @@ macro_rules! wrap_entity_handlers {
             // No transaction for GET
             let ret = match conn.transaction(|| {
                 let entity_id = FatCatId::from_str(&id)?;
-                Ok($model::db_get_history(&conn, entity_id, limit)?)
+                $model::db_get_history(&conn, entity_id, limit)
             }) {
                 Ok(history) =>
                     $get_history_resp::FoundEntityHistory(history),
@@ -273,7 +281,6 @@ macro_rules! wrap_fcid_handler {
 impl Api for Server {
     wrap_entity_handlers!(
         get_container,
-        get_container_handler,
         GetContainerResponse,
         create_container,
         CreateContainerResponse,
@@ -291,7 +298,6 @@ impl Api for Server {
 
     wrap_entity_handlers!(
         get_creator,
-        get_creator_handler,
         GetCreatorResponse,
         create_creator,
         CreateCreatorResponse,
@@ -308,7 +314,6 @@ impl Api for Server {
     );
     wrap_entity_handlers!(
         get_file,
-        get_file_handler,
         GetFileResponse,
         create_file,
         CreateFileResponse,
@@ -325,7 +330,6 @@ impl Api for Server {
     );
     wrap_entity_handlers!(
         get_release,
-        get_release_handler,
         GetReleaseResponse,
         create_release,
         CreateReleaseResponse,
@@ -342,7 +346,6 @@ impl Api for Server {
     );
     wrap_entity_handlers!(
         get_work,
-        get_work_handler,
         GetWorkResponse,
         create_work,
         CreateWorkResponse,