hawkbee1 / where-assistant-public

public repository to deal with Where Assistant bugs and new features (code is private and hosted in another repository)
1 stars 0 forks source link

Thing list (assistant page) is blowing up the app when there's a lot of things #26

Closed hawkbee1 closed 4 years ago

hawkbee1 commented 4 years ago

App try to load all the images which cause too much memory consumption.

  1. impl
  1. data sources
    • [x] should retrieve image from image folder in application document directory when it exists
    • [x] should have error "image not on device" when image does not exist
    • [x] Should retrieve image stored in firebase storage and store it in image folder
    • [x] Should have error "download/writing of remote image failed"
    • [x] should retrieve image from image folder in application document directory when it exists and path is remote
  2. representation
    • [x] connect use case to image representation in assistant list
    • [ ] load only visible image
    • [ ] decrease memory size used by image displayed in the list
hawkbee1 commented 4 years ago

app usecase : display filtered list of things in assistant/search screen list.view builder ? taking userSearchResultList as parameter which is a thing list (stuff in code until now)

hawkbee1 commented 4 years ago

domain use case : get images of thing (thing) => for each path getImage(path) domain unit use case : get image (path) repository domain ? distant image or local image if local is here we get this one if local is not here 2 cases :

hawkbee1 commented 4 years ago

first : write first test for get image => should get image file for the path from the domain repository image_repository

hawkbee1 commented 4 years ago

data layer : we don't need models and mappers since we just deal with files But we need to take care of choosing between local and Firebase data source

hawkbee1 commented 4 years ago
we check path to know if it's a local path or a remote one

if local path we check if file is present, if file is present we return it

else we check connection
if device connected to internet we download file and store it locally
after storing file locally we change path of thing in thing list json file

we return file
hawkbee1 commented 4 years ago

I already know I will lack some tests at end of this dev process :-1:

hawkbee1 commented 4 years ago

no more app explosion but... I think something's fishy here Screenshot_20191015_115322_io whereassistant whereassistant

hawkbee1 commented 4 years ago

adding test : local image repository : should extract filename from remote path

hawkbee1 commented 4 years ago

switch on dev branch and ran debug mode : same pbm Screenshot_20191017_164822_io whereassistant whereassistant

hawkbee1 commented 4 years ago

method = "StorageReference#writeToFile" this = {MethodChannel} arguments = {_InternalLinkedHashMap} size = 4 result = {_ByteDataView} TypedDataView(cid: 143)

hawkbee1 commented 4 years ago

=> bucket and app value are null => some files should already be on local drive so the path has change during new version dev =>

hawkbee1 commented 4 years ago

checking old local storage parameters : old version stored image at root of application document directory and new one is storing in image folder. checking activity to see if migration needed... I have regular users so I won't change folder without handling migration.

changing or not ?

verdict => keep images in root folder

changing image folder of tests to point directly on tdd/fixtures/image in image_local_data_source_test.dart updating code to have passing test changing image folder of tests to point directly on tdd/fixtures/image in test/tdd/data/datasources/image_remote_data_source_test.dart => good without change... tdd hurra !

hawkbee1 commented 4 years ago

new bug : app is trying to download file with local storage path path

hawkbee1 commented 4 years ago

error is presently not due to memory explosion as expected but 404 error inside firebase storage moduel when using writeToFile.... try catch doesn't work on this one... :-/ time to attempt a new package update in new branch Capture d’écran de 2019-10-17 18-44-04

hawkbee1 commented 4 years ago

can't catch firebase storage error, issue is dealt in firebase repository : https://github.com/FirebaseExtended/flutterfire/issues/792

hawkbee1 commented 4 years ago

follow up in https://github.com/hawkbee1/where-assistant-public/issues/29 https://github.com/hawkbee1/where-assistant-public/issues/28