Skip to content

Make foreign key mapping null safe #451

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

Merged
merged 5 commits into from
Dec 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -587,12 +587,14 @@ The implementation and usage of the mentioned `DateTime` to `int` converter is d
class DateTimeConverter extends TypeConverter<DateTime, int> {
@override
DateTime decode(int databaseValue) {
return DateTime.fromMillisecondsSinceEpoch(databaseValue);
return databaseValue == null
? null
: DateTime.fromMillisecondsSinceEpoch(databaseValue);
}

@override
int encode(DateTime value) {
return value.millisecondsSinceEpoch;
return value == null ? null : value.millisecondsSinceEpoch;
}
}
```
Expand Down
6 changes: 4 additions & 2 deletions floor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -584,12 +584,14 @@ The implementation and usage of the mentioned `DateTime` to `int` converter is d
class DateTimeConverter extends TypeConverter<DateTime, int> {
@override
DateTime decode(int databaseValue) {
return DateTime.fromMillisecondsSinceEpoch(databaseValue);
return databaseValue == null
? null
: DateTime.fromMillisecondsSinceEpoch(databaseValue);
}

@override
int encode(DateTime value) {
return value.millisecondsSinceEpoch;
return value == null ? null : value.millisecondsSinceEpoch;
}
}
```
Expand Down
16 changes: 8 additions & 8 deletions floor_annotation/lib/src/foreign_key.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ class ForeignKey {
/// Action to take when the parent [Entity] is updated from the database.
///
/// By default, [ForeignKeyAction.noAction] is used.
final int onUpdate;
final ForeignKeyAction onUpdate;

/// [ForeignKeyAction]
/// Action to take when the parent [Entity] is deleted from the database.
///
/// By default, [ForeignKeyAction.noAction] is used.
final int onDelete;
final ForeignKeyAction onDelete;

/// Declares a foreign key on another [Entity].
const ForeignKey({
Expand All @@ -33,13 +33,13 @@ class ForeignKey {

/// Constants definition for values that can be used in
/// [ForeignKey.onDelete] and [ForeignKey.onUpdate]
abstract class ForeignKeyAction {
enum ForeignKeyAction {
/// Possible value for [ForeignKey.onDelete] or [ForeignKey.onUpdate].
///
/// When a parent key is modified or deleted from the database, no special
/// action is taken. This means that SQLite will not make any effort to fix
/// the constraint failure, instead, reject the change.
static const noAction = 1;
noAction,

/// Possible value for [ForeignKey.onDelete] or [ForeignKey.onUpdate].
///
Expand All @@ -55,22 +55,22 @@ abstract class ForeignKeyAction {
/// Even if the foreign key constraint it is attached to is deferred(),
/// configuring a RESTRICT action causes SQLite to return an error immediately
/// if a parent key with dependent child keys is deleted or modified.
static const restrict = 2;
restrict,

/// Possible value for [ForeignKey.onDelete] or [ForeignKey.onUpdate].
///
/// If the configured action is 'SET NULL', then when a parent key is deleted
/// (for [ForeignKey.onDelete]) or modified (for [ForeignKey.onUpdate]), the
/// child key columns of all rows in the child table that mapped to the parent
/// key are set to contain NULL values.
static const setNull = 3;
setNull,

/// Possible value for [ForeignKey.onDelete] or [ForeignKey.onUpdate].
///
/// The 'SET DEFAULT' actions are similar to SET_NULL, except that each of the
/// child key columns is set to contain the columns default value instead of
/// NULL.
static const setDefault = 4;
setDefault,

/// Possible value for [ForeignKey.onDelete] or [ForeignKey.onUpdate].
///
Expand All @@ -80,5 +80,5 @@ abstract class ForeignKeyAction {
/// deleted parent row is also deleted. For an [ForeignKey.onUpdate] action,
/// it means that the values stored in each dependent child key are modified
/// to match the new parent key values.
static const cascade = 5;
cascade,
}
25 changes: 22 additions & 3 deletions floor_generator/lib/misc/extension/dart_object_extension.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,35 @@
// ignore_for_file: import_of_legacy_library_into_null_safe
import 'package:analyzer/dart/constant/value.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:collection/collection.dart';
import 'package:floor_annotation/floor_annotation.dart';

extension DartObjectExtension on DartObject {
String toEnumValueString() {
/// get the String representation of the enum value,
/// or `null` if the enum was not valid
String? toEnumValueString() {
final interfaceType = type as InterfaceType;
final enumName = interfaceType.getDisplayString(withNullability: false);
final enumValue = interfaceType.element.fields
.where((element) => element.isEnumConstant)
.map((fieldElement) => fieldElement.name)
.singleWhere((valueName) => getField(valueName) != null);
.singleWhereOrNull((valueName) => getField(valueName) != null);
if (enumValue == null) {
return null;
} else {
return '$enumName.$enumValue';
}
}

return '$enumName.$enumValue';
/// get the ForeignKeyAction this enum represents,
/// or the result of `null` if the enum did not contain a valid value
ForeignKeyAction? toForeignKeyAction() {
final enumValueString = toEnumValueString();
if (enumValueString == null) {
return null;
} else {
return ForeignKeyAction.values.singleWhereOrNull(
(foreignKeyAction) => foreignKeyAction.toString() == enumValueString);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import 'package:floor_annotation/floor_annotation.dart';

extension ForeignKeyActionExtension on ForeignKeyAction {
String toSql() {
switch (this) {
case ForeignKeyAction.noAction:
return 'NO ACTION';
case ForeignKeyAction.restrict:
return 'RESTRICT';
case ForeignKeyAction.setNull:
return 'SET NULL';
case ForeignKeyAction.setDefault:
return 'SET DEFAULT';
case ForeignKeyAction.cascade:
return 'CASCADE';
}
}
}
25 changes: 0 additions & 25 deletions floor_generator/lib/misc/foreign_key_action.dart

This file was deleted.

32 changes: 25 additions & 7 deletions floor_generator/lib/processor/entity_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import 'package:analyzer/dart/element/element.dart';
import 'package:collection/collection.dart';
import 'package:floor_annotation/floor_annotation.dart' as annotations;
import 'package:floor_generator/misc/constants.dart';
import 'package:floor_generator/misc/extension/dart_object_extension.dart';
import 'package:floor_generator/misc/extension/string_extension.dart';
import 'package:floor_generator/misc/extension/type_converters_extension.dart';
import 'package:floor_generator/misc/foreign_key_action.dart';
import 'package:floor_generator/misc/type_utils.dart';
import 'package:floor_generator/processor/error/entity_processor_error.dart';
import 'package:floor_generator/processor/queryable_processor.dart';
Expand Down Expand Up @@ -91,13 +91,11 @@ class EntityProcessor extends QueryableProcessor<Entity> {
throw _processorError.missingParentColumns;
}

final onUpdateAnnotationValue =
foreignKeyObject.getField(ForeignKeyField.onUpdate)?.toIntValue();
final onUpdate = ForeignKeyAction.getString(onUpdateAnnotationValue);
final onUpdate =
_getForeignKeyAction(foreignKeyObject, ForeignKeyField.onUpdate);

final onDeleteAnnotationValue =
foreignKeyObject.getField(ForeignKeyField.onDelete)?.toIntValue();
final onDelete = ForeignKeyAction.getString(onDeleteAnnotationValue);
final onDelete =
_getForeignKeyAction(foreignKeyObject, ForeignKeyField.onDelete);

return ForeignKey(
parentName,
Expand Down Expand Up @@ -261,4 +259,24 @@ class EntityProcessor extends QueryableProcessor<Entity> {
return attributeValue;
}
}

annotations.ForeignKeyAction _getForeignKeyAction(
DartObject foreignKeyObject,
String triggerName,
) {
final field = foreignKeyObject.getField(triggerName);
// TODO #375 remove suppress when getField is null aware
// ignore: unnecessary_null_comparison
if (field == null) {
// field was not defined, return default value
return annotations.ForeignKeyAction.noAction;
}

final foreignKeyAction = field.toForeignKeyAction();
if (foreignKeyAction == null) {
throw _processorError.wrongForeignKeyAction(field, triggerName);
} else {
return foreignKeyAction;
}
}
}
11 changes: 11 additions & 0 deletions floor_generator/lib/processor/error/entity_processor_error.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// TODO #375 delete once dependencies have migrated
// ignore_for_file: import_of_legacy_library_into_null_safe
import 'package:analyzer/dart/constant/value.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:source_gen/source_gen.dart';

Expand Down Expand Up @@ -71,6 +72,16 @@ class EntityProcessorError {
);
}

InvalidGenerationSourceError wrongForeignKeyAction(
DartObject field, String triggerName) {
return InvalidGenerationSourceError(
'No ForeignKeyAction with the value $field exists for the $triggerName trigger.',
todo:
'Make sure to add a correct ForeignKeyAction like `ForeignKeyAction.noAction` or leave it out entirely.',
element: _classElement,
);
}

InvalidGenerationSourceError get autoIncrementInWithoutRowid {
return InvalidGenerationSourceError(
'autoGenerate is not allowed in WITHOUT ROWID tables',
Expand Down
13 changes: 11 additions & 2 deletions floor_generator/lib/processor/insertion_method_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:floor_annotation/floor_annotation.dart' as annotations
show Insert;
show Insert, OnConflictStrategy;
import 'package:floor_generator/misc/change_method_processor_helper.dart';
import 'package:floor_generator/misc/constants.dart';
import 'package:floor_generator/misc/extension/dart_object_extension.dart';
Expand Down Expand Up @@ -79,10 +79,19 @@ class InsertionMethodProcessor implements Processor<InsertionMethod> {
}

String _getOnConflictStrategy() {
return _methodElement
final onConflictStrategy = _methodElement
.getAnnotation(annotations.Insert)
.getField(AnnotationField.onConflict)
.toEnumValueString();

if (onConflictStrategy == null) {
throw InvalidGenerationSourceError(
'Value of ${AnnotationField.onConflict} must be one of ${annotations.OnConflictStrategy.values.map((e) => e.toString()).join(',')}',
element: _methodElement,
);
} else {
return onConflictStrategy;
}
}

void _assertMethodReturnsFuture(final DartType returnType) {
Expand Down
13 changes: 11 additions & 2 deletions floor_generator/lib/processor/update_method_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:floor_annotation/floor_annotation.dart' as annotations
show Update;
show Update, OnConflictStrategy;
import 'package:floor_generator/misc/change_method_processor_helper.dart';
import 'package:floor_generator/misc/constants.dart';
import 'package:floor_generator/misc/extension/dart_object_extension.dart';
Expand Down Expand Up @@ -63,10 +63,19 @@ class UpdateMethodProcessor implements Processor<UpdateMethod> {
}

String _getOnConflictStrategy() {
return _methodElement
final onConflictStrategy = _methodElement
.getAnnotation(annotations.Update)
.getField(AnnotationField.onConflict)
.toEnumValueString();

if (onConflictStrategy == null) {
throw InvalidGenerationSourceError(
'Value of ${AnnotationField.onConflict} must be one of ${annotations.OnConflictStrategy.values.map((e) => e.toString()).join(',')}',
element: _methodElement,
);
} else {
return onConflictStrategy;
}
}

DartType _getFlattenedReturnType(final DartType returnType) {
Expand Down
14 changes: 13 additions & 1 deletion floor_generator/lib/processor/view_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,21 @@ class ViewProcessor extends QueryableProcessor<View> {
.getField(AnnotationField.viewQuery)
?.toStringValue();

if (query == null || !query.trimLeft().toLowerCase().startsWith('select')) {
if (query == null || !(query.isSelectQuery || query.isCteWithSelect)) {
throw _processorError.missingQuery;
}
return query;
}
}

extension on String {
bool get isSelectQuery => toLowerCase().trimLeft().startsWith('select');

/// whether the string is a common table expression
/// followed by a `SELECT` query
bool get isCteWithSelect {
final lowerCasedString = toLowerCase();
return lowerCasedString.trimLeft().startsWith('with') &&
'select'.allMatches(lowerCasedString).length >= 2;
}
}
10 changes: 6 additions & 4 deletions floor_generator/lib/value_object/foreign_key.dart
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
// TODO #375 delete once dependencies have migrated
// ignore_for_file: import_of_legacy_library_into_null_safe
import 'package:collection/collection.dart';
import 'package:floor_annotation/floor_annotation.dart' show ForeignKeyAction;
import 'package:floor_generator/misc/extension/foreign_key_action_extension.dart';

class ForeignKey {
final String parentName;
final List<String> parentColumns;
final List<String> childColumns;
final String onUpdate;
final String onDelete;
final ForeignKeyAction onUpdate;
final ForeignKeyAction onDelete;

ForeignKey(
this.parentName,
Expand All @@ -25,8 +27,8 @@ class ForeignKey {

return 'FOREIGN KEY ($escapedChildColumns)'
' REFERENCES `$parentName` ($escapedParentColumns)'
' ON UPDATE $onUpdate'
' ON DELETE $onDelete';
' ON UPDATE ${onUpdate.toSql()}'
' ON DELETE ${onDelete.toSql()}';
}

final _listEquality = const ListEquality<String>();
Expand Down
Loading