Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(psl): add deprecation warning for the "referentialIntegrity" attribute #3449

Merged
merged 2 commits into from
Nov 28, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions psl/diagnostics/src/warning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ impl DatamodelWarning {
Self::new(message, span)
}

pub fn new_referential_integrity_attr_deprecation_warning(span: Span) -> DatamodelWarning {
let message = "The `referentialIntegrity` attribute is deprecated. Please use `relationMode` instead. Learn more at https://pris.ly/d/relation-mode";
Self::new(message.to_string(), span)
}

pub fn new_missing_index_on_emulated_relation(span: Span) -> DatamodelWarning {
let message = indoc!(
r#"
Expand Down
68 changes: 51 additions & 17 deletions psl/psl-core/src/validate/datasource_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ use crate::{
diagnostics::{DatamodelError, Diagnostics},
Datasource,
};
use parser_database::{ast::WithDocumentation, coerce, coerce_array, coerce_opt};
use diagnostics::DatamodelWarning;
use parser_database::{
ast::{Expression, WithDocumentation},
coerce, coerce_array, coerce_opt,
};
use std::{borrow::Cow, collections::HashMap};

const PREVIEW_FEATURES_KEY: &str = "previewFeatures";
Expand Down Expand Up @@ -185,47 +189,77 @@ fn lift_datasource(
})
}

/// Returns the relation mode for the datasource, validating against invalid relation mode settings and
/// the deprecated `referentialIntegrity` attribute.
fn get_relation_mode(
args: &mut HashMap<&str, (Span, &ast::Expression)>,
source: &SourceConfig,
diagnostics: &mut Diagnostics,
connector: &'static dyn crate::datamodel_connector::Connector,
) -> Option<RelationMode> {
match (args.remove("relationMode"), args.remove("referentialIntegrity")) {
(None, None) => None,
(Some(_), Some((span, _))) => {
diagnostics.push_error(DatamodelError::new_referential_integrity_and_relation_mode_cooccur_error(span));
None
}
(Some((_span, rm)), None) | (None, Some((_span, rm))) => {
let integrity = match coerce::string(rm, diagnostics)? {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the code down here to the new get_and_validate_relation_mode_value and validate_allowed_relation_mode_settings closures, as they're used both when only the relationMode attribute is used or when only the referentialIntegrity attribute is used

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are mixing concerns and making the code more complicated here. Would something much simpler like if args.contains("referentialIntegrity") { ... push the warning here ... } at the beginning of the function, before the match works. It would emit two warnings in case you defined both, but that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, done

"prisma" => RelationMode::Prisma,
"foreignKeys" => RelationMode::ForeignKeys,
let get_and_validate_relation_mode_value =
|rm: &Expression, diagnostics: &mut Diagnostics| -> Option<RelationMode> {
match coerce::string(rm, diagnostics)? {
"prisma" => Some(RelationMode::Prisma),
"foreignKeys" => Some(RelationMode::ForeignKeys),
other => {
let message = format!(
"Invalid relation mode setting: \"{other}\". Supported values: \"prisma\", \"foreignKeys\"",
);
let error = DatamodelError::new_source_validation_error(&message, "relationMode", source.span);
diagnostics.push_error(error);
return None;
None
}
};
}
};

if !connector.allowed_relation_mode_settings().contains(integrity) {
let validate_allowed_relation_mode_settings =
|rm: &Expression, relation_mode: RelationMode, diagnostics: &mut Diagnostics| {
if !connector.allowed_relation_mode_settings().contains(relation_mode) {
let supported_values = connector
.allowed_relation_mode_settings()
.iter()
.map(|v| format!(r#""{}""#, v))
.collect::<Vec<_>>()
.join(", ");

let message =
format!("Invalid relation mode setting: \"{integrity}\". Supported values: {supported_values}",);
let message = format!(
"Invalid relation mode setting: \"{relation_mode}\". Supported values: {supported_values}",
);
let error = DatamodelError::new_source_validation_error(&message, "relationMode", rm.span());
diagnostics.push_error(error);
}
};

// figure out which attribute is used for the "relationMode" feature
match (args.remove("relationMode"), args.remove("referentialIntegrity")) {
(None, None) => None,
(Some(_), Some((span, _))) => {
// both possible attributes are used, which is invalid
diagnostics.push_error(DatamodelError::new_referential_integrity_and_relation_mode_cooccur_error(span));
None
}
(None, Some((span, rm))) => {
// "referentialIntegrity" is used, which is valid but deprecated
diagnostics.push_warning(DatamodelWarning::new_referential_integrity_attr_deprecation_warning(
span,
));

let relation_mode_option = get_and_validate_relation_mode_value(rm, diagnostics);
relation_mode_option.iter().for_each(|relation_mode| {
validate_allowed_relation_mode_settings(rm, *relation_mode, diagnostics);
});

relation_mode_option
}
(Some((_span, rm)), None) => {
// "relationMode" is used, which is valid
let relation_mode_option = get_and_validate_relation_mode_value(rm, diagnostics);
relation_mode_option.iter().for_each(|relation_mode| {
validate_allowed_relation_mode_settings(rm, *relation_mode, diagnostics);
});

Some(integrity)
relation_mode_option
}
}
}
Expand Down
29 changes: 0 additions & 29 deletions psl/psl/tests/config/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,35 +641,6 @@ fn relation_mode_default() {
assert_eq!(config.relation_mode(), Some(RelationMode::ForeignKeys));
}

#[test]
fn relation_mode_and_referential_integrity_cannot_cooccur() {
let schema = indoc! {r#"
datasource ps {
provider = "sqlite"
url = "sqlite"
relationMode = "prisma"
referentialIntegrity = "foreignKeys"
}
generator client {
provider = "prisma-client-js"
}
"#};
Comment on lines -644 to -656
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to relation_mode_and_referential_integrity_cannot_cooccur.prisma in the validation_tests suite


let config = parse_config(schema);
let error = config.unwrap_err();

let expectation = expect![[r#"
error: The `referentialIntegrity` and `relationMode` attributes cannot be used together. Please use only `relationMode` instead.
--> schema.prisma:5
 | 
 4 |  relationMode = "prisma"
 5 |  referentialIntegrity = "foreignKeys"
 6 | }
 | 
"#]];
expectation.assert_eq(&error);
}

fn load_env_var(key: &str) -> Option<String> {
std::env::var(key).ok()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
datasource db {
provider = "sqlite"
url = "sqlite"
referentialIntegrity = "foreignKeys"
}
// warning: The `referentialIntegrity` attribute is deprecated. Please use `relationMode` instead. Learn more at https://pris.ly/d/relation-mode
// --> schema.prisma:4
//  | 
//  3 |  url = "sqlite"
//  4 |  referentialIntegrity = "foreignKeys"
//  5 | }
//  | 
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
datasource db {
provider = "sqlite"
url = "sqlite"
relationMode = "prisma"
referentialIntegrity = "foreignKeys"
}
// error: The `referentialIntegrity` and `relationMode` attributes cannot be used together. Please use only `relationMode` instead.
// --> schema.prisma:5
//  | 
//  4 |  relationMode = "prisma"
//  5 |  referentialIntegrity = "foreignKeys"
//  6 | }
//  |