From 00ffb540d728be9be6435c853fc008450a7fb3d3 Mon Sep 17 00:00:00 2001 From: Clay McLeod Date: Sun, 15 Oct 2023 20:57:32 -0500 Subject: [PATCH] revise: removes the null-based metadata fields --- packages/ccdi-models/src/metadata/field.rs | 92 ------------------- packages/ccdi-models/src/subject/metadata.rs | 62 +++++-------- .../src/subject/metadata/builder.rs | 53 ++++------- packages/ccdi-openapi/src/api.rs | 6 -- packages/ccdi-server/src/responses/subject.rs | 2 +- packages/ccdi-server/src/routes/subject.rs | 22 ++--- swagger.yml | 54 +++-------- 7 files changed, 64 insertions(+), 227 deletions(-) diff --git a/packages/ccdi-models/src/metadata/field.rs b/packages/ccdi-models/src/metadata/field.rs index ad7390c..1feeef2 100644 --- a/packages/ccdi-models/src/metadata/field.rs +++ b/packages/ccdi-models/src/metadata/field.rs @@ -25,95 +25,3 @@ pub enum Field { /// An unowned field. Unowned(unowned::Field), } - -macro_rules! unowned_field_or_null { - ($name: ident, $as: ty, $inner: ty) => { - /// An unowned metadata field or null. - #[derive(Clone, Debug, Deserialize, Eq, Serialize, PartialEq, ToSchema)] - #[schema(as = $as)] - #[serde(untagged)] - pub enum $name { - /// An unowned value representing the field. - #[schema(value_type = field::$inner)] - Unowned($inner), - - /// A null field. - Null, - } - }; -} - -unowned_field_or_null!(SexOrNull, field::SexOrNull, Sex); -unowned_field_or_null!(EthnicityOrNull, field::EthnicityOrNull, Ethnicity); - -macro_rules! multiple_unowned_fields_or_null { - ($name: ident, $as: ty, $inner: ty) => { - /// Multiple unowned metadata fields or null. - #[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq, ToSchema)] - #[schema(as = $as)] - #[serde(untagged)] - pub enum $name { - /// Multiple unowned values representing the field(s). - #[schema(value_type = Vec)] - MultipleUnowned(Vec<$inner>), - - /// A null field. - Null, - } - }; -} - -multiple_unowned_fields_or_null!(RacesOrNull, field::RacesOrNull, Race); - -#[allow(unused_macros)] -macro_rules! owned_field_or_null { - ($name: ident, $as: ty, $inner: ty) => { - /// An owned metadata field. - #[derive(Clone, Debug, Deserialize, Eq, Serialize, PartialEq, ToSchema)] - #[schema(as = $as)] - #[serde(untagged)] - pub enum $name { - /// An owned value representing the field. - #[schema(value_type = field::$inner)] - Owned($inner), - - /// A null field. - Null, - } - }; -} - -macro_rules! multiple_owned_fields_or_null { - ($name: ident, $as: ty, $inner: ty) => { - /// Multiple owned metadata fields or null. - #[derive(Clone, Debug, Deserialize, Eq, Serialize, PartialEq, ToSchema)] - #[schema(as = $as)] - #[serde(untagged)] - pub enum $name { - /// Multiple owned values representing the field(s). - #[schema(value_type = Vec)] - MultipleOwned(Vec<$inner>), - - /// A null field. - Null, - } - }; -} - -multiple_owned_fields_or_null!(IdentifiersOrNull, field::IdentifiersOrNull, Identifier); - -#[cfg(test)] -mod tests { - use serde_test::assert_tokens; - use serde_test::Token; - - use super::*; - - #[test] - fn it_serializes_null_correctly() { - let field = SexOrNull::Null; - - assert_tokens(&field, &[Token::Unit]); - assert_eq!(serde_json::to_string(&field).unwrap(), "null"); - } -} diff --git a/packages/ccdi-models/src/subject/metadata.rs b/packages/ccdi-models/src/subject/metadata.rs index b1d5413..59f4c85 100644 --- a/packages/ccdi-models/src/subject/metadata.rs +++ b/packages/ccdi-models/src/subject/metadata.rs @@ -5,10 +5,6 @@ use serde::Serialize; use utoipa::ToSchema; use crate::metadata::field; -use crate::metadata::field::EthnicityOrNull; -use crate::metadata::field::IdentifiersOrNull; -use crate::metadata::field::RacesOrNull; -use crate::metadata::field::SexOrNull; use ccdi_cde as cde; @@ -21,23 +17,23 @@ pub use builder::Builder; #[schema(as = models::subject::Metadata)] pub struct Metadata { /// The sex of the subject. - #[schema(value_type = field::SexOrNull, nullable = true)] - sex: SexOrNull, + #[schema(value_type = field::Sex, nullable = true)] + sex: Option, /// The race(s) of the subject. - #[schema(value_type = field::RacesOrNull, nullable = true)] - race: RacesOrNull, + #[schema(value_type = Vec, nullable = true)] + race: Option>, /// The ethnicity of the subject. - #[schema(value_type = field::EthnicityOrNull, nullable = true)] - ethnicity: EthnicityOrNull, + #[schema(value_type = field::Ethnicity, nullable = true)] + ethnicity: Option, /// The identifiers for the subject. /// /// Note that this list of identifiers *must* include the main identifier /// for the [`Subject`]. - #[schema(value_type = field::IdentifiersOrNull, nullable = true)] - identifiers: IdentifiersOrNull, + #[schema(value_type = Vec, nullable = true)] + identifiers: Option>, } impl Metadata { @@ -49,8 +45,7 @@ impl Metadata { /// use ccdi_cde as cde; /// use ccdi_models as models; /// - /// use models::metadata::field::SexOrNull; - /// use models::metadata::field::unowned::Sex; + /// use models::metadata::field::Sex; /// use models::subject::metadata::Builder; /// /// let metadata = Builder::default() @@ -59,12 +54,10 @@ impl Metadata { /// /// assert_eq!( /// metadata.sex(), - /// &SexOrNull::Unowned( - /// Sex::new(cde::v1::Sex::Female, None, None) - /// ) + /// &Some(Sex::new(cde::v1::Sex::Female, None, None)) /// ); /// ``` - pub fn sex(&self) -> &SexOrNull { + pub fn sex(&self) -> &Option { &self.sex } @@ -76,8 +69,7 @@ impl Metadata { /// use ccdi_cde as cde; /// use ccdi_models as models; /// - /// use models::metadata::field::RacesOrNull; - /// use models::metadata::field::unowned::Race; + /// use models::metadata::field::Race; /// use models::subject::metadata::Builder; /// /// let metadata = Builder::default() @@ -86,14 +78,10 @@ impl Metadata { /// /// assert_eq!( /// metadata.race(), - /// &RacesOrNull::MultipleUnowned( - /// vec![ - /// Race::new(cde::v1::Race::Asian, None, None) - /// ] - /// ) + /// &Some(vec![Race::new(cde::v1::Race::Asian, None, None)]) /// ); /// ``` - pub fn race(&self) -> &RacesOrNull { + pub fn race(&self) -> &Option> { &self.race } @@ -105,8 +93,7 @@ impl Metadata { /// use ccdi_cde as cde; /// use ccdi_models as models; /// - /// use models::metadata::field::EthnicityOrNull; - /// use models::metadata::field::unowned::Ethnicity; + /// use models::metadata::field::Ethnicity; /// use models::subject::metadata::Builder; /// /// let metadata = Builder::default() @@ -115,10 +102,10 @@ impl Metadata { /// /// assert_eq!( /// metadata.ethnicity(), - /// &EthnicityOrNull::Unowned(Ethnicity::new(cde::v2::Ethnicity::NotHispanicOrLatino, None, None)) + /// &Some(Ethnicity::new(cde::v2::Ethnicity::NotHispanicOrLatino, None, None)) /// ); /// ``` - pub fn ethnicity(&self) -> &EthnicityOrNull { + pub fn ethnicity(&self) -> &Option { &self.ethnicity } @@ -130,8 +117,7 @@ impl Metadata { /// use ccdi_cde as cde; /// use ccdi_models as models; /// - /// use models::metadata::field::IdentifiersOrNull; - /// use models::metadata::field::owned::Identifier; + /// use models::metadata::field::Identifier; /// use models::subject::metadata::Builder; /// /// let metadata = Builder::default() @@ -145,7 +131,7 @@ impl Metadata { /// /// assert_eq!( /// metadata.identifiers(), - /// &IdentifiersOrNull::MultipleOwned( + /// &Some( /// vec![ /// Identifier::new( /// cde::v1::Identifier::parse("organization:Name", ":").unwrap(), @@ -155,7 +141,7 @@ impl Metadata { /// ) /// ); /// ``` - pub fn identifiers(&self) -> &IdentifiersOrNull { + pub fn identifiers(&self) -> &Option> { &self.identifiers } @@ -174,10 +160,10 @@ impl Metadata { /// ``` pub fn random(identifier: cde::v1::Identifier) -> Metadata { Metadata { - sex: SexOrNull::Unowned(rand::random()), - race: RacesOrNull::MultipleUnowned(vec![rand::random()]), - ethnicity: EthnicityOrNull::Unowned(rand::random()), - identifiers: IdentifiersOrNull::MultipleOwned(vec![field::owned::Identifier::new( + sex: Some(rand::random()), + race: Some(vec![rand::random()]), + ethnicity: Some(rand::random()), + identifiers: Some(vec![field::owned::Identifier::new( identifier, None, None, diff --git a/packages/ccdi-models/src/subject/metadata/builder.rs b/packages/ccdi-models/src/subject/metadata/builder.rs index 6be5524..d6bfe47 100644 --- a/packages/ccdi-models/src/subject/metadata/builder.rs +++ b/packages/ccdi-models/src/subject/metadata/builder.rs @@ -1,26 +1,22 @@ //! A builder for [`Metadata`]. use crate::metadata::field; -use crate::metadata::field::EthnicityOrNull; -use crate::metadata::field::IdentifiersOrNull; -use crate::metadata::field::RacesOrNull; -use crate::metadata::field::SexOrNull; use crate::subject::Metadata; /// A builder for [`Metadata`]. #[derive(Clone, Debug, Default)] pub struct Builder { /// The sex of the subject. - sex: Option, + sex: Option, /// The race of the subject. - race: Option>, + race: Option>, /// The ethnicity of the subject. - ethnicity: Option, + ethnicity: Option, /// The identifiers for the subject. - identifiers: Option>, + identifiers: Option>, } impl Builder { @@ -32,13 +28,13 @@ impl Builder { /// use ccdi_cde as cde; /// use ccdi_models as models; /// - /// use models::metadata::field::unowned::Sex; + /// use models::metadata::field::Sex; /// use models::subject::metadata::Builder; /// /// let field = Sex::new(cde::v1::Sex::Unknown, None, None); /// let builder = Builder::default().sex(field); /// ``` - pub fn sex(mut self, sex: field::unowned::Sex) -> Self { + pub fn sex(mut self, sex: field::Sex) -> Self { self.sex = Some(sex); self } @@ -51,13 +47,13 @@ impl Builder { /// use ccdi_cde as cde; /// use ccdi_models as models; /// - /// use models::metadata::field::unowned::Race; + /// use models::metadata::field::Race; /// use models::subject::metadata::Builder; /// /// let field = Race::new(cde::v1::Race::Unknown, None, None); /// let builder = Builder::default().append_race(field); /// ``` - pub fn append_race(mut self, race: field::unowned::Race) -> Self { + pub fn append_race(mut self, race: field::Race) -> Self { let mut inner = self.race.unwrap_or_default(); inner.push(race); @@ -74,13 +70,13 @@ impl Builder { /// use ccdi_cde as cde; /// use ccdi_models as models; /// - /// use models::metadata::field::unowned::Ethnicity; + /// use models::metadata::field::Ethnicity; /// use models::subject::metadata::Builder; /// /// let field = Ethnicity::new(cde::v2::Ethnicity::Unknown, None, None); /// let builder = Builder::default().ethnicity(field); /// ``` - pub fn ethnicity(mut self, ethnicity: field::unowned::Ethnicity) -> Self { + pub fn ethnicity(mut self, ethnicity: field::Ethnicity) -> Self { self.ethnicity = Some(ethnicity); self } @@ -93,7 +89,7 @@ impl Builder { /// use ccdi_cde as cde; /// use ccdi_models as models; /// - /// use models::metadata::field::owned::Identifier; + /// use models::metadata::field::Identifier; /// use models::subject::metadata::Builder; /// /// let field = Identifier::new( @@ -107,7 +103,7 @@ impl Builder { /// /// # Ok::<(), Box>(()) /// ``` - pub fn append_identifier(mut self, identifier: field::owned::Identifier) -> Self { + pub fn append_identifier(mut self, identifier: field::Identifier) -> Self { let mut inner = self.identifiers.unwrap_or_default(); inner.push(identifier); @@ -128,28 +124,11 @@ impl Builder { /// let builder = Builder::default(); /// ``` pub fn build(self) -> Metadata { - let sex = self.sex.map(SexOrNull::Unowned).unwrap_or(SexOrNull::Null); - - let race = self - .race - .map(RacesOrNull::MultipleUnowned) - .unwrap_or(RacesOrNull::Null); - - let ethnicity = self - .ethnicity - .map(EthnicityOrNull::Unowned) - .unwrap_or(EthnicityOrNull::Null); - - let identifiers = self - .identifiers - .map(IdentifiersOrNull::MultipleOwned) - .unwrap_or(IdentifiersOrNull::Null); - Metadata { - sex, - race, - ethnicity, - identifiers, + sex: self.sex, + race: self.race, + ethnicity: self.ethnicity, + identifiers: self.identifiers, } } } diff --git a/packages/ccdi-openapi/src/api.rs b/packages/ccdi-openapi/src/api.rs index ac5e232..4371572 100644 --- a/packages/ccdi-openapi/src/api.rs +++ b/packages/ccdi-openapi/src/api.rs @@ -79,12 +79,6 @@ a variety of query parameters.", field::Ethnicity, field::Identifier, - // Fields or null. - field::SexOrNull, - field::RacesOrNull, - field::EthnicityOrNull, - field::IdentifiersOrNull, - // Models. models::Subject, models::subject::Kind, diff --git a/packages/ccdi-server/src/responses/subject.rs b/packages/ccdi-server/src/responses/subject.rs index 993034c..eda86c5 100644 --- a/packages/ccdi-server/src/responses/subject.rs +++ b/packages/ccdi-server/src/responses/subject.rs @@ -12,7 +12,7 @@ use models::count::Total; pub struct Subject { /// Subject. #[serde(flatten)] - inner: models::Subject + inner: models::Subject, } /// A response representing multiple subjects known about by the server with a diff --git a/packages/ccdi-server/src/routes/subject.rs b/packages/ccdi-server/src/routes/subject.rs index f230cb3..823502c 100644 --- a/packages/ccdi-server/src/routes/subject.rs +++ b/packages/ccdi-server/src/routes/subject.rs @@ -9,8 +9,6 @@ use actix_web::web::ServiceConfig; use actix_web::HttpResponse; use actix_web::Responder; use indexmap::IndexMap; -use models::metadata::field::RacesOrNull; -use models::metadata::field::SexOrNull; use serde_json::Value; use ccdi_cde as cde; @@ -182,23 +180,21 @@ pub async fn subjects_by_count(path: Path, subjects: Data) -> imp fn parse_field(field: &str, subject: &Subject) -> Option { match field { "sex" => match subject.metadata() { - Some(metadata) => match metadata.sex() { - SexOrNull::Unowned(unowned) => Some(Value::String(unowned.value().to_string())), - SexOrNull::Null => Some(Value::Null), - }, + Some(metadata) => metadata + .sex() + .as_ref() + .map(|sex| Value::String(sex.value().to_string())), None => None, }, "race" => match subject.metadata() { - Some(metadata) => match metadata.race() { - RacesOrNull::MultipleUnowned(unowned) => Some(Value::String( - unowned - .iter() + Some(metadata) => metadata.race().as_ref().map(|race| { + Value::String( + race.iter() .map(|race| race.value().to_string()) .collect::>() .join(" AND "), - )), - RacesOrNull::Null => Some(Value::Null), - }, + ) + }), None => None, }, _ => None, diff --git a/swagger.yml b/swagger.yml index 7d73f12..bc03514 100644 --- a/swagger.yml +++ b/swagger.yml @@ -219,13 +219,6 @@ components: type: string description: A free-text comment field. nullable: true - field.EthnicityOrNull: - oneOf: - - $ref: '#/components/schemas/field.Ethnicity' - - type: object - default: null - nullable: true - description: An unowned metadata field or null. field.Identifier: type: object required: @@ -251,16 +244,6 @@ components: type: boolean description: Whether or not the field is owned by the source server. nullable: true - field.IdentifiersOrNull: - oneOf: - - type: array - items: - $ref: '#/components/schemas/field.Identifier' - description: Multiple owned values representing the field(s). - - type: object - default: null - nullable: true - description: Multiple owned metadata fields or null. field.Race: type: object required: @@ -282,16 +265,6 @@ components: type: string description: A free-text comment field. nullable: true - field.RacesOrNull: - oneOf: - - type: array - items: - $ref: '#/components/schemas/field.Race' - description: Multiple unowned values representing the field(s). - - type: object - default: null - nullable: true - description: Multiple unowned metadata fields or null. field.Sex: type: object required: @@ -313,13 +286,6 @@ components: type: string description: A free-text comment field. nullable: true - field.SexOrNull: - oneOf: - - $ref: '#/components/schemas/field.Sex' - - type: object - default: null - nullable: true - description: An unowned metadata field or null. models.Subject: type: object description: A subject. @@ -371,19 +337,27 @@ components: properties: sex: allOf: - - $ref: '#/components/schemas/field.SexOrNull' + - $ref: '#/components/schemas/field.Sex' nullable: true race: - allOf: - - $ref: '#/components/schemas/field.RacesOrNull' + type: array + items: + $ref: '#/components/schemas/field.Race' + description: The race(s) of the subject. nullable: true ethnicity: allOf: - - $ref: '#/components/schemas/field.EthnicityOrNull' + - $ref: '#/components/schemas/field.Ethnicity' nullable: true identifiers: - allOf: - - $ref: '#/components/schemas/field.IdentifiersOrNull' + type: array + items: + $ref: '#/components/schemas/field.Identifier' + description: |- + The identifiers for the subject. + + Note that this list of identifiers *must* include the main identifier + for the [`Subject`]. nullable: true responses.Error: type: object