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

chore(deps): bump firebase_storage from 11.2.4 to 11.6.0 #245

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jan 5, 2024

This serves as a simple explanation of what should be done versus what we should avoid doing to follow best practices. To also follow the humble object approach.

DO ✅:

Domain Layer

class UserModel {
  final String id;
  final int age;
  final String name;

  factory UserModel.fromJson(final Map<String, Object?> json) =>
      UserModelFromJson(json);

  factory UserModel.toJson(final Map<String, Object?> json) =>
      _$UserModelFromJson(json);
  UserModel(this.id, this.age, this.name);
}

Data Layer

class APIPath {
  APIPath();
  static String company(final String uid, final String companyId) =>
      'users/$uid/company/$companyId';
  static String user(final String uid, final String companyId) => 'users/$uid';
}
abstract class ApiInterface {
  UserModel? get currentUser;
  Future<void> setUser(final UserModel model);
  Future<void> updateUser(final UserModel model);
  Future<void> deleteUser(final UserModel model);
  Stream<List<UserModel>> userStream();
}

Description:

Here the repository level implementation does not know anything about the domain layer. We have single line baring as little logic as possible. Each of the implementation is a single line of code. It remains testable, readable and scalable, making the code less bug prone.

class ApiImpl implements ApiInterface {
  factory ApiImpl({required final String uid}) {
    return _singleton ??= ApiImpl._internal(uid);
  }

  ApiImpl._internal(this.uid);
  static ApiImpl? _singleton;

  final _service = locator<AuthService>();

  @override
  UserModel? get currentUser => _user;
  
  @override
  Future<void> setUser(final UserModel model) => _service.setData(
        path: APIPath.user(uid, model.id),
        data: model.toJson(),
      );

  @override
  Future<void> updateUser(final UserModel model) => _service.updateData(
        path: APIPath.user(uid, model.id),
        data: model.toJson(),
      );

  @override
  Future<void> deleteUser(final UserModel model) async {
    await _service.deleteData(
      path: APIPath.user(uid, model.id),
      image: model.image,
    );
  }

  @override
  Stream<UserModel> userStream({required final String userId}) =>
      _service.documentStream(
        path: APIPath.user(uid, userId),
        builder: (final data, final documentId) => UserModel.fromJson(data),
      );

  @override
  Stream<List<UserModel>> userStream() => _service.collectionStream(
        path: APIPath.user(uid),
        builder: UserModel.fromJson,
      );
}

Service:

This file should not bear specific functions but rather. The idea of having generic method is that we do not have to test these but rather only test the implementation level.If there are break in data or issue, this layer will be out of cause.

typedef QueryBuilder<T> = T Function(Map<String, dynamic> data);

class ApiService {
  ApiService._();

  static final instance = ApiService._();

  Future<T> setData({
    required final String path,
    required final Map<String, dynamic> data,
  }) async {
    final reference = Http.instance.doc(path);
    await reference.set(data);
  }

  Future<T> updateData({
    required final String path,
    required final Map<String, dynamic> data,
  }) async {
    final reference = Http.instance.doc(path);
    await reference.update(data);
  }

  Future<T> deleteData(
      {required final String path, final String? image}) async {
    final reference = ApiService.instance.doc(path);
    if (image != null) {
      final storageReference = Http.instance.refFromURL(image);
      await storageReference.delete();
      await reference.delete();
    }
    await reference.delete();
  }

  Stream<List<T>> collectionStream<T>({
    required final String path,
    required final QueryBuilder<T> builder,
    final Function(Query query)? queryBuilder,
    final int Function(T lhs, T rhs)? sort,
  }) {
    var query = ApiService.instance.collection(path);

    if (queryBuilder != null) {
      query = queryBuilder(query) as CollectionReference<Map<String, dynamic>>;
    }
    final snapshots = query.snapshots();
    return snapshots.map((final snapshot) {
      final result = snapshot.docs
          .map((final snapshot) => builder(snapshot.data()))
          .where((final value) => value != null)
          .toList();
      if (sort != null) {
        result.sort(sort);
      }

      return result;
    });
  }

  Stream<T> documentStream<T>({
    required final String path,
    required final T Function(Map<String, dynamic> data, String documentID)
        builder,
  }) {
    final reference = Http.instance.doc(path);
    final snapshots = reference.snapshots();
    return snapshots
        .map((final snapshot) => builder(snapshot.data()!, snapshot.id));
  }
}

DO NOT DO: ❌

Domain layer

class UserModel {
  final String id;
  final int age;
  final String name;

  //❌ 1- Domain logic mixed with data parsing logic
  factory UserModel.fromJson(final Map<String, Object?> json) {
    // Domain logic (business rules)
    if (json['age'] == null || json['name'] == null) {
      throw Exception("Invalid user data");
    }
    
    // ❌2- Data parsing logic mixed with domain logic
    return UserModel(
      json['id'] as String,
      json['age'] as int,
      json['name'] as String,
    );
  }

  UserModel(this.id, this.age, this.name);
}

Data Layer

In the following case a common mistake is to duplicate code, mix layers and have unhandled edge case.

class ApiService {
  ApiService._();

  static final instance = ApiService._();

// ❌ - 1 Seems great but we confuse layers and throw exception
  Future<UserModel> getUser({required final String userId}) async {
    final response = await http.get('https://api.example.com/user/$userId');

    if (response.statusCode == 200) {
      // Data parsing logic mixed with domain logic
      final userData = json.decode(response.body);
      return UserModel.fromJson(userData); // <----Dependency on UserModel domain class
    } else {
      throw Exception('Failed to load user');
    }
  }


  // ❌ - 2 We mix layers and repeat the same code with also handling logic.  
  Future<UserModel?> deleteUser({required final String userId}) async {
    final response = await http.get('https://api.example.com/user/$userId');

    if (response.statusCode == 200) {
      // Data parsing logic mixed with domain logic
      final userData = json.decode(response.body);
      return UserModel.fromJson(userData); // <----Dependency on UserModel domain class
    } else {
      throw Exception('Failed to load user');
    }
  }
}


  // ❌ - 2 Using notify listener in wrong layer and method holds to much business logic. In 
  //addition the value returned could be null, this will cause us to cover this edge case. By 
  //trying to cover it we might cause more issues and testing that will be a nightmanre.
  Future<UserModel?> updateUser({required final String userId}) async {
    final response = await http.get('https://api.example.com/user/$userId');

    if (response.statusCode == 200) {
      // Data parsing logic mixed with domain logic
      final userData = json.decode(response.body);
      return UserModel.fromJson(userData); //
      notifyListeners(); <----Dependency on UserModel domain class
    } else if(await authService.token != null) { //<- referencing a value of different layer
         //... do some garbage here.
      throw Exception('Failed to load user');
    
    }
  }

JordyHers and others added 2 commits December 11, 2023 23:38
Bumps [firebase_storage](https://github.com/firebase/flutterfire/tree/master/packages/firebase_storage) from 11.2.4 to 11.6.0.
- [Release notes](https://github.com/firebase/flutterfire/releases)
- [Changelog](https://github.com/firebase/flutterfire/blob/master/CHANGELOG.md)
- [Commits](https://github.com/firebase/flutterfire/commits/firebase_storage-v11.6.0/packages/firebase_storage)

---
updated-dependencies:
- dependency-name: firebase_storage
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot added dart Pull requests that update Dart code dependencies Pull requests that update a dependency file labels Jan 5, 2024
@JordyHers JordyHers changed the base branch from master to dev January 25, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dart Pull requests that update Dart code dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant