From c33888189021f385313f4f53c07b88e504e7f6be Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 26 Oct 2022 17:28:44 +0200 Subject: [PATCH 01/16] Update RemoteContentScanner to move local file deletion outside and add some test case for this class --- .../contentScanner/RemoteContentScanner.java | 7 +- .../e/drive/models/SyncRequest.java | 2 +- .../RemoteContentScannerTest.java | 104 ++++++++++++++++++ 3 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index 047bd8ff..31e13b85 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -7,6 +7,7 @@ */ package foundation.e.drive.contentScanner; +import static foundation.e.drive.models.SyncRequest.Type.LOCAL_DELETE; import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; import android.accounts.Account; @@ -20,6 +21,7 @@ import java.util.List; import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.DownloadRequest; +import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.utils.CommonUtils; @@ -87,6 +89,9 @@ public class RemoteContentScanner extends AbstractContentScanner { @Override protected void onMissingRemoteFile(SyncedFileState fileState) { + this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, LOCAL_DELETE)); + + /* todo: move commented code into synchronizationService if (!CommonUtils.isThisSyncAllowed(account, fileState.isMediaType())) { Timber.d("Sync of current file: %s isn't allowed", fileState.getName()); return; @@ -112,7 +117,7 @@ public class RemoteContentScanner extends AbstractContentScanner { if (DbHelper.manageSyncedFileStateDB(fileState, "DELETE", context) <= 0) { Timber.e("Failed to remove %s from DB", file.getName()); - } + }*/ } @Override diff --git a/app/src/main/java/foundation/e/drive/models/SyncRequest.java b/app/src/main/java/foundation/e/drive/models/SyncRequest.java index d2b820c8..29bab6b9 100644 --- a/app/src/main/java/foundation/e/drive/models/SyncRequest.java +++ b/app/src/main/java/foundation/e/drive/models/SyncRequest.java @@ -10,7 +10,7 @@ package foundation.e.drive.models; import androidx.annotation.Nullable; public class SyncRequest { - public enum Type { UPLOAD, DOWNLOAD, REMOTE_DELETE}; + public enum Type { UPLOAD, DOWNLOAD, REMOTE_DELETE, LOCAL_DELETE}; private final SyncedFileState syncedFileState; diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java new file mode 100644 index 00000000..72799565 --- /dev/null +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java @@ -0,0 +1,104 @@ +package foundation.e.drive.contentScanner; + +import static org.junit.Assert.assertEquals; + +import android.accounts.Account; +import android.accounts.AccountManager; +import android.content.Context; +import android.os.Build; + +import androidx.test.core.app.ApplicationProvider; + +import com.owncloud.android.lib.resources.files.model.RemoteFile; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowLog; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + +import foundation.e.drive.TestUtils; +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncRequest; +import foundation.e.drive.models.SyncedFileState; +import foundation.e.drive.models.SyncedFolder; + +@RunWith(RobolectricTestRunner.class) +@Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) +public class RemoteContentScannerTest { + + private final Context context; + private final Account account; + private RemoteContentScanner scanner_under_test; + + public RemoteContentScannerTest(){ + context = ApplicationProvider.getApplicationContext(); + ShadowLog.stream = System.out; + TestUtils.loadServerCredentials(); + TestUtils.prepareValidAccount(AccountManager.get(context)); + account = TestUtils.getValidAccount(); + + } + + @Before + public void setUp() { + prepareDB(); //Create DB + List directoryList = new ArrayList<>(); //Improve to have realistic Content + scanner_under_test = new RemoteContentScanner(context, account, directoryList); + } + + private void prepareDB() { + DbHelper.insertSyncedFolder(createSyncedFolder("small"), context); + DbHelper.insertSyncedFolder(createSyncedFolder("medium"), context); + DbHelper.insertSyncedFolder(createSyncedFolder("large"), context); + + //assertion for debug purpose + assertEquals("There isn't three folder in DB as expected", 3, DbHelper.getSyncedFolderList(context, true).size()); + } + + /** + * Create the syncedFolder instance + * @param name the name of the folder + * @return SyncedFolder instance + */ + private SyncedFolder createSyncedFolder(String name) { + return new SyncedFolder(name, + TestUtils.TEST_LOCAL_ROOT_FOLDER_PATH+name+"/", + TestUtils.TEST_REMOTE_ROOT_FOLDER_PATH+name+"/", + true, true, true, true); + } + + + @Test + public void scanEmptyCloudContent_generateLocalDeletionRequest() { + final List cloudContent = new ArrayList<>(); + Assert.assertTrue("Fake list of cloud Content is not empty", cloudContent.isEmpty()); + + final List dbContent = new ArrayList<>(); + dbContent.add(new SyncedFileState(5, "toto", "local/path/toto", "remote/path/toto", "5555", 22222, 2, true, 3)); + dbContent.add(new SyncedFileState(3, "titi", "local/path/titi", "remote/path/titi", "5556", 22322, 2, true, 3)); + dbContent.add(new SyncedFileState(2, "tata", "local/path/tata", "remote/path/tata", "5557", 22232, 2, true, 3)); + + Assert.assertFalse("List of SyncedFileState is empty", dbContent.isEmpty()); + + final HashMap scanResult = scanner_under_test.scanContent(cloudContent, dbContent); + + //If the list of SyncedFileState is not empty then all the local file must be deleted... + //In fact, the list of SyncedFileStates that is given as parameter to scanContent, should only contains content + //that has been identified has potentially changed, new or removed + //However, the issue is that...the scanner remove the file by itself, which should be out of its scope + //TODO: find how to assert the behaviour in current state ? + + Assert.assertEquals("SyncRequest doesn't match the expected result", 3, scanResult.size()); + + for (SyncRequest request : scanResult.values()) { + Assert.assertEquals("SyncRequest's type should be LOCAL_DELETE but is "+request.getOperationType(), SyncRequest.Type.LOCAL_DELETE, request.getOperationType()); + } + } +} \ No newline at end of file -- GitLab From 9247f93d33de008fb8f16b12a1402f865d4be68c Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 27 Oct 2022 11:46:49 +0200 Subject: [PATCH 02/16] Add a database file to use for test and change set up method to use it --- .../RemoteContentScannerTest.java | 25 +++++++++++++----- .../databases/database_71SyncedFile.db | Bin 0 -> 69632 bytes 2 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 app/src/test/resources/databases/database_71SyncedFile.db diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java index 72799565..97e19495 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java @@ -9,6 +9,7 @@ import android.os.Build; import androidx.test.core.app.ApplicationProvider; +import com.google.common.io.Files; import com.owncloud.android.lib.resources.files.model.RemoteFile; import org.junit.Assert; @@ -19,6 +20,8 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLog; +import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -53,13 +56,23 @@ public class RemoteContentScannerTest { scanner_under_test = new RemoteContentScanner(context, account, directoryList); } - private void prepareDB() { - DbHelper.insertSyncedFolder(createSyncedFolder("small"), context); - DbHelper.insertSyncedFolder(createSyncedFolder("medium"), context); - DbHelper.insertSyncedFolder(createSyncedFolder("large"), context); + private void prepareDB(){ - //assertion for debug purpose - assertEquals("There isn't three folder in DB as expected", 3, DbHelper.getSyncedFolderList(context, true).size()); + final File usedDB = context.getDatabasePath(DbHelper.DATABASE_NAME); + System.out.println(usedDB.getAbsolutePath()); + + final File dbToCopy = new File(getClass().getResource("/databases/database_71SyncedFile.db").getFile()); + Assert.assertTrue("DB files not found", dbToCopy.exists()); + + try { + Files.copy(dbToCopy, usedDB); + } catch (IOException e) { + e.printStackTrace(); + Assert.fail("Impossible to copy db from resources"); + } + + System.out.println(dbToCopy); + assertEquals("There isn't 10 folder in DB as expected", 10, DbHelper.getSyncedFolderList(context, true).size()); } /** diff --git a/app/src/test/resources/databases/database_71SyncedFile.db b/app/src/test/resources/databases/database_71SyncedFile.db new file mode 100644 index 0000000000000000000000000000000000000000..fd1a23b650a82d14e6a103169cd3da1ae95d0cd2 GIT binary patch literal 69632 zcmWFz^vNtqRY=P(%1ta$FlG>7U}R))P*7lCV0z8Kz#zzg0L%;w3@lJC2Ll5GBLf42 z2$YY5VtUP>XQIH%Kc9h(C!K+RIUg5)3cn^#I`45D8b*ajLtr!nMnhmU1V%$(Gz3ON zU^E0qLtr!nhJFY%#j>)C>+3VNWR@f*<)ju@<|U`5#HVHEq{bJQB$i+busa92I)=C^ zgg83+xGG?%SI|(1&rDJ9^b2uycMVbq2=erG42o3nc8%0g0GpbZn478);u;a6qmYxI zoR|||kXVud6(~y0%`Zts7RgC0E{RVqNlb^TfM@}W=H{nlre&tW?b3leCO;=7wFt=- zItrP^@wurfnThcwl?AB^PX7Kru8w{>#mR|zc_3fIoU5bY?C%#G66EOV7XmY_G%qtb zv!pUUIX|zsq$n{nuS6l#&oeO8RY3#pA7roSC?NSlQ`3=+O*~Yeks&iLCAFfsFekGl zH9oPlBp=L=#|XZ7LkuB~CK)DnadmaZ=5mxM0{arC8q8tA8Z}@ke9@AeSdyBaUzC{| zpOcuBnu8M3@W@dp&C4t-O-1n?SS4Hn94W|A1&TT>k(8X67Y~j`cswY8A|f6VYzVF<_He85kHqU2TSNMmBL>UB+Z^ z)FkGm6y;~8#OJ1#B&H;mB%G3NcMIZW zFK6IkyU)OJmEDl-KF$^V7}$?OAg={7a3icoSZ?KdFdtjd8x(v#U=SgiRr2Oskx;&i6yBi`Ud(4NhGG3 zv9XDXnW3?vg{4^vW8>L5r3YM?xEaJ9r6DHzmKJ9wV>1mRp%0g`(8t{Gya%eW(jj1tIRWFf=u^ zG&AB7^IpTnItAMHedCcUg6Ei%^>NnNVFvggV7AT*q@=m6U%guiUBZc)My9{q7aA{ z;AW5vH71l*T%0|9_5BM{^PCfNQ;QPy^#d~UOY)16h0Kgi63t9a5)+dxQjHm#Z%qjk z;ALk>rmI4!%Z5ybYP5+cVx2+Ge@ zC{8UY$;?YH)(5Q|*DtOtE=kSRFD*_jD%Lm9*LO)R%S=u!)^|(HEGo#z&r6MW3owZ< z&CAZqFVE92%Fm65>t}3!HSzj{G7fGA)zETW=7TLs164VRC7JnodZ~ITMVV!(`k*}x z#UxpmSWpn3lUZD%&)EEG?net>c5Vh)SP>tPpOTzdT!OV;hs){1SOLJ56u$zaF)JHHOHKu8*xlxjNDr57-ZpQ`gtlSKe&d`MEn_rfRy`qE4 z=tF6!(+myMlG2PJPGewTh@)NF;bLH5;Nc5q;BVyH#&62MitiJ@C|?=>WxlEW!F(6^ zg87vB`1$zx@1nrDv~$m>$)h1K8UmvsFd71*Aut*OqaiRF0;3@?8Ulkg1o)*Gym7mY zcP_(e+_FPThW9jsJRS%088H;&k*0+dUnGMuiRSWiGMFQ@@O3h1B6xUM{QV3T*re^S z2{JG+C^367VjB}jjIcAyF*{;a0vV!aQebxH#AX6~{8y6MoDu5)2B#vkJ64n7oqu** z&O`L(+2om>u^IyHq_fI0+hdgnkCL(IGRI?;g}WYkOq)xCITY#@{9`2?V1-zQ^}&`R z4b2JYFvnuG9}>8zL;Vtv{{K@3{;B-O_%-=0`JVD8^Skla@_pp{&o9Lv%(stk8{am* zx5$wH8jhGB6&?+N(GVC7fzc2c4S~@R7!85Z5Eu=C(GVC7fzc2kCj|KCGH4+5@l`NP zLhuFyi*GUm56m5W#~5s3Y?5jI00t%en)tIBB(SOAcVQ64CN766$jvOx2wmyM#LKM6 ziCFI?#caul;GuNP`Ir?M5o-q6`I(g&k!Jkac$j4wVYBwEU=DOi3=3E#(qIR8umWr6 zTb$XL5w<7=xd(5+oXSYbQVdQ@=5mrWz!y_+u=0OnVBvob3NK#%-~1o>-|#<&a6v+& zcr*k?Ltr!nMnhmU1V%$(Gz3ONU^E0qLtr!nMnhmU1V%$(XoY}a8Uq6Ze*gmm1D68> z0|V$Re+EuNCcY^Q%zVNiuL$rj=8u6eLEKS18UmvsFd71*Aut*OqaiRF0;3@?8Umvs zFd71*AuzZ?fN?YN?%^+H*odi`znEb*swiI#n_(k!EMrn)P9g;7CMD{ngnwLqLAJ7dU?0z&I7N!O%hQ^6Wh6brdiDsq- zrWPsY#ztnA<|#=jX{P4MaJL)lb2F$n3g?2|ZmtlVm|I$um`Q%+Qj^nC%~H)RObrdqQw_|FOjAwZuGWy}W&rhXz^=y9QNgy$7JeoeRr)x_ zDXGR5=4OcohQ^6z1_oy4re?-wrj{v2iHQb@MwZ|JR#p&XVqjp{G_>+%BfA6_gKV}Z zIBW|tlP&S~SCaCR^h@$f^7TtHOEUH0%Aq|L?6T&mmTAVurpcBThK3gAX-TGO=4NRr zmImg@Mn;CFso;zyp25%5$S#iJ5_8gA0#T03C16=2;}j!{WYc70V+%8r)I`ffQ?o=9 zqvTWzOEU|@M00S0V}1Z~i5QAY%t&(yL^&>(fMt`@EG-ffQ%y|FOe_qHj11F^j7&{Z z(h|*+jV;m)EYd(pU$_C}5>XVFn3Coah;m#m0m~X28XBY-Bqk@BnHpFan;Dy@SQwa? znIhbN2q|>4i~UUNwi2w zHBU=RwJ@|uNlP|0H8(R)Ni$4KGcYkvGfoBBch-P~33UG-Kl4fkep9}1-l;qc>l%Va|{ z6EmYkb5qN-q*TjP&}pl6S3d|aH!0ZjGBmO_`Zj8V!^a@i+#uD&)YRD6$Sldo*wDb- zG}YY1Fwr70Ims~9&_Y+YJU=H*H#xDWEHf`%w>%@Wq$DRZFP+!`qR4#A5TeAmWMeaP zP)VO^m~5VCY;KZdmS~=Ao@{7gY?)+iW@5tFe6j8lhX8Y9xg8HfYNL6hAUK2&yWQfA zKy3yC0~1|CO9LY#<9Mf>%;I97#L~RvjMO4rwISB?Y)CScViU{^Ow-bm(~>MqlFZEv z%?wf#4a_Y}Qp^$!&C`-nEYmbKj@uXw}OG8UtLo-toBLnKCWc-E_=Vv_5N=dUwGc`6#G_^D_Oi4>IO)^hQG_tTr zOieaUOG-{O0>$v53@+wIZYwSZ{Y+tGjlUj6I=xs+-aDqaZ9D8 zS{N9l86_KA7?>wn8krlLn_3ztT3Q$+o0+E>CMJQLv#)`<(cY4qLBG6F(wCKygRvkn zIU97L1NfY7z2xFD$Pr!m&$YtTM8X;5@O`WprWjgUn3@=w8yF^8m{=O5S{SC78ygxL z8Cn_`8l|RKnlLuMO3pdJ!PKZ?!OdV^->AsT!N|c;l3NguVH7dGz^;)TpI|kKqfyeF zi$S$I6kL4yh6L+^N^HG?lr*ee1Nc~lV^N82ZemeMW@2fDK8iNP&Jv;cL2jT4hC%*`z=(hSm!OiaLo z0jV8gOpRKm+zg71yy;+9mS&fhBo>i5Ol4@PkE9>rN~~jpD2fv;lM+*slG75+43ZK} zEs`t^jgpK_ElkXljSb8!EzLl^bUz7Erbc@cZU+5E=~A#`lZx`QQ}YUn@^dqb$sX7= z(8n}}fYXhz=rJ%)G&VCyN-;A{u{2Ljv$Ql!H8eCbHApozG)b{E1dlTWF^DiVMjP`m zI5z4wvV*9ufkFck3YfdvTO0+OAx3n-xGBGkU zNHa3FFitWywM;X$Fb54`fa1r^Lxic(!-$)~v{A7d>^bL*{NmI+g@DxJ;>_I4;u12G zm63ryR+9*O2%EMfBSQm=L=$rhi{unDlN6&gL$j15%T&`OQ$tfzlN8W+!+k#q5hgCi z3k;%+*$n(#eB!*)jM>bEOx-*&+#5J2aopt4Wp8B7VoTx5V7<-F!t{n^4dWjcb;cWD zXv9=QVxOkboISQtwUL?8B+b&?+$=3oH`(08MAsxa)q>fY;VFrx4L#*4tm^r`jD{u# zMi%AqIYf6fP&Huo98lz)SY#P=hz*)XUN+FY10&ktKmIBOYbj@BsE=+4p=67p!ytiG zKhv3pZ5pdaW;W|a=0rxP(xTkd>O`oU}#z%Z)jj-QUM}}bto>am~O+N(yvj54OHkdVk_^+NwicgaE;gn z7!>g3yaHDJ@=!+8mF`~?okrPga`lv<_ z&f2KzVM*T1($px$EZN-L*fhn+DA~|5E!D`>G}$~k#mF?-*vydF;2_Ez%n%|*n>Uvo zLm|Fcq`Dj|a7+^o49rc;%*;~_EsV?zQ&J6+jg6C2jLi&8(h`kL%!myfqRhbz9AdP2 z!rf|VY-(<7o@kbqnqq8}l9*y{WSMM`l0>vyi7^L@TZz!-fv2^hn`CL6 zsGDkNk!)aYkYr?*NOa1>Y`Net3)9iWYITPNvWaP;fgyOd$k-$`DKR1p2jZ$o(jr^I6#AHHjWfXjQXLNqaSKzU z)YOzT6EoA4#55y=q{Osj6GIbA6VsG53k#FPG-8uAQRZNVI5FCMU~V-wF-%D`F|#l< zGEGTLF*Hm}HBU4&G)_%PGDrI{s(MXAN{sX3|1B}JKqrK!a@M+-pfqmd>PNH7i4;lyipX{_RCbR7T=sEpyXX81#$ z)HU=>!HfVS(|AK8Q$u5L;YL~nplZbz0VpbSu{Z*hs7)-4$w-PuC|dD40$F7aPDdCK z-7Q8WMW`d7T8VH3L}fOY>!57pz`R5B=ptrFK(rDQ5@3~CJduphapIwusi^B>TnW?M z1RPQ%W>IjOhiN4o9Z;2-xE(?CKo@$NFgJl{CB_k8l^F~SqiF(?wlJf^&?Mf-z|_Q$ zj1m)7E57JJQJKk9mY9NbqSk&Sh1t4rN4Htb=zy63+EINVgT>RAbJ|W5&SFrNF>1!RyDD#@)ho ziDwQM-gzuYq8=qjLtr!nMnhmU1V%$(Gz3ONU^E1VX9zUv=W;WER?UHzx*~N8=|1w5 zmX>Ccn3QT_Xl7_;WMYzRVqj)$Zfa_oY@C#oWNMVc*ur<@HfWcVel~U&gL~5!kii?Q zO@6d_W+cy|XvOTI;81C7o@!udoSK?uW@?dUXk?mZV3KN@W{_%TWNBz>oMgnHTjNrp*DhRI2Z29~LomZoNjMk!_~W(F3CDTc|Qg-K~3 z7w2dQF!M1mfKC&qaguM;&%qZibR2Lrwg7F(F;2EjH8(P}uuL|xFfuYXHnlV~ut+j8 zO11!n%X9@$OlIJ5F>DY8TX&8?+Jk8&9FtI$NvS5usb)r|X%@zo28OAY2B{|ImdVM9 zM#hOrsc8nuj4fRD2RsCr8R!&~nRvqmocPGelh7eAESKn^=SiqabAx2V)YRlO69cm} zqa+ivBy)2!%cMk03(G_^i_}D8P`I3bA;8?2nvQ2DouPrDiLQZ#siiShcGBT84!(>D zv8%FJAHjyrYGZSXsfmH9iDe4t*cK!66jO^NQ&Y1P19OWcb4vr0VOb!KWZ#Nmtk!5!eztk?8}gaTMrH$6Z(ry<;ovaK!_T;~&V4 z2VBMx6Aw7_+7fU&`iTw{Ivv9}5}b~%*M@TVkSNqNSyQp`lrFTAFF9xq+!!nvtbRvW112iHT_n_>8fa6(EnfA$g3b zU9?1$Bt)5m=`muonI{@rT9_GHq?sg{q?x9fCng)Dm>OG}fleGxNj3qWlnC}%rV9@P zcw;6wT@t?|l9-4i!8A-?60g}f$s#e;%)m0q!q7M+F)=YI(J;x-%*fcpG&L>B%mN&V zPbYvP(HSWci8=Iw7*7&q4yN~r(UxjyXq0N0VwP%@Y-((ol9FsXvk+IoPADBIyJk(yEz{D>EDVxU4U$2Xf>D}z za#Cutv1y{2xw&Z?xW>DA12pHxvDaF{_)WXEn+|>RWZ|@u~;|9*nsFA{Ya`YD=LI!XgaHAp+93$ zVu@~IQHd^Si)boQ_fVkez_bHZK}aJ%n{y*~CSzt!ys5cCg`b{@rGc5bg{iS&e6m?0 z(dRMY(~ju|JgS3X2X=uPl4;4Q7Kuhi1_lNONd}gdpdnF1Q`4lxG*dI81Dq&xFr7$@ zwxC97Ht$B^LPq#8RvG!Zxv9Fz8L4Hdr4_n4`NUt3LC6S97ZT7BSjPrBzMB#0=xuBb zZT#CdQBLDV+8>E^k0**^(=t~5`bb91Q_Ar=0?)zaNVelqW!5Oq20ERd5$mz=gWzO( zi$+N{&?St6@B&6d-cANJ!eu4A)H5?7IujP_=D<{ACS-(USTw6_wkIR}T03lM1+?%R z`#pECMa|e{CE#X*ucE`*-N4WdQ4V!Bc3E+_+2G6Nh&LOe9GBT(SuwcT;OpXuHyffH zm)T%hQMlRQ3*d-18=@SS*KW26Zdw@?>;lpo>GuaVtzKVYfn6nphepCmANESeh7_rlp!DCK?;0 z7^b9|r>0n@8CWEnF*e_t@(;AFk24wKVqQ?_qC_g4kAxFoZsbhD=N8DJZN!8ghBu&E zF+&fBN&)7^_(UEC@M3ZBFf`gWhrzsH+|a-{6?AKETAGoerG=%XxdrIBa}#qzvotd^ zOJmSFioS^+pv%4z@Lcu<^Wjk1tdN+TXq=pEn3kHHW)504lA35_oNQ^BY+__?VQORo z8rkpD-oeG(=o$|ib>`0m-}Xh!Y9efj4!&fTC<91Roobk5l4@aUWMrCToRnyoVxE?g zl5CKeYGIa~l$>G=x__@fLxH(bKMs;#z)2J-DMJncq~jrmNr~o3#wkgOCP~Hyrin>L z2C1oOX~`xAmPzK8X{MH-d(HZ}K&x8xWAV8ddMGBgh6JcRkG)QTcowP^Zw^ORX=IUX zWNDFRX=HAeWMOWSXke5ITGL@n4TujGPdW}*| zlTuO)6OGN15)+ftl8iygI?3E1$t*S1#LNOTao^|l1LWc;JT8Wwwu>zbqOT`J4`Y~C z!l?qP(jv()DJ9v!IMFgW6;#bznx~khCa0#E8z(2F8H4ZV>$xBRa&aU+7ekKbCCkN7 ztwgvOqS7?g$j~y;GBM4_(k$6D#W>L{DFt+UWolZIWtyRZA!GB!o+A+;7f0ZAG5FXr z@>~qjN{oxaD$NZoEKQ7z3@p!?+j} zGu^?(F=Dua7&juR#&i}!(%jJ8)I25G$iz6=I3?LE+1S7+&B!v**x1-2$;iYARL<7( zED&IBWDiAg3H3*wO%n}FOj1&kj4TX{lakF$&5TlwlT!>$3@ptOjV#kZ3y14DK(Qzs z!o{Fj=noDP)Inup!UatSX4s%A0NwXwo@!uWVrgcYlA3C4V37iD1REPC8>X71f?^8d zD(hfw2GD>u*jaeSnTc^DKJAze#-lpTDA~l&($vBr$<)liIL*Q!ImOJ}!o(oi(#$Z; z0<^jOBE!zS_q6>6XPdBMqqk}fDRMW#MBf+(9NA@X=%nONd~D# WhRG?(DXFQ3rba0y-~ Date: Thu, 27 Oct 2022 14:03:15 +0200 Subject: [PATCH 03/16] add new basic test case for RemoteContentScanner --- .../operations/DownloadFileOperation.java | 4 +- .../e/drive/utils/FileDiffUtils.java | 3 +- .../RemoteContentScannerTest.java | 164 ++++++++++++++++-- 3 files changed, 158 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java index 9fe4a53c..9260379f 100644 --- a/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/DownloadFileOperation.java @@ -89,7 +89,9 @@ public class DownloadFileOperation extends RemoteOperation { if (!localFile.getParentFile().exists()) { localFile.getParentFile().mkdirs(); } else if (localFile.exists()) { - localFile.delete(); + localFile.delete(); //We remove the old file to avoid overwrite issue after + //WARNING todo: however, if the call to renameTo fails after this one, we have lost the local file + } if (!tmpFile.renameTo(localFile)) { diff --git a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java index 3e9410fe..7b5c1814 100644 --- a/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java +++ b/app/src/main/java/foundation/e/drive/utils/FileDiffUtils.java @@ -90,6 +90,7 @@ public class FileDiffUtils { */ private static boolean isRemoteSizeSameAsLocalSize(RemoteFile remoteFile, File localFile) { // if local file doesn't exist its size will be 0 - return remoteFile.getLength() == localFile.length(); + // remoteFile.getSize() : getSize() is equal to getLength() except that for folder is also sum the content of the folder! + return remoteFile.getSize() == localFile.length(); } } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java index 97e19495..2c2270b0 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteContentScannerTest.java @@ -1,3 +1,10 @@ +/* + * Copyright © ECORP SAS 2022. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ package foundation.e.drive.contentScanner; import static org.junit.Assert.assertEquals; @@ -16,6 +23,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLog; @@ -32,13 +40,17 @@ import foundation.e.drive.models.SyncRequest; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; + +/** + * @author vincent Bourgmayergit status + */ @RunWith(RobolectricTestRunner.class) @Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) public class RemoteContentScannerTest { private final Context context; private final Account account; - private RemoteContentScanner scanner_under_test; + private RemoteContentScanner scannerUnderTest; public RemoteContentScannerTest(){ context = ApplicationProvider.getApplicationContext(); @@ -51,9 +63,10 @@ public class RemoteContentScannerTest { @Before public void setUp() { - prepareDB(); //Create DB - List directoryList = new ArrayList<>(); //Improve to have realistic Content - scanner_under_test = new RemoteContentScanner(context, account, directoryList); + prepareDB(); + List directoryList = new ArrayList<>(); + directoryList.add(new SyncedFolder("Photos", "local/path/", "remote/path/", true, true, true, true)); + scannerUnderTest = new RemoteContentScanner(context, account, directoryList); } private void prepareDB(){ @@ -88,6 +101,11 @@ public class RemoteContentScannerTest { } + /** + * This is a base, that check that it works as expected in a normal situation + * todo: decline this test to cover unexpected cases! + * i.e: what if dbContent is empty + */ @Test public void scanEmptyCloudContent_generateLocalDeletionRequest() { final List cloudContent = new ArrayList<>(); @@ -100,18 +118,142 @@ public class RemoteContentScannerTest { Assert.assertFalse("List of SyncedFileState is empty", dbContent.isEmpty()); - final HashMap scanResult = scanner_under_test.scanContent(cloudContent, dbContent); + final HashMap scanResult = scannerUnderTest.scanContent(cloudContent, dbContent); - //If the list of SyncedFileState is not empty then all the local file must be deleted... - //In fact, the list of SyncedFileStates that is given as parameter to scanContent, should only contains content - //that has been identified has potentially changed, new or removed - //However, the issue is that...the scanner remove the file by itself, which should be out of its scope - //TODO: find how to assert the behaviour in current state ? + /* If the list of SyncedFileState is not empty then all the local file must be deleted... + * In fact, the list of SyncedFileStates that is given as parameter to scanContent, should only contains content + * that has been identified has potentially changed, new or removed + * However, the issue is that...the scanner remove the file by itself, which should be out of its scope + * TODO: find how to assert the behaviour in current state ? */ - Assert.assertEquals("SyncRequest doesn't match the expected result", 3, scanResult.size()); + Assert.assertEquals("scanResult's size doesn't match the expected result", 3, scanResult.size()); for (SyncRequest request : scanResult.values()) { Assert.assertEquals("SyncRequest's type should be LOCAL_DELETE but is "+request.getOperationType(), SyncRequest.Type.LOCAL_DELETE, request.getOperationType()); } } + + private List generateListOfNewRemoteFile() { + final ArrayList result = new ArrayList<>(); + + final RemoteFile newFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(newFile1.getRemotePath()).thenReturn("remote/path/tutu"); + Mockito.when(newFile1.getEtag()).thenReturn("33323"); + + + final RemoteFile newFile2 = Mockito.mock(RemoteFile.class); + Mockito.when(newFile2.getRemotePath()).thenReturn("remote/path/tete"); + Mockito.when(newFile2.getEtag()).thenReturn("33423"); + + + result.add(newFile1); + result.add(newFile2); + + return result; + } + + + /** + * This is a base, that check that it works as expected in a normal situation + * todo: decline this test to cover unexpected cases! + * i.e: what if dbContent is empty + */ + @Test + public void scanUpdatedRemoteContent_generateDownloadRequest() { + final List cloudContent = new ArrayList<>(); + + final RemoteFile updatedFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(updatedFile1.getRemotePath()).thenReturn("remote/path/toto"); + Mockito.when(updatedFile1.getEtag()).thenReturn("33423"); + Mockito.when(updatedFile1.getSize()).thenReturn(12l); + + final RemoteFile updatedFile2 = Mockito.mock(RemoteFile.class); + Mockito.when(updatedFile2.getRemotePath()).thenReturn("remote/path/titi"); + Mockito.when(updatedFile2.getEtag()).thenReturn("34523"); + Mockito.when(updatedFile2.getSize()).thenReturn(14l); + + final RemoteFile uptodateFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(uptodateFile1.getRemotePath()).thenReturn("remote/path/tata"); + Mockito.when(uptodateFile1.getEtag()).thenReturn("5557"); + + cloudContent.add(uptodateFile1); + cloudContent.add(updatedFile1); + cloudContent.add(updatedFile2); + + Assert.assertEquals("Expected cloudContent size: 3 but got : " + cloudContent.size(), 3, cloudContent.size()); + + + final List dbContent = new ArrayList<>(); + dbContent.add(new SyncedFileState(5, "toto", "local/path/toto", "remote/path/toto", "5555", 22222, 2, true, 3)); + dbContent.add(new SyncedFileState(3, "titi", "local/path/titi", "remote/path/titi", "5556", 22322, 2, true, 3)); + dbContent.add(new SyncedFileState(2, "tata", "local/path/tata", "remote/path/tata", "5557", 22232, 2, true, 3)); + + Assert.assertFalse("List of SyncedFileState is empty", dbContent.isEmpty()); + + final HashMap scanResult = scannerUnderTest.scanContent(cloudContent, dbContent); + Assert.assertEquals("scanResult's size doesn't match the expected result", 2, scanResult.size()); + + for (SyncRequest request : scanResult.values()) { + Assert.assertEquals("SyncRequest's type should be DOWNLOAD but is "+request.getOperationType(), SyncRequest.Type.DOWNLOAD, request.getOperationType()); + } + } + + /** + * This is a base, that check that it works as expected in a normal situation + * todo: decline this test to cover unexpected cases! + * i.e: what if dbContent is empty + */ + @Test + public void scanNewRemoteContent_generateDownloadRequest() { + final List cloudContent = generateListOfNewRemoteFile(); + + final RemoteFile updatedFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(updatedFile1.getRemotePath()).thenReturn("remote/path/toto"); + Mockito.when(updatedFile1.getEtag()).thenReturn("5555"); + + final RemoteFile updatedFile2 = Mockito.mock(RemoteFile.class); + Mockito.when(updatedFile2.getRemotePath()).thenReturn("remote/path/titi"); + Mockito.when(updatedFile2.getEtag()).thenReturn("5556"); + + final RemoteFile uptodateFile1 = Mockito.mock(RemoteFile.class); + Mockito.when(uptodateFile1.getRemotePath()).thenReturn("remote/path/tata"); + Mockito.when(uptodateFile1.getEtag()).thenReturn("5557"); + + cloudContent.add(uptodateFile1); + cloudContent.add(updatedFile1); + cloudContent.add(updatedFile2); + + Assert.assertEquals("Expected cloudContent size: 5 but got : " + cloudContent.size(), 5, cloudContent.size()); + + + final List dbContent = new ArrayList<>(); + dbContent.add(new SyncedFileState(5, "toto", "local/path/toto", "remote/path/toto", "5555", 22222, 2, true, 3)); + dbContent.add(new SyncedFileState(3, "titi", "local/path/titi", "remote/path/titi", "5556", 22322, 2, true, 3)); + dbContent.add(new SyncedFileState(2, "tata", "local/path/tata", "remote/path/tata", "5557", 22232, 2, true, 3)); + + Assert.assertFalse("List of SyncedFileState is empty", dbContent.isEmpty()); + + final HashMap scanResult = scannerUnderTest.scanContent(cloudContent, dbContent); + Assert.assertEquals("scanResult's size doesn't match the expected result", 2, scanResult.size()); + + for (SyncRequest request : scanResult.values()) { + Assert.assertEquals("SyncRequest's type should be DOWNLOAD but is "+request.getOperationType(), SyncRequest.Type.DOWNLOAD, request.getOperationType()); + } + } + + + + + @Test + public void whenDoWeGetUnexpectedFileDeletion() { + final List cloudContent = new ArrayList<>(); + final List dbContent = new ArrayList<>(); + + final HashMap scanResult = scannerUnderTest.scanContent(cloudContent, dbContent); + Assert.assertEquals("scanResult's size doesn't match the expected result", 0, scanResult.size()); + + for (SyncRequest request : scanResult.values()) { + System.out.println(request.getOperationType() + "for " +request.getSyncedFileState().getRemotePath()); + } + } } \ No newline at end of file -- GitLab From 448507e753c5cd7459d5851d1df16e293ac89e6a Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 28 Oct 2022 10:34:00 +0200 Subject: [PATCH 04/16] Changes in contentScanner package & listFileRemoteOperation - Fix method name in contentScanner : onMissingRemoteFile() replaced by onMissingFile() - Removed useless comment in contentScanner classes - Extract http request in dedicated method for testing in ListFileRemoteOperation --- .../AbstractContentScanner.java | 4 ++-- .../contentScanner/LocalContentScanner.java | 8 +++----- .../contentScanner/RemoteContentScanner.java | 5 +---- .../operations/ListFileRemoteOperation.java | 20 +++++++++++++++++-- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java index cd841618..45fa47a6 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractContentScanner.java @@ -67,7 +67,7 @@ public abstract class AbstractContentScanner { } for(SyncedFileState remainingFileState : fileStates) { - onMissingRemoteFile(remainingFileState); + onMissingFile(remainingFileState); } return syncRequests; }; @@ -93,7 +93,7 @@ public abstract class AbstractContentScanner { * When a file doesn't exist anymore we remove it from device/cloud (depending of implementation) & from Database * @param fileState SyncedFileState for which we lack remote file */ - protected abstract void onMissingRemoteFile(SyncedFileState fileState); + protected abstract void onMissingFile(SyncedFileState fileState); /** * A new file has been found diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java index 23ac48db..845323d7 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalContentScanner.java @@ -33,7 +33,7 @@ public class LocalContentScanner extends AbstractContentScanner{ } @Override - protected void onMissingRemoteFile(SyncedFileState fileState) { + protected void onMissingFile(SyncedFileState fileState) { if (!fileState.hasBeenSynchronizedOnce()) { return; } @@ -61,12 +61,10 @@ public class LocalContentScanner extends AbstractContentScanner{ if (parentDir.isScanLocal()) scannableValue += 2; } - //create the syncedFile State final SyncedFileState newSyncedFileState = new SyncedFileState(-1, file.getName(), filePath, parentDir.getRemoteFolder() + file.getName(), "", 0, parentDir.getId(), parentDir.isMediaType(),scannableValue); - //Store it in DB - int storedId = DbHelper.manageSyncedFileStateDB(newSyncedFileState, "INSERT", context); - if (storedId > 0){ + final int storedId = DbHelper.manageSyncedFileStateDB(newSyncedFileState, "INSERT", context); + if (storedId > 0) { newSyncedFileState.setId( storedId ); Timber.d("Add upload SyncRequest for new file %s", filePath); syncRequests.put(storedId, new SyncRequest(newSyncedFileState, SyncRequest.Type.UPLOAD)); diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java index 31e13b85..3e7c4ba1 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteContentScanner.java @@ -12,11 +12,9 @@ import static foundation.e.drive.utils.FileDiffUtils.getActionForFileDiff; import android.accounts.Account; import android.content.Context; -import android.provider.MediaStore; import com.owncloud.android.lib.resources.files.model.RemoteFile; -import java.io.File; import java.util.List; import foundation.e.drive.database.DbHelper; @@ -76,7 +74,6 @@ public class RemoteContentScanner extends AbstractContentScanner { final SyncedFileState newFileState = new SyncedFileState(-1, fileName, parentDir.getLocalFolder() + fileName, remoteFilePath, file.getEtag(), 0, parentDir.getId(), parentDir.isMediaType(), scannableValue); - //Store it in DB final int storedId = DbHelper.manageSyncedFileStateDB(newFileState, "INSERT", context); if (storedId > 0) { newFileState.setId(storedId); @@ -88,7 +85,7 @@ public class RemoteContentScanner extends AbstractContentScanner { } @Override - protected void onMissingRemoteFile(SyncedFileState fileState) { + protected void onMissingFile(SyncedFileState fileState) { this.syncRequests.put(fileState.getId(), new SyncRequest(fileState, LOCAL_DELETE)); /* todo: move commented code into synchronizationService diff --git a/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java b/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java index 0ab10daa..fa1a9c67 100644 --- a/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java @@ -10,6 +10,9 @@ package foundation.e.drive.operations; import android.content.Context; + +import androidx.annotation.VisibleForTesting; + import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.operations.RemoteOperation; import com.owncloud.android.lib.common.operations.RemoteOperationResult; @@ -65,6 +68,7 @@ public class ListFileRemoteOperation extends RemoteOperation Date: Mon, 31 Oct 2022 15:53:39 +0100 Subject: [PATCH 05/16] Implement new classes to handle listing of existing content The goal is to have dedicated classes which will allow dedicated tests - AbstractFileLister: which is an abstract class that bear the general workflow - RemoteFileLister: implementation of AbstractFileLister for RemoteContent - LocalFileLister: implementation of AbstractFileLister for LocalContent - FolderWrapper: a utility class used by the implementations of AbstractFileLister --- .../contentScanner/AbstractFileLister.java | 192 ++++++++++++++++++ .../e/drive/contentScanner/FolderWrapper.java | 55 +++++ .../drive/contentScanner/LocalFileLister.java | 93 +++++++++ .../contentScanner/RemoteFileLister.java | 117 +++++++++++ 4 files changed, 457 insertions(+) create mode 100644 app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java create mode 100644 app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java create mode 100644 app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java create mode 100644 app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java new file mode 100644 index 00000000..59516904 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -0,0 +1,192 @@ +/* + * Copyright © ECORP SAS 2022. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ +package foundation.e.drive.contentScanner; + +import android.content.Context; + +import java.util.ArrayList; +import java.util.List; +import java.util.ListIterator; + +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFolder; +import timber.log.Timber; + +/** + * Factorisation of codes used to list file on cloud and on device + * Implementation will vary depending of it is RemoteFile or File instance + * that we want to list + * @author vincent Bourgmayer + */ +public abstract class AbstractFileLister { + + /** + * This allows to use RemoteOperationResult for RemoteFileLister + * todo: some optimization may be done with FolderWrapper + * @param RemoteFile or File + */ + /* package */ interface DirectoryLoader { + /* package */ FolderWrapper getFolder(); + /* package */ boolean load(SyncedFolder folder); + } + + protected final List folders; + private final List contentToScan; + + /** + * constructor to be call with super + * @param folders List of SyncedFolder to read + */ + protected AbstractFileLister(List folders) { + this.folders = folders; + this.contentToScan = new ArrayList<>(); + } + + /** + * Perform the listing of files + * @param context + * @return + */ + public boolean listContentToScan(Context context) { + //For each SyncedFolder + final ListIterator iterator = folders.listIterator() ; + boolean isSomeContentToSync = false; + while (iterator.hasNext()) { + final SyncedFolder syncedFolder = iterator.next(); + Timber.v("SyncedFolder : %s, %s, %s, %s, %s", syncedFolder.getLibelle(), syncedFolder.getLocalFolder(), syncedFolder.getLastModified(), syncedFolder.isScanLocal(), syncedFolder.getId()); + isSomeContentToSync = isSomeContentToSync || hasDirectoryContentToScan(syncedFolder, iterator, context); + } + + if (isSomeContentToSync) { + DbHelper.updateSyncedFolders(folders, context); //@ToDo: maybe do this when all contents will be synced. + } + return isSomeContentToSync; + } + + + /** + * Detailed process for reading a single folder + * @param syncedFolder the SyncedFolder to check + * @param iterator iterator over list of SyncedFolder + * @param context context used to call DB + * @return true the directory has content to synchronized false otherwise + */ + private boolean hasDirectoryContentToScan( SyncedFolder syncedFolder, ListIterator iterator, Context context) { + // I check if I can skip it; If then then go to next + final DirectoryLoader dirLoader = getDirectoryLoader(); + if (skipSyncedFolder(syncedFolder) + || !isSyncedFolderInDb(syncedFolder, context) + || !dirLoader.load(syncedFolder)) { // I try to get the real directory (File or RemoteFile). + iterator.remove(); + return false; + } + + final FolderWrapper currentDirectory = dirLoader.getFolder(); + + // If directory is missing (not exist or http 404) + if (currentDirectory.isMissing()) { + return true; + } + + //--> I no content has changed, skip + if (skipDirectory(currentDirectory.getFolder(), syncedFolder, context)) { + iterator.remove(); + return false; + } + + //else I have to check if some content has changed inside + final List dirContent = currentDirectory.getContent(); + + //todo: look where to put in subclasses : syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); + + if (!dirContent.isEmpty()) { + contentToScan.addAll(sortContent(iterator, syncedFolder, dirContent)); + } + return true; + } + + + /** + * Tell if the given syncedFolder has been persisted in database. + * If it's not the case initally, it tries to persist it. + * @param syncedFolder SyncedFolder to check for persistance + * @param context context to contact database + * @return false if not persisted in db + */ + private boolean isSyncedFolderInDb(SyncedFolder syncedFolder, Context context) { + if (syncedFolder.getId() > 0) return true; + + final int syncedFolder_id = (int) DbHelper.insertSyncedFolder(syncedFolder, context); //It will return -1 if there is an error, like an already existing folder with same value + if (syncedFolder_id <= 0) { + Timber.v("insertion of syncedFolder for %s failed: %s ", syncedFolder.getRemoteFolder(), syncedFolder_id); + return false; + } + + syncedFolder.setId(syncedFolder_id); + return true; + } + + + /** + * Split content of a folder: subfolder on one side are added into the iterator loop + * while subfiles are added to result list + * @param iterator iterator intance over list of SyncedFolder + * @param syncedFolder the SyncFolder which own the content + * @param dirContent Content to sort + * @return List of subfiles to scan or empty list if nothing + */ + private List sortContent(ListIterator iterator, SyncedFolder syncedFolder, List dirContent) { + final List result = new ArrayList<>(); + if (dirContent == null) return result; + + for (T subFile : dirContent) { + final String fileName = getFileName(subFile); + if (isDirectory(subFile)) { + final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, getFileName(subFile), 0L, "");//Need to set lastModified to 0 to handle it on next iteration + iterator.add(subSyncedFolder); + iterator.previous(); + } else if (!skipFile(subFile)) { + Timber.v("added %s into list of file to scan", fileName); + result.add(subFile); + } + } + + return result; + } + + /** + * List of file to scan + * @return List or List is expected based on implementations + */ + public List getContentToScan() { + return contentToScan; + } + + /** + * Share the list of syncedFolder's ID for syncedFolder which has content to scan + * @return List of syncedFolder ids + */ + public List getSyncedFoldersId() { + final List result = new ArrayList<>(); + for (SyncedFolder syncedFolder : folders) { + //if (syncedFolder.isToSync() ){ //todo: is it usefull ? maybe this propertie can be removed from SyncedFolder + result.add( (long) syncedFolder.getId() ); + //} + } + return result; + } + + + + protected abstract boolean skipSyncedFolder(SyncedFolder syncedFolder); + protected abstract boolean skipFile(T file); + protected abstract boolean skipDirectory(T currentDirectory, SyncedFolder syncedFolder, Context context); + protected abstract boolean isDirectory(T file); + protected abstract String getFileName(T file); + protected abstract DirectoryLoader getDirectoryLoader(); +} diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java new file mode 100644 index 00000000..ca273584 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java @@ -0,0 +1,55 @@ +/* + * Copyright © ECORP SAS 2022. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ +package foundation.e.drive.contentScanner; + +import java.util.ArrayList; +import java.util.List; + +/** + * This wrapper is used to contains either a File/RemoteFile instance with its content + * or a "missing file" information. The explanation of this, is because scanning remote file + * may result in different type of error while loading content (network issues, etc.) + * but we want to handle 404 error (= missing resources) has to be considered like a + * potential file deletions. There is probably something better to do for that + * @author vincent Bourgmayer + * @param File or RemoteFile expected + */ +public class FolderWrapper { + + private final boolean missing; + private final T folder; + private final List content; + + protected FolderWrapper(T folder) { + content = new ArrayList<>(); + this.folder = folder; + missing = false; + } + + protected FolderWrapper() { + missing = true; + folder = null; + content = null; + } + + public T getFolder() { + return folder; + } + + public List getContent() { + return content; + } + + public void addContent(List newContent) { + content.addAll(newContent); + } + + public boolean isMissing() { + return missing; + } +} diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java new file mode 100644 index 00000000..93fed784 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -0,0 +1,93 @@ +/* + * Copyright © ECORP SAS 2022. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ +package foundation.e.drive.contentScanner; + +import android.content.Context; + +import java.io.File; +import java.io.FileFilter; +import java.util.Arrays; +import java.util.List; + +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.fileFilters.FileFilterFactory; +import foundation.e.drive.models.SyncedFolder; +import foundation.e.drive.utils.CommonUtils; +import timber.log.Timber; + +/** + * Implementation of AbstractFileLister with method adapted to local content + * @author vincent Bourgmayer + */ +public class LocalFileLister extends AbstractFileLister { + public LocalFileLister(List directories) { + super(directories); + } + + @Override + protected boolean skipDirectory(File currentDirectory, SyncedFolder syncedFolder, Context context) { + return currentDirectory.lastModified() == syncedFolder.getLastModified() + || !DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), context); + } + + @Override + protected boolean skipSyncedFolder(SyncedFolder syncedFolder) { + final String fileName = CommonUtils.getFileNameFromPath(syncedFolder.getLocalFolder()); + if (fileName == null || fileName.startsWith(".") || !syncedFolder.isScanLocal()) { + return true; + } + + return false; + } + + @Override + protected boolean skipFile(File file) { + return file.isHidden(); + } + + @Override + protected boolean isDirectory(File file) { + return file.isDirectory(); + } + + @Override + protected String getFileName(File file) { + return file.getName(); + } + + @Override + protected DirectoryLoader getDirectoryLoader() { + return new LocalDirLoader(); + } + + private final class LocalDirLoader implements DirectoryLoader { + private FolderWrapper folder; + + @Override + public FolderWrapper getFolder() { + return folder; + } + + @Override + public boolean load(SyncedFolder syncedFolder) { + final File dir = new File(syncedFolder.getLocalFolder()); + Timber.v("Local Folder (last modified / exists): %s, %s", dir.lastModified(),dir.exists()); + + if (!dir.exists()) { + folder = new FolderWrapper(); //missing Folder Wrapper + } + else { + folder = new FolderWrapper(dir); + final String category = syncedFolder.isMediaType() ? "media" : syncedFolder.getLibelle(); + final FileFilter filter = FileFilterFactory.getFileFilter(category); + folder.addContent(Arrays.asList(dir.listFiles(filter))); + } + return true; + } + } +} diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java new file mode 100644 index 00000000..8a3a0313 --- /dev/null +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -0,0 +1,117 @@ +/* + * Copyright © ECORP SAS 2022. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ +package foundation.e.drive.contentScanner; + +import android.content.Context; + +import androidx.annotation.VisibleForTesting; + +import com.owncloud.android.lib.common.OwnCloudClient; +import com.owncloud.android.lib.common.operations.RemoteOperationResult; +import com.owncloud.android.lib.resources.files.ReadFolderRemoteOperation; +import com.owncloud.android.lib.resources.files.model.RemoteFile; + +import java.util.ArrayList; +import java.util.List; + +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFolder; +import foundation.e.drive.utils.CommonUtils; + + +/** + * Implementation of AbstractFileLister with method adapted to remote content + * @author vincent Bourgmayer + */ +public class RemoteFileLister extends AbstractFileLister { + private static final int HTTP_404 = 404; + private OwnCloudClient client; + + public RemoteFileLister(List directories, OwnCloudClient client) { + super(directories); + this.client = client; + } + + @Override + protected boolean skipSyncedFolder(SyncedFolder syncedFolder) { + return (syncedFolder.isMediaType() + && CommonUtils.getFileNameFromPath(syncedFolder.getRemoteFolder()).startsWith(".")) + || !syncedFolder.isScanRemote(); + } + + @Override + protected boolean skipFile(RemoteFile file) { + final String fileName = CommonUtils.getFileNameFromPath(file.getRemotePath()); + return fileName.startsWith("."); + } + + @Override + protected boolean skipDirectory(RemoteFile currentDirectory, SyncedFolder syncedFolder, Context context) { + return currentDirectory.getEtag().equals(syncedFolder.getLastEtag()) + || !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context); + } + + @Override + protected boolean isDirectory(RemoteFile file) { + return file.getMimeType().equals("DIR"); + } + + @Override + protected String getFileName(RemoteFile file) { + return null; + } + + @Override + protected DirectoryLoader getDirectoryLoader() { + return new RemoteDirectoryLoader(); + } + + private final class RemoteDirectoryLoader implements DirectoryLoader { + + private FolderWrapper directory; + + @Override + public boolean load(SyncedFolder syncedFolder) { + final RemoteOperationResult remoteOperationResult = readRemoteFolder(syncedFolder.getRemoteFolder(), client); + + if (!remoteOperationResult.isSuccess()) { + if (remoteOperationResult.getHttpCode() == HTTP_404) { + directory = new FolderWrapper<>(); + return true; + } + return false; + } + + final int dataSize = remoteOperationResult.getData().size(); + directory = new FolderWrapper<>( (RemoteFile) remoteOperationResult.getData().get(0)); + List subFiles = new ArrayList<>(); + + for (Object o: remoteOperationResult.getData().subList(1, dataSize)) { + subFiles.add((RemoteFile) o); + } + return true; + } + + /** + * Execute the Propfind query to list files under a directory + * @param remotePath RemotePath of the directory to observe + * @param client Client used to execute query + * @return RemoteOperationResult + */ + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + private RemoteOperationResult readRemoteFolder(String remotePath, OwnCloudClient client) { + final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(remotePath); + return operation.execute(client); + } + + @Override + public FolderWrapper getFolder() { + return directory; + } + } +} -- GitLab From b3eae457e20613f838f81c6d59e5f3e6be7db947 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 31 Oct 2022 15:59:58 +0100 Subject: [PATCH 06/16] Call FileLister implementation from ObserverService & ListFileRemoteOperation Remove code for listing local and remote content from ObserverService & ListFileRemoteOperation It makes the class lighter, easier to understand and to test --- .../operations/ListFileRemoteOperation.java | 158 ++---------------- .../e/drive/services/ObserverService.java | 109 +++--------- 2 files changed, 35 insertions(+), 232 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java b/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java index fa1a9c67..627f31ff 100644 --- a/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java +++ b/app/src/main/java/foundation/e/drive/operations/ListFileRemoteOperation.java @@ -22,6 +22,8 @@ import java.io.File; import java.util.ArrayList; import java.util.List; import java.util.ListIterator; + +import foundation.e.drive.contentScanner.RemoteFileLister; import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.utils.CommonUtils; @@ -33,19 +35,17 @@ import timber.log.Timber; * @author Vincent Bourgmayer */ public class ListFileRemoteOperation extends RemoteOperation> { - private static final int HTTP_404 = 404; private final List syncedFolders; private final Context context; - private final int initialFolderNumber; - private final ArrayList remoteFiles; - public ListFileRemoteOperation(List syncedFolders, Context context, int initialFolderNumber) { + private final ArrayList updatedSyncedFoldersId; + + public ListFileRemoteOperation(List syncedFolders, Context context) { Timber.tag(ListFileRemoteOperation.class.getSimpleName()); this.syncedFolders = syncedFolders; this.context = context; - this.initialFolderNumber = initialFolderNumber; - remoteFiles = new ArrayList<>(); + updatedSyncedFoldersId = new ArrayList<>(); } /** @@ -55,151 +55,23 @@ public class ListFileRemoteOperation extends RemoteOperation> run(OwnCloudClient ownCloudClient) { - RemoteOperationResult finalResult; - - boolean atLeastOneDirAsChanged = false; - final ListIterator mSyncedFolderIterator = syncedFolders.listIterator(); - - while (mSyncedFolderIterator.hasNext()) { - try { - Thread.sleep(150); - } catch (InterruptedException exception) { - Timber.v( "listFileRemoteOperation's sleep had been interrupted"); - } - - final SyncedFolder syncedFolder = mSyncedFolderIterator.next(); + final RemoteFileLister fileLister = new RemoteFileLister(syncedFolders, ownCloudClient); + final boolean isContentToScan = fileLister.listContentToScan(context); - if (shouldSkipSyncedFolder(syncedFolder)) { - mSyncedFolderIterator.remove(); - continue; - } - - if (syncedFolder.getId() == -1) { - final int syncedFolderId = (int) DbHelper.insertSyncedFolder(syncedFolder, context); - if (syncedFolderId <= 0) { - mSyncedFolderIterator.remove(); - Timber.v("insertion of syncedFolder for %s failed: %s ", syncedFolder.getRemoteFolder(), syncedFolderId); - continue; - } - syncedFolder.setId(syncedFolderId); - } - - final RemoteOperationResult result = readRemoteFolder(syncedFolder.getRemoteFolder(), ownCloudClient); - - if (result.isSuccess()) { - final boolean dirHasChanged = onListFileSuccess(mSyncedFolderIterator, syncedFolder, result); - atLeastOneDirAsChanged = dirHasChanged || atLeastOneDirAsChanged; //stay true if set previously - } else { - if (result.getHttpCode() == HTTP_404) { //dir has not been found - atLeastOneDirAsChanged = true; - onHttp404Received(mSyncedFolderIterator, syncedFolder); - } - Timber.d("ReadFolderRemoteOperation failed : http %s, %s => ignored", result.getHttpCode(), result.getLogMessage()); - } - } - finalResult = new RemoteOperationResult<>(RemoteOperationResult.ResultCode.OK); - if (atLeastOneDirAsChanged) { + final RemoteOperationResult result = new RemoteOperationResult<>(RemoteOperationResult.ResultCode.OK); + if (isContentToScan) { DbHelper.updateSyncedFolders(this.syncedFolders, this.context); - finalResult.setResultData(remoteFiles); - } - return finalResult; - } - - /** - * Execute the Propfind query to list files under a directory - * @param remotePath RemotePath of the directory to observe - * @param client Client used to execute query - * @return RemoteOperationResult - */ - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - public RemoteOperationResult readRemoteFolder(String remotePath, OwnCloudClient client) { - final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(remotePath); - return operation.execute(client); - } - - - /** - * Analyze list of remote files for a given Directory - * @param iterator ListIterator contains list of syncedFolder to check - * @param syncedFolder the SyncedFolder for the given Directory - * @param result Result of ListRemoteFile operation - * @return true if some directory has changed or its content - */ - private boolean onListFileSuccess(final ListIterator iterator, SyncedFolder syncedFolder, RemoteOperationResult result) { - final int dataSize = result.getData().size(); - final RemoteFile directory = (RemoteFile) result.getData().get(0); - - if (directory.getEtag().equals(syncedFolder.getLastEtag()) - && !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context)) { - return false; + result.setResultData(fileLister.getContentToScan()); } - syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); + updatedSyncedFoldersId.addAll(fileLister.getSyncedFoldersId()); - final List subFiles = result.getData().subList(1, dataSize); - - for (int i = -1, subFilesSize = subFiles.size(); ++i < subFilesSize;) { - final RemoteFile remoteFile = (RemoteFile) subFiles.get(i); - - if (remoteFile.getMimeType().equals("DIR")) { - final String suffixPath = remoteFile.getRemotePath().substring(syncedFolder.getRemoteFolder().length()); - final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, suffixPath, 0L, ""); //need to set empty etag to allow it to be scan - iterator.add(subSyncedFolder); - iterator.previous(); - } else if (!isHiddenFile(remoteFile)) { - Timber.v("Add remote file %s", remoteFile.getRemotePath()); - this.remoteFiles.add(remoteFile); - } - } - return true; - } - - /** - * Handle a case where the checked directory is missing on cloud - * @param iterator ListIterator list of syncedFolder to scan - * @param syncedFolder Missing syncedFolder on cloud - */ - private void onHttp404Received(final ListIterator iterator, SyncedFolder syncedFolder) { - syncedFolder.setToSync(true); - - final File localFolder = new File(syncedFolder.getLocalFolder()); - if (localFolder.listFiles().length == 0) { - localFolder.delete(); - } - - if (!localFolder.exists()) { - if (syncedFolder.getId() > this.initialFolderNumber) { //Do not remove initial syncedFolder - DbHelper.deleteSyncedFolder(syncedFolder.getId(), context); - } - iterator.remove(); - } - } - - - /** - * Indicate if remote file is an hidden file - * @param file Remote file - * @return true if it's an hidden file - */ - private boolean isHiddenFile(RemoteFile file) { - final String fileName = CommonUtils.getFileNameFromPath(file.getRemotePath()); - return fileName.startsWith("."); - } - - /** - * Indicate if we should skip SyncedFolder - * @param syncedFolder - * @return - */ - private boolean shouldSkipSyncedFolder(SyncedFolder syncedFolder) { - return (syncedFolder.isMediaType() - && CommonUtils.getFileNameFromPath(syncedFolder.getRemoteFolder()).startsWith(".")) - || !syncedFolder.isScanRemote(); + return result; } /** * @return list of syncedFolder */ - public List getSyncedFolderList(){ - return this.syncedFolders; + public List getSyncedFoldersId(){ + return this.updatedSyncedFoldersId; } } diff --git a/app/src/main/java/foundation/e/drive/services/ObserverService.java b/app/src/main/java/foundation/e/drive/services/ObserverService.java index 7d6a2239..0da565c1 100644 --- a/app/src/main/java/foundation/e/drive/services/ObserverService.java +++ b/app/src/main/java/foundation/e/drive/services/ObserverService.java @@ -41,6 +41,7 @@ import java.util.List; import java.util.ListIterator; import foundation.e.drive.contentScanner.LocalContentScanner; +import foundation.e.drive.contentScanner.LocalFileLister; import foundation.e.drive.contentScanner.RemoteContentScanner; import foundation.e.drive.database.DbHelper; import foundation.e.drive.fileFilters.CrashlogsFileFilter; @@ -69,7 +70,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene private List mSyncedFolders; //List of synced folder private boolean isWorking = false; - private int initialFolderCounter; private Account mAccount; private HashMap syncRequests; //integer is SyncedFileState id; Parcelable is the operation private SynchronizationServiceConnection synchronizationServiceConnection = new SynchronizationServiceConnection(); @@ -113,7 +113,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene } this.syncRequests = new HashMap<>(); - initialFolderCounter = prefs.getInt(AppConstants.INITIALFOLDERS_NUMBER, 0); handlerThread = new HandlerThread("syncService_onResponse"); handlerThread.start(); @@ -263,7 +262,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene } try { - final ListFileRemoteOperation loadOperation = new ListFileRemoteOperation(this.mSyncedFolders, this, this.initialFolderCounter); + final ListFileRemoteOperation loadOperation = new ListFileRemoteOperation(this.mSyncedFolders, this); loadOperation.execute(client, this, handler); } catch (IllegalArgumentException exception) { Timber.e(exception); @@ -288,7 +287,7 @@ public class ObserverService extends Service implements OnRemoteOperationListene } else if (settingsSyncedEnabled) { return DbHelper.getSyncedFolderList(this, false); } else { - return new ArrayList(); + return new ArrayList<>(); } } @@ -309,16 +308,15 @@ public class ObserverService extends Service implements OnRemoteOperationListene final List remoteFiles = ((RemoteOperationResult>)result).getResultData(); if (remoteFiles != null) { - final ListFileRemoteOperation listFileOperation = (ListFileRemoteOperation) operation; - mSyncedFolders = listFileOperation.getSyncedFolderList(); //The list may have been reduced if some directory hasn't changed + final List syncedFoldersId = listFileOperation.getSyncedFoldersId(); - final List syncedFileStateList = DbHelper.getSyncedFileStatesByFolders(this, - getIdsFromFolderToScan()); + final List syncedFileStates = DbHelper.getSyncedFileStatesByFolders(this, + syncedFoldersId); - if (!remoteFiles.isEmpty() || !syncedFileStateList.isEmpty()) { + if (!remoteFiles.isEmpty() || !syncedFileStates.isEmpty()) { final RemoteContentScanner scanner = new RemoteContentScanner(getApplicationContext(), mAccount, mSyncedFolders); - syncRequests.putAll(scanner.scanContent(remoteFiles, syncedFileStateList)); + syncRequests.putAll(scanner.scanContent(remoteFiles, syncedFileStates)); } } @@ -350,22 +348,6 @@ public class ObserverService extends Service implements OnRemoteOperationListene } } - /** - * Method to get Id of SyncedFolder to scan - * @return List id of SyncedFolder to scan - */ - private List getIdsFromFolderToScan() { - final List result = new ArrayList<>(); - for (int i = -1, size = this.mSyncedFolders.size(); ++i < size;) { - final SyncedFolder syncedFolder = this.mSyncedFolders.get(i); - if (syncedFolder.isToSync() ){ - result.add( (long) syncedFolder.getId() ); - } - } - return result; - } - - /* Methods related to device Scanning */ /** @@ -405,76 +387,25 @@ public class ObserverService extends Service implements OnRemoteOperationListene */ private void scanLocalFiles(){ Timber.i("scanLocalFiles()"); - final List fileList = new ArrayList<>(); - final List folderIdList= new ArrayList<>(); - boolean contentToSyncFound = false; - if (CommonUtils.isSettingsSyncEnabled(mAccount)) generateAppListFile(); - final ListIterator iterator = mSyncedFolders.listIterator() ; - - while(iterator.hasNext()) { - final SyncedFolder syncedFolder = iterator.next(); - Timber.v("SyncedFolder : %s, %s, %s, %s, %s", syncedFolder.getLibelle(), syncedFolder.getLocalFolder(), syncedFolder.getLastModified(), syncedFolder.isScanLocal(), syncedFolder.getId()); - - //Check it's not a hidden file - final String fileName = CommonUtils.getFileNameFromPath(syncedFolder.getLocalFolder()); - if (fileName == null || fileName.startsWith(".") || !syncedFolder.isScanLocal()) { - iterator.remove(); - continue; - } - if (syncedFolder.getId() == -1) { - final int syncedFolder_id = (int) DbHelper.insertSyncedFolder(syncedFolder, this); //It will return -1 if there is an error, like an already existing folder with same value - if (syncedFolder_id <= 0) { - iterator.remove(); - continue; - } - syncedFolder.setId(syncedFolder_id); - } - - - final File localDirectory = new File(syncedFolder.getLocalFolder()); //Obtention du fichier local - Timber.v("Local Folder (last modified / exists): %s, %s", localDirectory.lastModified(),localDirectory.exists()); - - if (!localDirectory.exists()) { - contentToSyncFound = true; - folderIdList.add( (long) syncedFolder.getId() ); - continue; - } - - if (localDirectory.lastModified() > syncedFolder.getLastModified() - || DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), getApplicationContext())) { - syncedFolder.setLastModified(localDirectory.lastModified()); //@Todo: it would be better to set it after all it's content has been synced - contentToSyncFound = true; - folderIdList.add((long) syncedFolder.getId()); - } + final LocalFileLister fileLister = new LocalFileLister(mSyncedFolders); + final boolean isContentToScan = fileLister.listContentToScan(getApplicationContext()); - final FileFilter filter = FileFilterFactory.getFileFilter( (syncedFolder.isMediaType()) ? "media" : syncedFolder.getLibelle() ); - final File[] subFiles = localDirectory.listFiles(filter); //skip hidden media files - - if (subFiles == null) continue; - for (File subFile : subFiles) { - if (subFile.isDirectory()) { - final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, subFile.getName() + FileUtils.PATH_SEPARATOR, 0L, "");//Need to set lastModified to 0 to handle it on next iteration - iterator.add(subSyncedFolder); - iterator.previous(); - } else if (contentToSyncFound) { - Timber.v("added %s into list of file to scan", subFile.getAbsolutePath()); - fileList.add(subFile); - } - } + if (!isContentToScan) { + return; } - if (contentToSyncFound) { - DbHelper.updateSyncedFolders(mSyncedFolders, this); //@ToDo: maybe do this when all contents will be synced. - final List syncedFileStates = DbHelper.getSyncedFileStatesByFolders(this, - folderIdList); + final List fileList = fileLister.getContentToScan(); + final List folderIdList = fileLister.getSyncedFoldersId(); - if (!syncedFileStates.isEmpty() || !fileList.isEmpty() ) { - final LocalContentScanner scanner= new LocalContentScanner(getApplicationContext(), mAccount, mSyncedFolders); - syncRequests.putAll(scanner.scanContent(fileList, syncedFileStates)); - } + final List syncedFileStates = DbHelper.getSyncedFileStatesByFolders(this, + folderIdList); + + if (!syncedFileStates.isEmpty() || !fileList.isEmpty() ) { + final LocalContentScanner scanner= new LocalContentScanner(getApplicationContext(), mAccount, mSyncedFolders); + syncRequests.putAll(scanner.scanContent(fileList, syncedFileStates)); } } /* end of methods related to device Scanning */ -- GitLab From 94ada4c8606666e7d9a8c3ff630dcb6cd143a4db Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 2 Nov 2022 15:18:55 +0100 Subject: [PATCH 07/16] Add new Test file: RemoteFileListerTest.java with unit tests - Fix error in LocalFileLister.skipDirectory(...) and RemoteFileLister.skipDirectory() - Add test cases for skipSyncedFolder(...) in RemoteFileListerTest - Add test cases for SkipFile(...) in RemoteFileListerTest - Add test cases for SkipDirectory(...) in RemoteFileListerTest - Add test cases for LocalFileLister.RemoteDirectoryLoader.load() - rename some method to replace "directory" by "folder" to keep consistency across classes - Prevent NPE & indexOutOfBoundsException in RemoteFileLister.RemoteFolderLoader.load --- .../contentScanner/AbstractFileLister.java | 12 +- .../drive/contentScanner/LocalFileLister.java | 8 +- .../contentScanner/RemoteFileLister.java | 39 +- .../contentScanner/LocalFileListerTest.java | 2 + .../contentScanner/RemoteFileListerTest.java | 365 ++++++++++++++++++ 5 files changed, 399 insertions(+), 27 deletions(-) create mode 100644 app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java create mode 100644 app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 59516904..34dfe74b 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -30,8 +30,8 @@ public abstract class AbstractFileLister { * todo: some optimization may be done with FolderWrapper * @param RemoteFile or File */ - /* package */ interface DirectoryLoader { - /* package */ FolderWrapper getFolder(); + /* package */ interface FolderLoader { + /* package */ FolderWrapper getFolderWrapper(); /* package */ boolean load(SyncedFolder folder); } @@ -78,7 +78,7 @@ public abstract class AbstractFileLister { */ private boolean hasDirectoryContentToScan( SyncedFolder syncedFolder, ListIterator iterator, Context context) { // I check if I can skip it; If then then go to next - final DirectoryLoader dirLoader = getDirectoryLoader(); + final FolderLoader dirLoader = getFolderLoader(); if (skipSyncedFolder(syncedFolder) || !isSyncedFolderInDb(syncedFolder, context) || !dirLoader.load(syncedFolder)) { // I try to get the real directory (File or RemoteFile). @@ -86,7 +86,7 @@ public abstract class AbstractFileLister { return false; } - final FolderWrapper currentDirectory = dirLoader.getFolder(); + final FolderWrapper currentDirectory = dirLoader.getFolderWrapper(); // If directory is missing (not exist or http 404) if (currentDirectory.isMissing()) { @@ -147,7 +147,7 @@ public abstract class AbstractFileLister { for (T subFile : dirContent) { final String fileName = getFileName(subFile); if (isDirectory(subFile)) { - final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, getFileName(subFile), 0L, "");//Need to set lastModified to 0 to handle it on next iteration + final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName, 0L, "");//Need to set lastModified to 0 to handle it on next iteration iterator.add(subSyncedFolder); iterator.previous(); } else if (!skipFile(subFile)) { @@ -188,5 +188,5 @@ public abstract class AbstractFileLister { protected abstract boolean skipDirectory(T currentDirectory, SyncedFolder syncedFolder, Context context); protected abstract boolean isDirectory(T file); protected abstract String getFileName(T file); - protected abstract DirectoryLoader getDirectoryLoader(); + protected abstract FolderLoader getFolderLoader(); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 93fed784..a37fcf00 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -32,7 +32,7 @@ public class LocalFileLister extends AbstractFileLister { @Override protected boolean skipDirectory(File currentDirectory, SyncedFolder syncedFolder, Context context) { return currentDirectory.lastModified() == syncedFolder.getLastModified() - || !DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), context); + && !DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), context); } @Override @@ -61,15 +61,15 @@ public class LocalFileLister extends AbstractFileLister { } @Override - protected DirectoryLoader getDirectoryLoader() { + protected FolderLoader getFolderLoader() { return new LocalDirLoader(); } - private final class LocalDirLoader implements DirectoryLoader { + private final class LocalDirLoader implements FolderLoader { private FolderWrapper folder; @Override - public FolderWrapper getFolder() { + public FolderWrapper getFolderWrapper() { return folder; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index 8a3a0313..eec94bf9 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -29,7 +29,6 @@ import foundation.e.drive.utils.CommonUtils; * @author vincent Bourgmayer */ public class RemoteFileLister extends AbstractFileLister { - private static final int HTTP_404 = 404; private OwnCloudClient client; public RemoteFileLister(List directories, OwnCloudClient client) { @@ -47,13 +46,13 @@ public class RemoteFileLister extends AbstractFileLister { @Override protected boolean skipFile(RemoteFile file) { final String fileName = CommonUtils.getFileNameFromPath(file.getRemotePath()); - return fileName.startsWith("."); + return fileName == null || fileName.isEmpty() || fileName.startsWith("."); } @Override - protected boolean skipDirectory(RemoteFile currentDirectory, SyncedFolder syncedFolder, Context context) { - return currentDirectory.getEtag().equals(syncedFolder.getLastEtag()) - || !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context); + protected boolean skipDirectory(RemoteFile directory, SyncedFolder syncedFolder, Context context) { + return directory.getEtag().equals(syncedFolder.getLastEtag()) + && !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context); } @Override @@ -63,23 +62,25 @@ public class RemoteFileLister extends AbstractFileLister { @Override protected String getFileName(RemoteFile file) { - return null; + return CommonUtils.getFileNameFromPath(file.getRemotePath()); } @Override - protected DirectoryLoader getDirectoryLoader() { - return new RemoteDirectoryLoader(); + protected FolderLoader getFolderLoader() { + return new RemoteFolderLoader(); } - private final class RemoteDirectoryLoader implements DirectoryLoader { - + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + public class RemoteFolderLoader implements FolderLoader { + private static final int HTTP_404 = 404; private FolderWrapper directory; @Override public boolean load(SyncedFolder syncedFolder) { final RemoteOperationResult remoteOperationResult = readRemoteFolder(syncedFolder.getRemoteFolder(), client); - + System.out.println("succeed: " + remoteOperationResult.isSuccess()); if (!remoteOperationResult.isSuccess()) { + if (remoteOperationResult.getHttpCode() == HTTP_404) { directory = new FolderWrapper<>(); return true; @@ -87,13 +88,17 @@ public class RemoteFileLister extends AbstractFileLister { return false; } - final int dataSize = remoteOperationResult.getData().size(); - directory = new FolderWrapper<>( (RemoteFile) remoteOperationResult.getData().get(0)); - List subFiles = new ArrayList<>(); + final List datas = remoteOperationResult.getData(); + if (datas == null || datas.size() < 1) return false; + + final int dataSize = datas.size(); + directory = new FolderWrapper<>( (RemoteFile) datas.get(0)); - for (Object o: remoteOperationResult.getData().subList(1, dataSize)) { + final List subFiles = new ArrayList<>(); + for (Object o: datas.subList(1, dataSize)) { subFiles.add((RemoteFile) o); } + directory.addContent(subFiles); return true; } @@ -104,13 +109,13 @@ public class RemoteFileLister extends AbstractFileLister { * @return RemoteOperationResult */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - private RemoteOperationResult readRemoteFolder(String remotePath, OwnCloudClient client) { + public RemoteOperationResult readRemoteFolder(String remotePath, OwnCloudClient client) { final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(remotePath); return operation.execute(client); } @Override - public FolderWrapper getFolder() { + public FolderWrapper getFolderWrapper() { return directory; } } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java new file mode 100644 index 00000000..fe37fd75 --- /dev/null +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -0,0 +1,2 @@ +package foundation.e.drive.contentScanner;public class LocalFileListerTest { +} diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java new file mode 100644 index 00000000..200e3456 --- /dev/null +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java @@ -0,0 +1,365 @@ +/* + * Copyright © ECORP SAS 2022. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ + +package foundation.e.drive.contentScanner; + +import android.accounts.Account; +import android.accounts.AccountManager; +import android.content.Context; +import android.database.sqlite.SQLiteDatabase; +import android.os.Build; + +import androidx.test.core.app.ApplicationProvider; + +import com.owncloud.android.lib.common.OwnCloudClient; +import com.owncloud.android.lib.common.operations.RemoteOperationResult; +import com.owncloud.android.lib.resources.files.model.RemoteFile; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowLog; + +import java.util.ArrayList; + +import foundation.e.drive.TestUtils; +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFileState; +import foundation.e.drive.models.SyncedFolder; +import foundation.e.drive.utils.DavClientProvider; + +/** + * @author vincent Bourgmayer + */ +@RunWith(RobolectricTestRunner.class) +@Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) +public class RemoteFileListerTest { + private static final String DIR_NO_SKIP_EXPECTED = "directory shouldn't have been skipped"; + private static final String DIR_SKIP_EXPECTED = "directory should have been skipped"; + + private RemoteFileLister fileListerUnderTest; + private final Account account; + private final OwnCloudClient ocClient; + private final Context context; + + public RemoteFileListerTest() { + context = ApplicationProvider.getApplicationContext(); + ShadowLog.stream = System.out; + TestUtils.loadServerCredentials(); + TestUtils.prepareValidAccount(AccountManager.get(context)); + account = TestUtils.getValidAccount(); + ocClient = DavClientProvider.getInstance().getClientInstance(account, context); + } + + @Before + public void setUp() { + fileListerUnderTest = new RemoteFileLister(null, ocClient); + SQLiteDatabase.deleteDatabase(context.getDatabasePath(DbHelper.DATABASE_NAME)); + } + + @Test + public void skipSyncedFolder_notMedia_notHidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, false, false); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("Not media, not hidden, not remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_notMedia_notHidden_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, false, true); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertFalse("Not media, not hidden, remotly scannable " + DIR_NO_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_notMedia_hidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, true, false); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("Not media, hidden, not remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_notMedia_hidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, true, true); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertFalse("Not media, hidden, remotly scannable " + DIR_NO_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_media_notHidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, false, false); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("media, not hidden, not remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_media_notHidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, false, true); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertFalse("media, not hidden, remotly scannable " + DIR_NO_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_media_hidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, true, false); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("media, hidden, not remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + @Test + public void skipSyncedFolder_media_hidden_scannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, true, true); + final boolean skipFolder = fileListerUnderTest.skipSyncedFolder(folder); + Assert.assertTrue("media, hidden, remotly scannable " + DIR_SKIP_EXPECTED, skipFolder); + } + + private SyncedFolder buildSyncedFolderForSkipSyncedFolderTest(boolean isMedia, boolean isHidden, boolean isRemoteScannable) { + final String dirName = isHidden ? ".myFolder/" : "myFolder/"; + return new SyncedFolder("any", "/local/path/" + dirName, "/remote/path/" + dirName, true, isRemoteScannable, true, isMedia); + } + + + @Test + public void skipFile_hidden_shouldReturnTrue() { + final String hiddenRemoteFilePath = "remote/path/.tutu.png"; + final RemoteFile file = Mockito.mock(RemoteFile.class); + Mockito.when(file.getRemotePath()).thenReturn(hiddenRemoteFilePath); + + final boolean skipFile = fileListerUnderTest.skipFile(file); + Assert.assertTrue("Hidden remoteFile (" + hiddenRemoteFilePath + ") should have been skipped", skipFile); + } + + @Test + public void skipFile_notHidden_shouldReturnFalse() { + final String notHiddenRemoteFilePath = "remote/path/tutu.png"; + final RemoteFile file = Mockito.mock(RemoteFile.class); + Mockito.when(file.getRemotePath()).thenReturn(notHiddenRemoteFilePath); + + final boolean skipFile = fileListerUnderTest.skipFile(file); + Assert.assertFalse("Not hidden remoteFile (" + notHiddenRemoteFilePath + ") should not have been skipped", skipFile); + } + + @Test + public void skipFile_emptyPath_shouldReturnTrue() { + final String emptyPath = ""; + final RemoteFile file = Mockito.mock(RemoteFile.class); + Mockito.when(file.getRemotePath()).thenReturn(emptyPath); + + final boolean skipFile = fileListerUnderTest.skipFile(file); + Assert.assertTrue("RemoteFile with empty path (" + emptyPath + ") should have been skipped", skipFile); + } + + @Test + @Ignore + public void skipFile_invalidPath_shouldReturnTrue() { + final String invalidPath = "/invalid/path/"; + /*in this case: No file, just a directory + the current implementation of CommonUtils.getFileName(String path) + will return "path" while empty string should be expected. + todo: fix CommonUtils.getFileName(String path)*/ + final RemoteFile file = Mockito.mock(RemoteFile.class); + Mockito.when(file.getRemotePath()).thenReturn(invalidPath); + + final boolean skipFile = fileListerUnderTest.skipFile(file); + Assert.assertTrue("RemoteFile with empty path (" + invalidPath + ") should have been skipped", skipFile); + } + + @Test + public void skipDirectory_etagChanged_noUnsyncedContent_shouldReturnFalse() { + final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); + Mockito.when(remoteFile.getEtag()).thenReturn("6666bbbb"); + + final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + syncedFolder.setId(120); + syncedFolder.setLastEtag("5555aaaa"); + + final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); + + Assert.assertFalse("Remote directory with new etag should not have been skipped but it had", skip); + } + + @Test + public void skipDirectory_etagSame_noUnsyncedContent_shouldReturnTrue() { + final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); + Mockito.when(remoteFile.getEtag()).thenReturn("5555aaaa"); + + final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + syncedFolder.setId(12); + syncedFolder.setLastEtag("5555aaaa"); + + final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); + + Assert.assertTrue("Remote directory with no new etag should have been skipped but had not", skip); + } + + @Test + public void skipDirectory_etagSame_contentToSync_shouldReturnFalse() { + final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); + Mockito.when(remoteFile.getEtag()).thenReturn("5555aaaa"); + + final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + syncedFolder.setId(12); + syncedFolder.setLastEtag("5555aaaa"); + + final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "local/tutu.jpg", "remote/tutu.jpg", "7777", 0L, 12, true, 3); + DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); + + final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); + + Assert.assertFalse("Remote directory with no new etag but there is content to sync from DB: should not have been skipped but it had", skip); + } + + @Test + public void skipDirectory_etagChanged_contentToSync_shouldReturnFalse() { + final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); + Mockito.when(remoteFile.getEtag()).thenReturn("6666bbbb"); + + final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + syncedFolder.setId(12); + syncedFolder.setLastEtag("5555aaaa"); + + final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "local/tutu.jpg", "remote/tutu.jpg", "7777", 0L, 12, true, 3); + DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); + + final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); + + Assert.assertFalse("Remote directory with new etag and there is content to sync from DB: should not have been skipped but it had", skip); + } + //Test about inner class: DirectoryLoader + + @Test + public void loadDirectory_serverResponse404_shouldReturnTrue() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(404); + Mockito.when(fakeResult.isSuccess()).thenReturn(false); + + final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); + + Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = mockLoaderSpied.load(folder); + Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); + } + + @Test + public void loadDirectory_serverResponse405_shouldReturnFalse() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(405); + Mockito.when(fakeResult.isSuccess()).thenReturn(false); + + final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); + + Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = mockLoaderSpied.load(folder); + Assert.assertFalse("Loading remote folder resulting in 405 should return false", loadSuccess); + } + + @Test + public void loadDirectory_successAndErrorCode_shouldReturnTrue() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(405); + Mockito.when(fakeResult.isSuccess()).thenReturn(true); + + final ArrayList fakeData = new ArrayList<>(); + final RemoteFile mockRemoteFile = Mockito.mock(RemoteFile.class); + fakeData.add(mockRemoteFile); + Mockito.when(fakeResult.getData()).thenReturn(fakeData); + + final RemoteFileLister.RemoteFolderLoader loaderUnderTest = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(loaderUnderTest); + + Mockito.when(spiedLoaderUnderTest.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = spiedLoaderUnderTest.load(folder); + Assert.assertTrue("Loading remote folder with success despite error code (405) should return true", loadSuccess); + Assert.assertEquals("Loaded Folder is not the one expected despite loading success", mockRemoteFile, spiedLoaderUnderTest.getFolderWrapper().getFolder()); + } + + @Test + public void loadDirectory_successAndRemoteContent_shouldReturnTrue() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(405); + Mockito.when(fakeResult.isSuccess()).thenReturn(true); + + final ArrayList fakeData = new ArrayList<>(); + final RemoteFile mockDirRemoteFile = Mockito.mock(RemoteFile.class); + fakeData.add(mockDirRemoteFile); + final RemoteFile mockContentRemoteFile1 = Mockito.mock(RemoteFile.class); + final RemoteFile mockContentRemoteFile2 = Mockito.mock(RemoteFile.class); + fakeData.add(mockContentRemoteFile1); + fakeData.add(mockContentRemoteFile2); + Mockito.when(fakeResult.getData()).thenReturn(fakeData); + + final RemoteFileLister.RemoteFolderLoader loaderUnderTest = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(loaderUnderTest); + + Mockito.when(spiedLoaderUnderTest.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = spiedLoaderUnderTest.load(folder); + Assert.assertTrue("Loading remote folder with success despite error code (405) should return true", loadSuccess); + Assert.assertEquals("Loaded Folder is not the one expected despite loading success", mockDirRemoteFile, spiedLoaderUnderTest.getFolderWrapper().getFolder()); + Assert.assertEquals("mockDirRemoteFile should have been the directory loader but is not", 2, spiedLoaderUnderTest.getFolderWrapper().getContent().size()); + Assert.assertTrue("mockContentRemoteFile1 should have been in the result of loading but is not", spiedLoaderUnderTest.getFolderWrapper().getContent().contains(mockContentRemoteFile1)); + Assert.assertTrue("mockContentRemoteFile2 should have been in the result of loading but is not", spiedLoaderUnderTest.getFolderWrapper().getContent().contains(mockContentRemoteFile2)); + } + + + + + @Test + public void loadDirectory_successButNoData_shouldReturnFalse() { + final String remotePath = "remote/path/myFolder/"; + final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); + final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + + final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); + Mockito.when(fakeResult.getHttpCode()).thenReturn(204); + Mockito.when(fakeResult.isSuccess()).thenReturn(true); + + final ArrayList fakeData = new ArrayList<>(); + Mockito.when(fakeResult.getData()).thenReturn(fakeData); + + final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + + RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); + + Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + + final boolean loadSuccess = mockLoaderSpied.load(folder); + Assert.assertFalse("Loading remote folder resulting in 204 but without any RemoteFile should return false", loadSuccess); + } + +} -- GitLab From c63ce85113e0a45f5e091272439f36da7e490baa Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Fri, 4 Nov 2022 12:13:57 +0100 Subject: [PATCH 08/16] Add unit test for LocalFileLister.java - Rewrite skipSyncedFolder(SynceFolder folder) method of LocalFileLister - Add test cases for skipSyncedFolder(SyncedFolder) method --- .../drive/contentScanner/LocalFileLister.java | 9 +- .../contentScanner/LocalFileListerTest.java | 119 +++++++++++++++++- 2 files changed, 121 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index a37fcf00..e85ef7dc 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -25,6 +25,7 @@ import timber.log.Timber; * @author vincent Bourgmayer */ public class LocalFileLister extends AbstractFileLister { + public LocalFileLister(List directories) { super(directories); } @@ -37,12 +38,8 @@ public class LocalFileLister extends AbstractFileLister { @Override protected boolean skipSyncedFolder(SyncedFolder syncedFolder) { - final String fileName = CommonUtils.getFileNameFromPath(syncedFolder.getLocalFolder()); - if (fileName == null || fileName.startsWith(".") || !syncedFolder.isScanLocal()) { - return true; - } - - return false; + final File folder = new File(syncedFolder.getLocalFolder()); + return (syncedFolder.isMediaType() && folder.isHidden()) || !syncedFolder.isScanLocal(); } @Override diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index fe37fd75..49d49d0a 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -1,2 +1,119 @@ -package foundation.e.drive.contentScanner;public class LocalFileListerTest { +/* + * Copyright © ECORP SAS 2022. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ + +package foundation.e.drive.contentScanner; + +import android.accounts.Account; +import android.accounts.AccountManager; +import android.content.Context; +import android.database.sqlite.SQLiteDatabase; +import android.os.Build; + +import androidx.test.core.app.ApplicationProvider; + +import com.owncloud.android.lib.common.OwnCloudClient; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowLog; + + +import java.io.File; + +import foundation.e.drive.TestUtils; +import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFolder; +import foundation.e.drive.utils.DavClientProvider; + +/** + * @author vincent Bourgmayer + */ +@RunWith(RobolectricTestRunner.class) +@Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) +public class LocalFileListerTest { + + private LocalFileLister fileListerUnderTest; + private final Account account; + private final OwnCloudClient ocClient; + private final Context context; + + public LocalFileListerTest() { + context = ApplicationProvider.getApplicationContext(); + ShadowLog.stream = System.out; + TestUtils.loadServerCredentials(); + TestUtils.prepareValidAccount(AccountManager.get(context)); + account = TestUtils.getValidAccount(); + ocClient = DavClientProvider.getInstance().getClientInstance(account, context); + } + + @Before + public void setUp() { + fileListerUnderTest = new LocalFileLister(null); + SQLiteDatabase.deleteDatabase(context.getDatabasePath(DbHelper.DATABASE_NAME)); + } + + @Test + public void skipSyncedFolder_notMedia_notHidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, false, false); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_notMedia_notHidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, false, true); + Assert.assertFalse(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_notMedia_hidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, true, false); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_notMedia_hidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(false, true, true); + Assert.assertFalse(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_media_notHidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, false, false); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_media_notHidden_scannable_shouldReturnFalse() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, false, true); + Assert.assertFalse("folder shouldn't be skip but it had", fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_media_Hidden_notScannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, true, false); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + @Test + public void skipSyncedFolder_media_Hidden_scannable_shouldReturnTrue() { + final SyncedFolder folder = buildSyncedFolderForSkipSyncedFolderTest(true, true, true); + Assert.assertTrue(fileListerUnderTest.skipSyncedFolder(folder)); + } + + private SyncedFolder buildSyncedFolderForSkipSyncedFolderTest(boolean isMedia, boolean isHidden, boolean isLocalScannable) { + final String dirName = isHidden ? ".myFolder/" : "myFolder/"; + return new SyncedFolder("any", "/local/path/" + dirName, "/remote/path/" + dirName, isLocalScannable, true, true, isMedia); + } + + + } -- GitLab From a993debab01c3458a8f5a88767a2409f302bddfc Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 7 Nov 2022 15:20:02 +0100 Subject: [PATCH 09/16] - Check syncedFolder not null in AbstractFileFilter - Add NonNull annotation in FileLister classes - move Abstract method to top of AbstractFileLister - Add missing if null or not null --- .../contentScanner/AbstractFileLister.java | 55 ++++++++++--------- .../e/drive/contentScanner/FolderWrapper.java | 19 ++++++- .../drive/contentScanner/LocalFileLister.java | 28 ++++++---- .../contentScanner/RemoteFileLister.java | 31 +++++++---- 4 files changed, 82 insertions(+), 51 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 34dfe74b..71745489 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -9,6 +9,8 @@ package foundation.e.drive.contentScanner; import android.content.Context; +import androidx.annotation.NonNull; + import java.util.ArrayList; import java.util.List; import java.util.ListIterator; @@ -25,14 +27,29 @@ import timber.log.Timber; */ public abstract class AbstractFileLister { + protected abstract boolean skipSyncedFolder(@NonNull SyncedFolder syncedFolder); + protected abstract boolean skipFile(@NonNull T file); + + /** + * If the given folder has not changed, compared to database's data, skip it + * @param currentDirectory + * @param syncedFolder + * @param context + * @return + */ + protected abstract boolean skipDirectory(@NonNull T currentDirectory, @NonNull SyncedFolder syncedFolder, @NonNull Context context); + protected abstract boolean isDirectory(@NonNull T file); + protected abstract String getFileName(@NonNull T file); + protected abstract FolderLoader getFolderLoader(); + /** * This allows to use RemoteOperationResult for RemoteFileLister - * todo: some optimization may be done with FolderWrapper + * todo: some optimization could be done with FolderWrapper * @param RemoteFile or File */ /* package */ interface FolderLoader { /* package */ FolderWrapper getFolderWrapper(); - /* package */ boolean load(SyncedFolder folder); + /* package */ boolean load(@NonNull SyncedFolder folder); } protected final List folders; @@ -42,7 +59,7 @@ public abstract class AbstractFileLister { * constructor to be call with super * @param folders List of SyncedFolder to read */ - protected AbstractFileLister(List folders) { + protected AbstractFileLister(@NonNull List folders) { this.folders = folders; this.contentToScan = new ArrayList<>(); } @@ -52,12 +69,12 @@ public abstract class AbstractFileLister { * @param context * @return */ - public boolean listContentToScan(Context context) { - //For each SyncedFolder + public boolean listContentToScan(@NonNull Context context) { final ListIterator iterator = folders.listIterator() ; boolean isSomeContentToSync = false; while (iterator.hasNext()) { final SyncedFolder syncedFolder = iterator.next(); + if (syncedFolder == null) continue; Timber.v("SyncedFolder : %s, %s, %s, %s, %s", syncedFolder.getLibelle(), syncedFolder.getLocalFolder(), syncedFolder.getLastModified(), syncedFolder.isScanLocal(), syncedFolder.getId()); isSomeContentToSync = isSomeContentToSync || hasDirectoryContentToScan(syncedFolder, iterator, context); } @@ -68,7 +85,6 @@ public abstract class AbstractFileLister { return isSomeContentToSync; } - /** * Detailed process for reading a single folder * @param syncedFolder the SyncedFolder to check @@ -76,8 +92,8 @@ public abstract class AbstractFileLister { * @param context context used to call DB * @return true the directory has content to synchronized false otherwise */ - private boolean hasDirectoryContentToScan( SyncedFolder syncedFolder, ListIterator iterator, Context context) { - // I check if I can skip it; If then then go to next + private boolean hasDirectoryContentToScan(@NonNull SyncedFolder syncedFolder, @NonNull ListIterator iterator, @NonNull Context context) { + final FolderLoader dirLoader = getFolderLoader(); if (skipSyncedFolder(syncedFolder) || !isSyncedFolderInDb(syncedFolder, context) @@ -88,23 +104,19 @@ public abstract class AbstractFileLister { final FolderWrapper currentDirectory = dirLoader.getFolderWrapper(); - // If directory is missing (not exist or http 404) if (currentDirectory.isMissing()) { return true; } - //--> I no content has changed, skip if (skipDirectory(currentDirectory.getFolder(), syncedFolder, context)) { iterator.remove(); return false; } - //else I have to check if some content has changed inside - final List dirContent = currentDirectory.getContent(); - //todo: look where to put in subclasses : syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); - if (!dirContent.isEmpty()) { + final List dirContent = currentDirectory.getContent(); + if (dirContent != null && !dirContent.isEmpty()) { contentToScan.addAll(sortContent(iterator, syncedFolder, dirContent)); } return true; @@ -118,7 +130,7 @@ public abstract class AbstractFileLister { * @param context context to contact database * @return false if not persisted in db */ - private boolean isSyncedFolderInDb(SyncedFolder syncedFolder, Context context) { + private boolean isSyncedFolderInDb(@NonNull SyncedFolder syncedFolder, @NonNull Context context) { if (syncedFolder.getId() > 0) return true; final int syncedFolder_id = (int) DbHelper.insertSyncedFolder(syncedFolder, context); //It will return -1 if there is an error, like an already existing folder with same value @@ -140,11 +152,13 @@ public abstract class AbstractFileLister { * @param dirContent Content to sort * @return List of subfiles to scan or empty list if nothing */ - private List sortContent(ListIterator iterator, SyncedFolder syncedFolder, List dirContent) { + private List sortContent(@NonNull ListIterator iterator, @NonNull SyncedFolder syncedFolder, List dirContent) { final List result = new ArrayList<>(); if (dirContent == null) return result; for (T subFile : dirContent) { + if (subFile == null) continue; + final String fileName = getFileName(subFile); if (isDirectory(subFile)) { final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName, 0L, "");//Need to set lastModified to 0 to handle it on next iteration @@ -180,13 +194,4 @@ public abstract class AbstractFileLister { } return result; } - - - - protected abstract boolean skipSyncedFolder(SyncedFolder syncedFolder); - protected abstract boolean skipFile(T file); - protected abstract boolean skipDirectory(T currentDirectory, SyncedFolder syncedFolder, Context context); - protected abstract boolean isDirectory(T file); - protected abstract String getFileName(T file); - protected abstract FolderLoader getFolderLoader(); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java index ca273584..6fb4d307 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/FolderWrapper.java @@ -7,6 +7,8 @@ */ package foundation.e.drive.contentScanner; +import androidx.annotation.NonNull; + import java.util.ArrayList; import java.util.List; @@ -25,7 +27,7 @@ public class FolderWrapper { private final T folder; private final List content; - protected FolderWrapper(T folder) { + protected FolderWrapper(@NonNull T folder) { content = new ArrayList<>(); this.folder = folder; missing = false; @@ -37,18 +39,31 @@ public class FolderWrapper { content = null; } + /** + * Get the folder (should be RemoteFile or File instance ) represented by this abstraction + * @return null if it is missing + */ public T getFolder() { return folder; } + /** + * Get subfiles (including sub folder) of the current folder + * @return Null if the directory is missing + */ public List getContent() { return content; } - public void addContent(List newContent) { + public void addContent(@NonNull List newContent) { content.addAll(newContent); } + /** + * Should return true if local File for directory doesn't exists, or if ReadFolderRemoteOperation return 404 + * for a remote file + * @return + */ public boolean isMissing() { return missing; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index e85ef7dc..571eae86 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -9,6 +9,8 @@ package foundation.e.drive.contentScanner; import android.content.Context; +import androidx.annotation.NonNull; + import java.io.File; import java.io.FileFilter; import java.util.Arrays; @@ -17,7 +19,6 @@ import java.util.List; import foundation.e.drive.database.DbHelper; import foundation.e.drive.fileFilters.FileFilterFactory; import foundation.e.drive.models.SyncedFolder; -import foundation.e.drive.utils.CommonUtils; import timber.log.Timber; /** @@ -26,34 +27,34 @@ import timber.log.Timber; */ public class LocalFileLister extends AbstractFileLister { - public LocalFileLister(List directories) { + public LocalFileLister(@NonNull List directories) { super(directories); } @Override - protected boolean skipDirectory(File currentDirectory, SyncedFolder syncedFolder, Context context) { + protected boolean skipDirectory(@NonNull File currentDirectory, @NonNull SyncedFolder syncedFolder, @NonNull Context context) { return currentDirectory.lastModified() == syncedFolder.getLastModified() && !DbHelper.syncedFolderHasContentToUpload(syncedFolder.getId(), context); } @Override - protected boolean skipSyncedFolder(SyncedFolder syncedFolder) { + protected boolean skipSyncedFolder(@NonNull SyncedFolder syncedFolder) { final File folder = new File(syncedFolder.getLocalFolder()); return (syncedFolder.isMediaType() && folder.isHidden()) || !syncedFolder.isScanLocal(); } @Override - protected boolean skipFile(File file) { + protected boolean skipFile(@NonNull File file) { return file.isHidden(); } @Override - protected boolean isDirectory(File file) { + protected boolean isDirectory(@NonNull File file) { return file.isDirectory(); } @Override - protected String getFileName(File file) { + protected String getFileName(@NonNull File file) { return file.getName(); } @@ -71,18 +72,21 @@ public class LocalFileLister extends AbstractFileLister { } @Override - public boolean load(SyncedFolder syncedFolder) { + public boolean load(@NonNull SyncedFolder syncedFolder) { final File dir = new File(syncedFolder.getLocalFolder()); - Timber.v("Local Folder (last modified / exists): %s, %s", dir.lastModified(),dir.exists()); + Timber.v("Local Folder (last modified / exists): %s, %s", dir.lastModified(), dir.exists()); if (!dir.exists()) { folder = new FolderWrapper(); //missing Folder Wrapper - } - else { + } else { folder = new FolderWrapper(dir); final String category = syncedFolder.isMediaType() ? "media" : syncedFolder.getLibelle(); + final FileFilter filter = FileFilterFactory.getFileFilter(category); - folder.addContent(Arrays.asList(dir.listFiles(filter))); + final File[] subFiles = dir.listFiles(filter); + if (subFiles != null) { + folder.addContent(Arrays.asList(subFiles)); + } } return true; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index eec94bf9..1e0b6e09 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -9,6 +9,7 @@ package foundation.e.drive.contentScanner; import android.content.Context; +import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import com.owncloud.android.lib.common.OwnCloudClient; @@ -31,37 +32,37 @@ import foundation.e.drive.utils.CommonUtils; public class RemoteFileLister extends AbstractFileLister { private OwnCloudClient client; - public RemoteFileLister(List directories, OwnCloudClient client) { + public RemoteFileLister(@NonNull List directories, @NonNull OwnCloudClient client) { super(directories); this.client = client; } @Override - protected boolean skipSyncedFolder(SyncedFolder syncedFolder) { + protected boolean skipSyncedFolder(@NonNull SyncedFolder syncedFolder) { return (syncedFolder.isMediaType() && CommonUtils.getFileNameFromPath(syncedFolder.getRemoteFolder()).startsWith(".")) || !syncedFolder.isScanRemote(); } @Override - protected boolean skipFile(RemoteFile file) { + protected boolean skipFile(@NonNull RemoteFile file) { final String fileName = CommonUtils.getFileNameFromPath(file.getRemotePath()); return fileName == null || fileName.isEmpty() || fileName.startsWith("."); } @Override - protected boolean skipDirectory(RemoteFile directory, SyncedFolder syncedFolder, Context context) { + protected boolean skipDirectory(@NonNull RemoteFile directory, @NonNull SyncedFolder syncedFolder, @NonNull Context context) { return directory.getEtag().equals(syncedFolder.getLastEtag()) && !DbHelper.syncedFolderHasContentToDownload(syncedFolder.getId(), context); } @Override - protected boolean isDirectory(RemoteFile file) { + protected boolean isDirectory(@NonNull RemoteFile file) { return file.getMimeType().equals("DIR"); } @Override - protected String getFileName(RemoteFile file) { + protected String getFileName(@NonNull RemoteFile file) { return CommonUtils.getFileNameFromPath(file.getRemotePath()); } @@ -76,11 +77,11 @@ public class RemoteFileLister extends AbstractFileLister { private FolderWrapper directory; @Override - public boolean load(SyncedFolder syncedFolder) { + public boolean load(@NonNull SyncedFolder syncedFolder) { + if (syncedFolder.getRemoteFolder() == null) return false; final RemoteOperationResult remoteOperationResult = readRemoteFolder(syncedFolder.getRemoteFolder(), client); - System.out.println("succeed: " + remoteOperationResult.isSuccess()); - if (!remoteOperationResult.isSuccess()) { + if (!remoteOperationResult.isSuccess()) { if (remoteOperationResult.getHttpCode() == HTTP_404) { directory = new FolderWrapper<>(); return true; @@ -92,11 +93,17 @@ public class RemoteFileLister extends AbstractFileLister { if (datas == null || datas.size() < 1) return false; final int dataSize = datas.size(); - directory = new FolderWrapper<>( (RemoteFile) datas.get(0)); + final RemoteFile remoteDir = (RemoteFile) datas.get(0); + if (remoteDir == null) return false; + + directory = new FolderWrapper<>(remoteDir); final List subFiles = new ArrayList<>(); for (Object o: datas.subList(1, dataSize)) { - subFiles.add((RemoteFile) o); + final RemoteFile subFile = (RemoteFile) o; + if (subFile != null) { + subFiles.add(subFile); + } } directory.addContent(subFiles); return true; @@ -109,7 +116,7 @@ public class RemoteFileLister extends AbstractFileLister { * @return RemoteOperationResult */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - public RemoteOperationResult readRemoteFolder(String remotePath, OwnCloudClient client) { + public RemoteOperationResult readRemoteFolder(@NonNull String remotePath, @NonNull OwnCloudClient client) { final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(remotePath); return operation.execute(client); } -- GitLab From ad63ebf55d8bed40c0428ca726d095816f2354ad Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Mon, 7 Nov 2022 17:31:46 +0100 Subject: [PATCH 10/16] Add test cases for localFileLister.skipDirectory() Add test cases fpr LocalFolderLoader.loader() Update LocalFileLister to make LocalFolderLoader available for test --- .../drive/contentScanner/LocalFileLister.java | 15 ++- .../contentScanner/LocalFileListerTest.java | 122 ++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 571eae86..7e3caa0a 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -10,6 +10,7 @@ package foundation.e.drive.contentScanner; import android.content.Context; import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; import java.io.File; import java.io.FileFilter; @@ -60,10 +61,11 @@ public class LocalFileLister extends AbstractFileLister { @Override protected FolderLoader getFolderLoader() { - return new LocalDirLoader(); + return new LocalFolderLoader(); } - private final class LocalDirLoader implements FolderLoader { + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + public class LocalFolderLoader implements FolderLoader { private FolderWrapper folder; @Override @@ -71,6 +73,15 @@ public class LocalFileLister extends AbstractFileLister { return folder; } + + /** + * "Load" a directory to check if it exist, and list its content + * It is expected to always return true. Its remote equivalent (in RemoteFileLister) + * Might return false because of network, & RemoteFile classes. But that's not the case + * here. + * @param syncedFolder + * @return always true because it's about "File" instance. + */ @Override public boolean load(@NonNull SyncedFolder syncedFolder) { final File dir = new File(syncedFolder.getLocalFolder()); diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index 49d49d0a..ed3cefc0 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -17,20 +17,24 @@ import android.os.Build; import androidx.test.core.app.ApplicationProvider; import com.owncloud.android.lib.common.OwnCloudClient; +import com.owncloud.android.lib.common.operations.RemoteOperationResult; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLog; import java.io.File; +import java.io.IOException; import foundation.e.drive.TestUtils; import foundation.e.drive.database.DbHelper; +import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; import foundation.e.drive.utils.DavClientProvider; @@ -114,6 +118,124 @@ public class LocalFileListerTest { return new SyncedFolder("any", "/local/path/" + dirName, "/remote/path/" + dirName, isLocalScannable, true, true, isMedia); } + @Test + public void skipDirectory_lastModifiedSame_noUnsyncedContent_shouldReturnTrue() { + final long lastModified = 123456L; + final File directory = Mockito.spy(new File("/local/path/test/")); + Mockito.when(directory.lastModified()).thenReturn(lastModified); + + final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + syncedFolder.setId(12); + syncedFolder.setLastModified(lastModified); + + final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); + Assert.assertTrue("directory without new last modified and no content waiting for sync has not been skip", skip); + } + + @Test + public void skipDirectory_lastModifiedSame_unsyncedContent_shouldReturnFalse() { + final long lastModified = 123456L; + final File directory = Mockito.spy(new File("/local/path/test/")); + Mockito.when(directory.lastModified()).thenReturn(lastModified); + + final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "/local/path/test/tutu.jpg", "/remote/path/test/tutu.jpg", "", 0L, 12, true, 3); + DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); + + final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/" , "/remote/path/test/", true, true, true, true); + syncedFolder.setId(12); + syncedFolder.setLastModified(lastModified); + + final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); + Assert.assertFalse("directory without new last modified but content waiting for sync has been skip", skip); + } + + @Test + public void skipDirectory_lastModifiedChanged_noUnsyncedContent_shouldReturnFalse() { + final File directory = Mockito.spy(new File("/local/path/test/")); + Mockito.when(directory.lastModified()).thenReturn(654321L); + + final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + syncedFolder.setId(12); + syncedFolder.setLastModified(123456L); + + final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); + Assert.assertFalse("directory with new last modified and no content waiting for sync has been skip", skip); + } + + @Test + public void skipDirectory_lastModifiedChanged_unsyncedContent_shouldReturnFalse() { + final File directory = Mockito.spy(new File("/local/path/test/")); + Mockito.when(directory.lastModified()).thenReturn(654321L); + + final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + syncedFolder.setId(12); + syncedFolder.setLastModified(123456L); + + final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "/local/path/test/tutu.jpg", "/remote/path/test/tutu.jpg", "", 0L, 12, true, 3); + DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); + + final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); + Assert.assertFalse("directory with new last modified and content waiting for sync has been skip", skip); + } + + //Test about inner class: DirectoryLoader + + + @Test + public void loadDirectory_dirNotExist_shouldReturnTrue() { + final SyncedFolder folder = new SyncedFolder("any", "/local/path/myFolder", "/remote/path/myFolder", true); + + final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + + final boolean loadSuccess = folderLoader.load(folder); + Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); + Assert.assertTrue(folderLoader.getFolderWrapper().isMissing()); + } + @Test + public void loadDirectory_dirExist_noContent_shouldReturnTrue() { + final SyncedFolder folder = new SyncedFolder("any", "../local/path/myFolder/", "/remote/path/myFolder/", true); + final File file = new File(folder.getLocalFolder()); + final boolean dirCreationSucceed = file.mkdirs(); + + Assert.assertTrue("cannot create test folder: " + file.getAbsolutePath(), dirCreationSucceed); + + final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + + final boolean loadSuccess = folderLoader.load(folder); + final FolderWrapper wrapper = folderLoader.getFolderWrapper(); + Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); + Assert.assertFalse("dir shouldExist!", wrapper.isMissing()); + Assert.assertTrue("List of folderWrapper's content expected to be empty but contains: "+wrapper.getContent().size(), wrapper.getContent().isEmpty()); + //@todo beware, in case of previous assertion failure, the below code won't be executed + Assert.assertTrue("Test file has not been successfully removed: " + file.getAbsolutePath(), file.delete()); + } + @Test + public void loadDirectory_dirExist_withSubContent_shouldReturnTrue() { + final SyncedFolder folder = new SyncedFolder("any", "../local/path/myFolder/", "/remote/path/myFolder/", true); + final File folderFile = new File(folder.getLocalFolder()); + final boolean dirCreationSucceed = folderFile.mkdirs(); + + final File subContent = new File(folderFile, "toto.txt"); + try { + subContent.createNewFile(); + } catch (IOException exception) { + exception.printStackTrace(); + Assert.fail("IO Exception: Can't create new file as content of test folder"); + } + + Assert.assertTrue("cannot create test folder: " + folderFile.getAbsolutePath(), dirCreationSucceed); + + final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + + final boolean loadSuccess = folderLoader.load(folder); + final FolderWrapper wrapper = folderLoader.getFolderWrapper(); + Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); + Assert.assertFalse("dir shouldExist!", wrapper.isMissing()); + Assert.assertEquals("List of folderWrapper's content size expected to one but was: "+wrapper.getContent().size(), 1, wrapper.getContent().size()); + //@todo beware, in case of previous assertion failure, the below code won't be executed + Assert.assertTrue("SubFile has not been removed", subContent.delete()); + Assert.assertTrue("Test folder has not been removed: " + folderFile.getAbsolutePath(), folderFile.delete()); + } } -- GitLab From d8e85d5c2a3f6f55a56df87d68e07f9ac55fcdfc Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Thu, 10 Nov 2022 12:10:34 +0100 Subject: [PATCH 11/16] * LocalFileListerTest: - add a test for the listContentToScan() - add a @After section to remove fake file generated for test * AbstractFileLister: - fix a boolean assignement that generate errors - Add a new boolean to define if we can dive into a skipped directory to list content of subDirectory * LocalFileLister: - overriden boolean to dive into subContent of a skipped Directory --- .../contentScanner/AbstractFileLister.java | 16 +- .../drive/contentScanner/LocalFileLister.java | 3 +- .../contentScanner/LocalFileListerTest.java | 156 ++++++++++++++++++ 3 files changed, 171 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 71745489..7168b1fd 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -54,6 +54,7 @@ public abstract class AbstractFileLister { protected final List folders; private final List contentToScan; + protected boolean diveIntoSkippedDir = false; //to be overriden only /** * constructor to be call with super @@ -76,7 +77,7 @@ public abstract class AbstractFileLister { final SyncedFolder syncedFolder = iterator.next(); if (syncedFolder == null) continue; Timber.v("SyncedFolder : %s, %s, %s, %s, %s", syncedFolder.getLibelle(), syncedFolder.getLocalFolder(), syncedFolder.getLastModified(), syncedFolder.isScanLocal(), syncedFolder.getId()); - isSomeContentToSync = isSomeContentToSync || hasDirectoryContentToScan(syncedFolder, iterator, context); + isSomeContentToSync = hasDirectoryContentToScan(syncedFolder, iterator, context) || isSomeContentToSync; } if (isSomeContentToSync) { @@ -110,9 +111,18 @@ public abstract class AbstractFileLister { if (skipDirectory(currentDirectory.getFolder(), syncedFolder, context)) { iterator.remove(); - return false; + syncedFolder.setToSync(false); + /** + * LocalFileLister need to be able to access subFiles even for skipped directory + * because A folder's lastModified value won't change if a subfolder's content is changed, removed or created + * RemoteFileLister is based on eTag, which change even if a subfolder's content is changed + */ + if (!diveIntoSkippedDir) return false; + } else { + syncedFolder.setToSync(true); } + //todo: look where to put in subclasses : syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); final List dirContent = currentDirectory.getContent(); @@ -164,7 +174,7 @@ public abstract class AbstractFileLister { final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName, 0L, "");//Need to set lastModified to 0 to handle it on next iteration iterator.add(subSyncedFolder); iterator.previous(); - } else if (!skipFile(subFile)) { + } else if (syncedFolder.isToSync() && !skipFile(subFile)) { Timber.v("added %s into list of file to scan", fileName); result.add(subFile); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 7e3caa0a..785fafa7 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -30,6 +30,7 @@ public class LocalFileLister extends AbstractFileLister { public LocalFileLister(@NonNull List directories) { super(directories); + super.diveIntoSkippedDir = true; } @Override @@ -56,7 +57,7 @@ public class LocalFileLister extends AbstractFileLister { @Override protected String getFileName(@NonNull File file) { - return file.getName(); + return "/" + file.getName(); } @Override diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index ed3cefc0..951478a6 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -19,6 +19,7 @@ import androidx.test.core.app.ApplicationProvider; import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.operations.RemoteOperationResult; +import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -31,6 +32,8 @@ import org.robolectric.shadows.ShadowLog; import java.io.File; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import foundation.e.drive.TestUtils; import foundation.e.drive.database.DbHelper; @@ -45,6 +48,8 @@ import foundation.e.drive.utils.DavClientProvider; @Config(sdk = Build.VERSION_CODES.O, manifest = Config.NONE) public class LocalFileListerTest { + private static final String TEST_FILE_TREE_ROOT_PATH = "../tmp/"; + private LocalFileLister fileListerUnderTest; private final Account account; private final OwnCloudClient ocClient; @@ -62,7 +67,17 @@ public class LocalFileListerTest { @Before public void setUp() { fileListerUnderTest = new LocalFileLister(null); + } + + + @After + public void tearDown() { SQLiteDatabase.deleteDatabase(context.getDatabasePath(DbHelper.DATABASE_NAME)); + + final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); + if (rootDirectory.exists()) { + deleteDirectory(rootDirectory); + } } @Test @@ -238,4 +253,145 @@ public class LocalFileListerTest { Assert.assertTrue("SubFile has not been removed", subContent.delete()); Assert.assertTrue("Test folder has not been removed: " + folderFile.getAbsolutePath(), folderFile.delete()); } + + + /** + * Run on a more realistic files tree to check the whole behaviour with more than + * one directory: + * + * It should contains : + * - one directory with a subdirectory with some files in both + * - one hidden directory with some files + * - one empty directory + * - one "new" directory with subfiles + * + * with correspond syncedFolder in database: + * - except for one, so it will considered as new + * Something like: + * + * ## File tree + * >Folder 1 (unChanged) + * --> .File 1 (should be ignored) + * --> File 2 (should be ignored) + * --> Folder 2 (unsynced dir) + * ----> File 3 (new file should be added) + * ----> .File 4 (should be ignored) + * >Folder 3 (unChanged) + * -->Folder4 (new dir (so unsynced) + * ----> File 5 (new file should be added) + * -->.Folder5 (skip: hidden dir) + * ----> File 6 (skip because directory is hidden) + * + * + * ## SyncedFolder + * SyncedFolder 1 + * SyncedFolder 2 with empty lastModified + * SyncedFolder 3 + * SyncedFolderRemoved which correspond to a missing directory + */ + @Test + public void bigWholeTest() { + Assert.assertTrue("Preparation, of file Tree and syncedFolder has failed", prepareFakeContent()); + Assert.assertTrue("File listing failed", fileListerUnderTest.listContentToScan(context)); + + final List foldersId = fileListerUnderTest.getSyncedFoldersId(); + Assert.assertEquals("List of folders to scan's id should contains 3 but contained: " + foldersId.size(), 3, foldersId.size()); + Assert.assertTrue(foldersId.contains(2L)); //id of changed folder detected + Assert.assertTrue(foldersId.contains(5L)); //id of new folder detected + Assert.assertTrue(foldersId.contains(4L)); //id of Removed Folder detected + final List contentListed = fileListerUnderTest.getContentToScan(); + Assert.assertEquals("ContentListed should have 2 files but contained: " + contentListed.size(), 2, contentListed.size()); + } + + private boolean prepareFakeContent() { + /** + * Generate folders + */ + final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); + if (!rootDirectory.mkdirs()) return false; + + final File folder1 = new File(rootDirectory, "folder1/"); + if (!folder1.mkdirs()) return false; + final File folder2 = new File(folder1, "folder2/"); + if (!folder2.mkdirs()) return false; + + final File folder3 = new File(rootDirectory, "folder3/"); + if (!folder3.mkdirs()) return false; + final File folder4 = new File(folder3, "folder4/"); + if (!folder4.mkdirs()) return false; + final File folder5 = new File(folder3, ".folder5/"); + if (!folder5.mkdirs()) return false; + + + /** + * Generate files + * /!\ must be done before syncedFolders generation + * otherwise lastModified value of directory will change due to file creation + */ + try { + final File file1 = new File(folder1, ".file1.txt"); + if (!file1.createNewFile()) return false; + + final File file2 = new File(folder1, "file2.txt"); + if (!file2.createNewFile()) return false; + + final File file3 = new File(folder2, "file3.txt"); + if (!file3.createNewFile()) return false; + + final File file4 = new File(folder2, ".file4.txt"); + if (!file4.createNewFile()) return false; + + final File file5 = new File(folder4, "file5.txt"); + if (!file5.createNewFile()) return false; + + final File file6 = new File(folder5, "file6.txt"); + if (!file6.createNewFile()) return false; + } catch (IOException exception) { + exception.printStackTrace(); + return false; + } + + + /** + * Generate SyncedFolders + */ + final SyncedFolder syncedFolder1 = new SyncedFolder("any", folder1.getAbsolutePath(), "/remote/path/myFolder1/", true); + syncedFolder1.setLastModified(folder1.lastModified()); + syncedFolder1.setId((int) DbHelper.insertSyncedFolder(syncedFolder1, context)); + final SyncedFolder syncedFolder2 = new SyncedFolder("any", folder2.getAbsolutePath(), "/remote/path/myFolder2/", true); + syncedFolder2.setLastModified(0L); + syncedFolder2.setId((int) DbHelper.insertSyncedFolder(syncedFolder2, context)); + final SyncedFolder syncedFolder3 = new SyncedFolder("any", folder3.getAbsolutePath(), "/remote/path/myFolder3/", true); + syncedFolder3.setLastModified(folder3.lastModified()); + syncedFolder3.setId((int) DbHelper.insertSyncedFolder(syncedFolder3, context)); + final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", rootDirectory.getAbsolutePath()+"lambda/", "/remote/lambda", true); + syncedFolderRemoved.setLastModified(987654321L); + syncedFolderRemoved.setLastEtag("123456789"); + syncedFolderRemoved.setId((int) DbHelper.insertSyncedFolder(syncedFolderRemoved, context)); + + final ArrayList syncedFolders = new ArrayList<>(); + syncedFolders.add(syncedFolder1); + syncedFolders.add(syncedFolder2); + syncedFolders.add(syncedFolder3); + syncedFolders.add(syncedFolderRemoved); + fileListerUnderTest = new LocalFileLister(syncedFolders); + + return true; + } + + + + + private void deleteDirectory(File file) + { + for (File subfile : file.listFiles()) { + + if (subfile.isDirectory()) { + deleteDirectory(subfile); + } + subfile.delete(); + } + file.delete(); + } + } -- GitLab From 70b0be3465b9a66c20ae348d38aa7e60b15ef4b1 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 15 Nov 2022 12:23:59 +0100 Subject: [PATCH 12/16] move method to delete files for test into TestUtils --- .../java/foundation/e/drive/TestUtils.java | 18 ++++++++++++++++++ .../contentScanner/LocalFileListerTest.java | 16 ++-------------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/app/src/test/java/foundation/e/drive/TestUtils.java b/app/src/test/java/foundation/e/drive/TestUtils.java index 1da09d5d..19861db8 100644 --- a/app/src/test/java/foundation/e/drive/TestUtils.java +++ b/app/src/test/java/foundation/e/drive/TestUtils.java @@ -155,6 +155,24 @@ public abstract class TestUtils { } + /** + * Delete a local directory with all its content. + * Adapted from https://www.geeksforgeeks.org/java-program-to-delete-a-directory/ + * @param file + */ + public static void deleteDirectory(File file) + { + for (File subfile : file.listFiles()) { + + if (subfile.isDirectory()) { + deleteDirectory(subfile); + } + subfile.delete(); + } + file.delete(); + } + + public static void initializeWorkmanager(Context context) { final Configuration config = new Configuration.Builder() .setMinimumLoggingLevel(Log.DEBUG) diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index 951478a6..d3211065 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -17,7 +17,6 @@ import android.os.Build; import androidx.test.core.app.ApplicationProvider; import com.owncloud.android.lib.common.OwnCloudClient; -import com.owncloud.android.lib.common.operations.RemoteOperationResult; import org.junit.After; import org.junit.Assert; @@ -51,7 +50,6 @@ public class LocalFileListerTest { private static final String TEST_FILE_TREE_ROOT_PATH = "../tmp/"; private LocalFileLister fileListerUnderTest; - private final Account account; private final OwnCloudClient ocClient; private final Context context; @@ -60,7 +58,7 @@ public class LocalFileListerTest { ShadowLog.stream = System.out; TestUtils.loadServerCredentials(); TestUtils.prepareValidAccount(AccountManager.get(context)); - account = TestUtils.getValidAccount(); + final Account account = TestUtils.getValidAccount(); ocClient = DavClientProvider.getInstance().getClientInstance(account, context); } @@ -76,7 +74,7 @@ public class LocalFileListerTest { final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); if (rootDirectory.exists()) { - deleteDirectory(rootDirectory); + TestUtils.deleteDirectory(rootDirectory); } } @@ -382,16 +380,6 @@ public class LocalFileListerTest { - private void deleteDirectory(File file) - { - for (File subfile : file.listFiles()) { - if (subfile.isDirectory()) { - deleteDirectory(subfile); - } - subfile.delete(); - } - file.delete(); - } } -- GitLab From 0ab14647b3489fdbfa7cc8f17b0f20e1052eb318 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 15 Nov 2022 15:15:46 +0100 Subject: [PATCH 13/16] rewrite localFileListerTest to optimize fake file generation and deletion --- .../contentScanner/LocalFileListerTest.java | 110 ++++++++---------- 1 file changed, 49 insertions(+), 61 deletions(-) diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index d3211065..c6fc1573 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -8,16 +8,12 @@ package foundation.e.drive.contentScanner; -import android.accounts.Account; -import android.accounts.AccountManager; import android.content.Context; import android.database.sqlite.SQLiteDatabase; import android.os.Build; import androidx.test.core.app.ApplicationProvider; -import com.owncloud.android.lib.common.OwnCloudClient; - import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -28,7 +24,6 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLog; - import java.io.File; import java.io.IOException; import java.util.ArrayList; @@ -38,7 +33,6 @@ import foundation.e.drive.TestUtils; import foundation.e.drive.database.DbHelper; import foundation.e.drive.models.SyncedFileState; import foundation.e.drive.models.SyncedFolder; -import foundation.e.drive.utils.DavClientProvider; /** * @author vincent Bourgmayer @@ -50,16 +44,13 @@ public class LocalFileListerTest { private static final String TEST_FILE_TREE_ROOT_PATH = "../tmp/"; private LocalFileLister fileListerUnderTest; - private final OwnCloudClient ocClient; private final Context context; + private final File testRootDirectory; public LocalFileListerTest() { context = ApplicationProvider.getApplicationContext(); ShadowLog.stream = System.out; - TestUtils.loadServerCredentials(); - TestUtils.prepareValidAccount(AccountManager.get(context)); - final Account account = TestUtils.getValidAccount(); - ocClient = DavClientProvider.getInstance().getClientInstance(account, context); + testRootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); } @Before @@ -72,9 +63,8 @@ public class LocalFileListerTest { public void tearDown() { SQLiteDatabase.deleteDatabase(context.getDatabasePath(DbHelper.DATABASE_NAME)); - final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); - if (rootDirectory.exists()) { - TestUtils.deleteDirectory(rootDirectory); + if (testRootDirectory.exists()) { + TestUtils.deleteDirectory(testRootDirectory); } } @@ -128,16 +118,17 @@ public class LocalFileListerTest { private SyncedFolder buildSyncedFolderForSkipSyncedFolderTest(boolean isMedia, boolean isHidden, boolean isLocalScannable) { final String dirName = isHidden ? ".myFolder/" : "myFolder/"; - return new SyncedFolder("any", "/local/path/" + dirName, "/remote/path/" + dirName, isLocalScannable, true, true, isMedia); + return new SyncedFolder("any", TEST_FILE_TREE_ROOT_PATH + dirName, "/remote/path/" + dirName, isLocalScannable, true, true, isMedia); } @Test public void skipDirectory_lastModifiedSame_noUnsyncedContent_shouldReturnTrue() { final long lastModified = 123456L; - final File directory = Mockito.spy(new File("/local/path/test/")); + + final File directory = Mockito.spy(testRootDirectory); Mockito.when(directory.lastModified()).thenReturn(lastModified); - final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + final SyncedFolder syncedFolder = new SyncedFolder("any", directory.getAbsolutePath(), "/remote/path/test/", true, true, true, true); syncedFolder.setId(12); syncedFolder.setLastModified(lastModified); @@ -148,13 +139,16 @@ public class LocalFileListerTest { @Test public void skipDirectory_lastModifiedSame_unsyncedContent_shouldReturnFalse() { final long lastModified = 123456L; - final File directory = Mockito.spy(new File("/local/path/test/")); + + final String dirPath = testRootDirectory.getAbsolutePath(); + + final File directory = Mockito.spy(testRootDirectory); Mockito.when(directory.lastModified()).thenReturn(lastModified); - final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "/local/path/test/tutu.jpg", "/remote/path/test/tutu.jpg", "", 0L, 12, true, 3); + final SyncedFileState dummy = new SyncedFileState(6, "foo.jpg", dirPath + "foo.jpg", "/remote/path/test/foo.jpg", "", 0L, 12, true, 3); DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); - final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/" , "/remote/path/test/", true, true, true, true); + final SyncedFolder syncedFolder = new SyncedFolder("any", dirPath , "/remote/path/test/", true, true, true, true); syncedFolder.setId(12); syncedFolder.setLastModified(lastModified); @@ -164,10 +158,12 @@ public class LocalFileListerTest { @Test public void skipDirectory_lastModifiedChanged_noUnsyncedContent_shouldReturnFalse() { - final File directory = Mockito.spy(new File("/local/path/test/")); + final String dirPath = testRootDirectory.getAbsolutePath(); + + final File directory = Mockito.spy(testRootDirectory); Mockito.when(directory.lastModified()).thenReturn(654321L); - final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + final SyncedFolder syncedFolder = new SyncedFolder("any", dirPath, "/remote/path/test/", true, true, true, true); syncedFolder.setId(12); syncedFolder.setLastModified(123456L); @@ -177,14 +173,16 @@ public class LocalFileListerTest { @Test public void skipDirectory_lastModifiedChanged_unsyncedContent_shouldReturnFalse() { - final File directory = Mockito.spy(new File("/local/path/test/")); + final String dirPath = testRootDirectory.getAbsolutePath(); + + final File directory = Mockito.spy(testRootDirectory); Mockito.when(directory.lastModified()).thenReturn(654321L); - final SyncedFolder syncedFolder = new SyncedFolder("any", "/local/path/test/", "/remote/path/test/", true, true, true, true); + final SyncedFolder syncedFolder = new SyncedFolder("any", dirPath, "/remote/path/test/", true, true, true, true); syncedFolder.setId(12); syncedFolder.setLastModified(123456L); - final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "/local/path/test/tutu.jpg", "/remote/path/test/tutu.jpg", "", 0L, 12, true, 3); + final SyncedFileState dummy = new SyncedFileState(6, "foo.jpg", dirPath + "foo.jpg", "/remote/path/test/foo.jpg", "", 0L, 12, true, 3); DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); final boolean skip = fileListerUnderTest.skipDirectory(directory, syncedFolder, context); @@ -192,11 +190,11 @@ public class LocalFileListerTest { } //Test about inner class: DirectoryLoader - - @Test public void loadDirectory_dirNotExist_shouldReturnTrue() { - final SyncedFolder folder = new SyncedFolder("any", "/local/path/myFolder", "/remote/path/myFolder", true); + final String dirPath = testRootDirectory.getAbsolutePath(); + + final SyncedFolder folder = new SyncedFolder("any", dirPath, "/remote/path/test", true); final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); @@ -207,11 +205,13 @@ public class LocalFileListerTest { @Test public void loadDirectory_dirExist_noContent_shouldReturnTrue() { - final SyncedFolder folder = new SyncedFolder("any", "../local/path/myFolder/", "/remote/path/myFolder/", true); - final File file = new File(folder.getLocalFolder()); - final boolean dirCreationSucceed = file.mkdirs(); + final String dirPath = testRootDirectory.getAbsolutePath(); + + final SyncedFolder folder = new SyncedFolder("any", dirPath, "/remote/path/test/", true); - Assert.assertTrue("cannot create test folder: " + file.getAbsolutePath(), dirCreationSucceed); + final boolean dirCreationSucceed = testRootDirectory.mkdirs(); + + Assert.assertTrue("Cannot create test folder: " + dirPath, dirCreationSucceed); final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); @@ -219,37 +219,33 @@ public class LocalFileListerTest { final FolderWrapper wrapper = folderLoader.getFolderWrapper(); Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); Assert.assertFalse("dir shouldExist!", wrapper.isMissing()); - Assert.assertTrue("List of folderWrapper's content expected to be empty but contains: "+wrapper.getContent().size(), wrapper.getContent().isEmpty()); - //@todo beware, in case of previous assertion failure, the below code won't be executed - Assert.assertTrue("Test file has not been successfully removed: " + file.getAbsolutePath(), file.delete()); + Assert.assertTrue("List of folderWrapper's content expected to be empty but contains: " + wrapper.getContent().size(), wrapper.getContent().isEmpty()); } @Test public void loadDirectory_dirExist_withSubContent_shouldReturnTrue() { - final SyncedFolder folder = new SyncedFolder("any", "../local/path/myFolder/", "/remote/path/myFolder/", true); - final File folderFile = new File(folder.getLocalFolder()); - final boolean dirCreationSucceed = folderFile.mkdirs(); + final String dirPath = testRootDirectory.getAbsolutePath(); + + final SyncedFolder folder = new SyncedFolder("any", dirPath, "/remote/path/test/", true); + final boolean dirCreationSucceed = testRootDirectory.mkdirs(); + Assert.assertTrue("cannot create test folder: " + dirPath, dirCreationSucceed); + - final File subContent = new File(folderFile, "toto.txt"); + final File subContent = new File(testRootDirectory, "foo.txt"); try { - subContent.createNewFile(); + Assert.assertTrue("Can't create subFile", subContent.createNewFile()); } catch (IOException exception) { exception.printStackTrace(); Assert.fail("IO Exception: Can't create new file as content of test folder"); } - Assert.assertTrue("cannot create test folder: " + folderFile.getAbsolutePath(), dirCreationSucceed); - final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); final FolderWrapper wrapper = folderLoader.getFolderWrapper(); Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); Assert.assertFalse("dir shouldExist!", wrapper.isMissing()); - Assert.assertEquals("List of folderWrapper's content size expected to one but was: "+wrapper.getContent().size(), 1, wrapper.getContent().size()); - //@todo beware, in case of previous assertion failure, the below code won't be executed - Assert.assertTrue("SubFile has not been removed", subContent.delete()); - Assert.assertTrue("Test folder has not been removed: " + folderFile.getAbsolutePath(), folderFile.delete()); + Assert.assertEquals("List of folderWrapper's content size expected to one but was: " + wrapper.getContent().size(), 1, wrapper.getContent().size()); } @@ -302,18 +298,17 @@ public class LocalFileListerTest { } private boolean prepareFakeContent() { - /** + /* * Generate folders */ - final File rootDirectory = new File(TEST_FILE_TREE_ROOT_PATH); - if (!rootDirectory.mkdirs()) return false; + if (!testRootDirectory.mkdirs()) return false; - final File folder1 = new File(rootDirectory, "folder1/"); + final File folder1 = new File(testRootDirectory, "folder1/"); if (!folder1.mkdirs()) return false; final File folder2 = new File(folder1, "folder2/"); if (!folder2.mkdirs()) return false; - final File folder3 = new File(rootDirectory, "folder3/"); + final File folder3 = new File(testRootDirectory, "folder3/"); if (!folder3.mkdirs()) return false; final File folder4 = new File(folder3, "folder4/"); if (!folder4.mkdirs()) return false; @@ -321,7 +316,7 @@ public class LocalFileListerTest { if (!folder5.mkdirs()) return false; - /** + /* * Generate files * /!\ must be done before syncedFolders generation * otherwise lastModified value of directory will change due to file creation @@ -349,8 +344,7 @@ public class LocalFileListerTest { return false; } - - /** + /* * Generate SyncedFolders */ final SyncedFolder syncedFolder1 = new SyncedFolder("any", folder1.getAbsolutePath(), "/remote/path/myFolder1/", true); @@ -362,7 +356,7 @@ public class LocalFileListerTest { final SyncedFolder syncedFolder3 = new SyncedFolder("any", folder3.getAbsolutePath(), "/remote/path/myFolder3/", true); syncedFolder3.setLastModified(folder3.lastModified()); syncedFolder3.setId((int) DbHelper.insertSyncedFolder(syncedFolder3, context)); - final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", rootDirectory.getAbsolutePath()+"lambda/", "/remote/lambda", true); + final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", testRootDirectory.getAbsolutePath() + "lambda/", "/remote/lambda", true); syncedFolderRemoved.setLastModified(987654321L); syncedFolderRemoved.setLastEtag("123456789"); syncedFolderRemoved.setId((int) DbHelper.insertSyncedFolder(syncedFolderRemoved, context)); @@ -376,10 +370,4 @@ public class LocalFileListerTest { return true; } - - - - - - } -- GitLab From a481f8c98d673cee4fab2aa73341110c32367681 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Tue, 15 Nov 2022 15:56:27 +0100 Subject: [PATCH 14/16] refactor RemoteFileListerTest.java & RemoteFileLister --- .../contentScanner/RemoteFileLister.java | 4 +- .../contentScanner/RemoteFileListerTest.java | 311 +++++++++++++----- 2 files changed, 236 insertions(+), 79 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index 1e0b6e09..536e2b60 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -63,11 +63,12 @@ public class RemoteFileLister extends AbstractFileLister { @Override protected String getFileName(@NonNull RemoteFile file) { + System.out.println("File path: " + file.getRemotePath()); return CommonUtils.getFileNameFromPath(file.getRemotePath()); } @Override - protected FolderLoader getFolderLoader() { + protected RemoteFolderLoader getFolderLoader() { return new RemoteFolderLoader(); } @@ -117,6 +118,7 @@ public class RemoteFileLister extends AbstractFileLister { */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) public RemoteOperationResult readRemoteFolder(@NonNull String remotePath, @NonNull OwnCloudClient client) { + System.out.println("ReadRemoteFolder: "+remotePath); final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(remotePath); return operation.execute(client); } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java index 200e3456..31ae883f 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java @@ -8,6 +8,8 @@ package foundation.e.drive.contentScanner; +import static org.mockito.Mockito.when; + import android.accounts.Account; import android.accounts.AccountManager; import android.content.Context; @@ -26,11 +28,14 @@ import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; +import org.mockito.internal.configuration.injection.MockInjection; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLog; +import java.io.File; import java.util.ArrayList; +import java.util.List; import foundation.e.drive.TestUtils; import foundation.e.drive.database.DbHelper; @@ -46,6 +51,7 @@ import foundation.e.drive.utils.DavClientProvider; public class RemoteFileListerTest { private static final String DIR_NO_SKIP_EXPECTED = "directory shouldn't have been skipped"; private static final String DIR_SKIP_EXPECTED = "directory should have been skipped"; + private static final String TEST_FILE_TREE_ROOT_PATH = "/remote/"; private RemoteFileLister fileListerUnderTest; private final Account account; @@ -125,15 +131,15 @@ public class RemoteFileListerTest { private SyncedFolder buildSyncedFolderForSkipSyncedFolderTest(boolean isMedia, boolean isHidden, boolean isRemoteScannable) { final String dirName = isHidden ? ".myFolder/" : "myFolder/"; - return new SyncedFolder("any", "/local/path/" + dirName, "/remote/path/" + dirName, true, isRemoteScannable, true, isMedia); + return new SyncedFolder("any", "/local/path/" + dirName, TEST_FILE_TREE_ROOT_PATH + dirName, true, isRemoteScannable, true, isMedia); } @Test public void skipFile_hidden_shouldReturnTrue() { - final String hiddenRemoteFilePath = "remote/path/.tutu.png"; + final String hiddenRemoteFilePath = TEST_FILE_TREE_ROOT_PATH + ".foo.png"; final RemoteFile file = Mockito.mock(RemoteFile.class); - Mockito.when(file.getRemotePath()).thenReturn(hiddenRemoteFilePath); + when(file.getRemotePath()).thenReturn(hiddenRemoteFilePath); final boolean skipFile = fileListerUnderTest.skipFile(file); Assert.assertTrue("Hidden remoteFile (" + hiddenRemoteFilePath + ") should have been skipped", skipFile); @@ -141,9 +147,9 @@ public class RemoteFileListerTest { @Test public void skipFile_notHidden_shouldReturnFalse() { - final String notHiddenRemoteFilePath = "remote/path/tutu.png"; + final String notHiddenRemoteFilePath = TEST_FILE_TREE_ROOT_PATH + "foo.png"; final RemoteFile file = Mockito.mock(RemoteFile.class); - Mockito.when(file.getRemotePath()).thenReturn(notHiddenRemoteFilePath); + when(file.getRemotePath()).thenReturn(notHiddenRemoteFilePath); final boolean skipFile = fileListerUnderTest.skipFile(file); Assert.assertFalse("Not hidden remoteFile (" + notHiddenRemoteFilePath + ") should not have been skipped", skipFile); @@ -153,7 +159,7 @@ public class RemoteFileListerTest { public void skipFile_emptyPath_shouldReturnTrue() { final String emptyPath = ""; final RemoteFile file = Mockito.mock(RemoteFile.class); - Mockito.when(file.getRemotePath()).thenReturn(emptyPath); + when(file.getRemotePath()).thenReturn(emptyPath); final boolean skipFile = fileListerUnderTest.skipFile(file); Assert.assertTrue("RemoteFile with empty path (" + emptyPath + ") should have been skipped", skipFile); @@ -166,9 +172,9 @@ public class RemoteFileListerTest { /*in this case: No file, just a directory the current implementation of CommonUtils.getFileName(String path) will return "path" while empty string should be expected. - todo: fix CommonUtils.getFileName(String path)*/ + todo: fix CommonUtils.getFileName(String path) */ final RemoteFile file = Mockito.mock(RemoteFile.class); - Mockito.when(file.getRemotePath()).thenReturn(invalidPath); + when(file.getRemotePath()).thenReturn(invalidPath); final boolean skipFile = fileListerUnderTest.skipFile(file); Assert.assertTrue("RemoteFile with empty path (" + invalidPath + ") should have been skipped", skipFile); @@ -177,10 +183,10 @@ public class RemoteFileListerTest { @Test public void skipDirectory_etagChanged_noUnsyncedContent_shouldReturnFalse() { final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); - Mockito.when(remoteFile.getEtag()).thenReturn("6666bbbb"); + when(remoteFile.getEtag()).thenReturn("6666bbbb"); - final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); - syncedFolder.setId(120); + final SyncedFolder syncedFolder = new SyncedFolder("", "local", TEST_FILE_TREE_ROOT_PATH, true); + syncedFolder.setId(12); syncedFolder.setLastEtag("5555aaaa"); final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); @@ -191,9 +197,9 @@ public class RemoteFileListerTest { @Test public void skipDirectory_etagSame_noUnsyncedContent_shouldReturnTrue() { final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); - Mockito.when(remoteFile.getEtag()).thenReturn("5555aaaa"); + when(remoteFile.getEtag()).thenReturn("5555aaaa"); - final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + final SyncedFolder syncedFolder = new SyncedFolder("", "local", TEST_FILE_TREE_ROOT_PATH, true); syncedFolder.setId(12); syncedFolder.setLastEtag("5555aaaa"); @@ -205,53 +211,47 @@ public class RemoteFileListerTest { @Test public void skipDirectory_etagSame_contentToSync_shouldReturnFalse() { final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); - Mockito.when(remoteFile.getEtag()).thenReturn("5555aaaa"); + when(remoteFile.getEtag()).thenReturn("5555aaaa"); - final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + final SyncedFolder syncedFolder = new SyncedFolder("", "local", TEST_FILE_TREE_ROOT_PATH, true); syncedFolder.setId(12); syncedFolder.setLastEtag("5555aaaa"); - final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "local/tutu.jpg", "remote/tutu.jpg", "7777", 0L, 12, true, 3); + final SyncedFileState dummy = new SyncedFileState(6, "foo.jpg", "local/foo.jpg", TEST_FILE_TREE_ROOT_PATH + "foo.jpg", "7777", 0L, 12, true, 3); DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); - Assert.assertFalse("Remote directory with no new etag but there is content to sync from DB: should not have been skipped but it had", skip); } @Test public void skipDirectory_etagChanged_contentToSync_shouldReturnFalse() { final RemoteFile remoteFile = Mockito.mock(RemoteFile.class); - Mockito.when(remoteFile.getEtag()).thenReturn("6666bbbb"); + when(remoteFile.getEtag()).thenReturn("6666bbbb"); - final SyncedFolder syncedFolder = new SyncedFolder("", "local", "remote", true); + final SyncedFolder syncedFolder = new SyncedFolder("", "local", TEST_FILE_TREE_ROOT_PATH, true); syncedFolder.setId(12); syncedFolder.setLastEtag("5555aaaa"); - final SyncedFileState dummy = new SyncedFileState(6, "tutu.jpg", "local/tutu.jpg", "remote/tutu.jpg", "7777", 0L, 12, true, 3); + final SyncedFileState dummy = new SyncedFileState(6, "foo.jpg", "local/foo.jpg", TEST_FILE_TREE_ROOT_PATH + "foo.jpg", "7777", 0L, 12, true, 3); DbHelper.manageSyncedFileStateDB(dummy, "INSERT", context); final boolean skip = fileListerUnderTest.skipDirectory(remoteFile, syncedFolder, context); - Assert.assertFalse("Remote directory with new etag and there is content to sync from DB: should not have been skipped but it had", skip); } - //Test about inner class: DirectoryLoader + //Test about inner class: DirectoryLoader @Test public void loadDirectory_serverResponse404_shouldReturnTrue() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(404); - Mockito.when(fakeResult.isSuccess()).thenReturn(false); - - final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + when(fakeResult.getHttpCode()).thenReturn(404); + when(fakeResult.isSuccess()).thenReturn(false); - RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); - - Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(mockLoaderSpied.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = mockLoaderSpied.load(folder); Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); @@ -259,19 +259,15 @@ public class RemoteFileListerTest { @Test public void loadDirectory_serverResponse405_shouldReturnFalse() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(405); - Mockito.when(fakeResult.isSuccess()).thenReturn(false); - - final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); - - RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); + when(fakeResult.getHttpCode()).thenReturn(405); + when(fakeResult.isSuccess()).thenReturn(false); - Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(mockLoaderSpied.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = mockLoaderSpied.load(folder); Assert.assertFalse("Loading remote folder resulting in 405 should return false", loadSuccess); @@ -279,24 +275,20 @@ public class RemoteFileListerTest { @Test public void loadDirectory_successAndErrorCode_shouldReturnTrue() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(405); - Mockito.when(fakeResult.isSuccess()).thenReturn(true); + when(fakeResult.getHttpCode()).thenReturn(405); + when(fakeResult.isSuccess()).thenReturn(true); final ArrayList fakeData = new ArrayList<>(); final RemoteFile mockRemoteFile = Mockito.mock(RemoteFile.class); fakeData.add(mockRemoteFile); - Mockito.when(fakeResult.getData()).thenReturn(fakeData); + when(fakeResult.getData()).thenReturn(fakeData); - final RemoteFileLister.RemoteFolderLoader loaderUnderTest = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); - - RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(loaderUnderTest); - - Mockito.when(spiedLoaderUnderTest.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(spiedLoaderUnderTest.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = spiedLoaderUnderTest.load(folder); Assert.assertTrue("Loading remote folder with success despite error code (405) should return true", loadSuccess); @@ -305,13 +297,12 @@ public class RemoteFileListerTest { @Test public void loadDirectory_successAndRemoteContent_shouldReturnTrue() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(405); - Mockito.when(fakeResult.isSuccess()).thenReturn(true); + when(fakeResult.getHttpCode()).thenReturn(405); + when(fakeResult.isSuccess()).thenReturn(true); final ArrayList fakeData = new ArrayList<>(); final RemoteFile mockDirRemoteFile = Mockito.mock(RemoteFile.class); @@ -320,13 +311,10 @@ public class RemoteFileListerTest { final RemoteFile mockContentRemoteFile2 = Mockito.mock(RemoteFile.class); fakeData.add(mockContentRemoteFile1); fakeData.add(mockContentRemoteFile2); - Mockito.when(fakeResult.getData()).thenReturn(fakeData); - - final RemoteFileLister.RemoteFolderLoader loaderUnderTest = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); + when(fakeResult.getData()).thenReturn(fakeData); - RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(loaderUnderTest); - - Mockito.when(spiedLoaderUnderTest.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader spiedLoaderUnderTest = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(spiedLoaderUnderTest.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = spiedLoaderUnderTest.load(folder); Assert.assertTrue("Loading remote folder with success despite error code (405) should return true", loadSuccess); @@ -336,30 +324,197 @@ public class RemoteFileListerTest { Assert.assertTrue("mockContentRemoteFile2 should have been in the result of loading but is not", spiedLoaderUnderTest.getFolderWrapper().getContent().contains(mockContentRemoteFile2)); } - - - @Test public void loadDirectory_successButNoData_shouldReturnFalse() { - final String remotePath = "remote/path/myFolder/"; - final OwnCloudClient client = DavClientProvider.getInstance().getClientInstance(account, context); - final SyncedFolder folder = new SyncedFolder("any", "local/path/myFolder", remotePath, true); + final String remotePath = TEST_FILE_TREE_ROOT_PATH + "myFolder/"; + final SyncedFolder folder = new SyncedFolder("any", "local/myFolder", remotePath, true); final RemoteOperationResult fakeResult = Mockito.mock(RemoteOperationResult.class); - Mockito.when(fakeResult.getHttpCode()).thenReturn(204); - Mockito.when(fakeResult.isSuccess()).thenReturn(true); + when(fakeResult.getHttpCode()).thenReturn(204); + when(fakeResult.isSuccess()).thenReturn(true); final ArrayList fakeData = new ArrayList<>(); - Mockito.when(fakeResult.getData()).thenReturn(fakeData); + when(fakeResult.getData()).thenReturn(fakeData); - final RemoteFileLister.RemoteFolderLoader mockLoader = (RemoteFileLister.RemoteFolderLoader) fileListerUnderTest.getFolderLoader(); - - RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(mockLoader); - - Mockito.when(mockLoaderSpied.readRemoteFolder(remotePath, client)).thenReturn(fakeResult); + final RemoteFileLister.RemoteFolderLoader mockLoaderSpied = Mockito.spy(fileListerUnderTest.getFolderLoader()); + when(mockLoaderSpied.readRemoteFolder(remotePath, ocClient)).thenReturn(fakeResult); final boolean loadSuccess = mockLoaderSpied.load(folder); Assert.assertFalse("Loading remote folder resulting in 204 but without any RemoteFile should return false", loadSuccess); } + + /** + * Test the whole behaviour of localFileLister with a fake file's tree + * + * ## File tree + * >Folder 1 (unChanged) + * --> File 1 (should be ignored) + * --> Folder 2 (unsynced dir) + * ----> File 2 (new file should be added) + * ----> .File 3 (should be ignored) + * >Folder 3 (unChanged) + * -->Folder4 (skip because folder 3 unchanged) + * ----> File 4 (skip because folder 3 unchanged) + * -->.Folder5 (skip: hidden dir) + * ----> File 5 (skip because directory is hidden) + * + * There is also the missing remote folder, which is expected to be in the result's folder's id + */ + @Test + public void bigWholeTest() { + prepareFakeContent(); + Assert.assertTrue("File listing failed", fileListerUnderTest.listContentToScan(context)); + + final List foldersId = fileListerUnderTest.getSyncedFoldersId(); + Assert.assertEquals("List of folders to scan's id should contains 2 but contained: " + foldersId.size(), 2, foldersId.size()); + + final List contentListed = fileListerUnderTest.getContentToScan(); + Assert.assertEquals("ContentListed should have 1 files but contained: " + contentListed.size(), 1, contentListed.size()); + } + + /** + * Generate fake file tree & mock object for a full test of RemoteFileLister + */ + private void prepareFakeContent() { + /* + * Generate folders + */ + final String folder1Path = TEST_FILE_TREE_ROOT_PATH + "folder1/"; + final RemoteFile folder1 = Mockito.mock(RemoteFile.class); //unchanged + when(folder1.getRemotePath()).thenReturn(folder1Path); + when(folder1.getEtag()).thenReturn("123456789"); + when(folder1.getMimeType()).thenReturn("DIR"); + + final String folder2Path = folder1Path + "folder2/"; + final RemoteFile folder2 = Mockito.mock(RemoteFile.class); //updated one + when(folder2.getRemotePath()).thenReturn(folder2Path); + when(folder2.getEtag()).thenReturn("234567891"); + when(folder2.getMimeType()).thenReturn("DIR"); + + final String folder3Path = TEST_FILE_TREE_ROOT_PATH + "folder3/"; + final RemoteFile folder3 = Mockito.mock(RemoteFile.class); //unchanged + when(folder3.getRemotePath()).thenReturn(folder3Path); + when(folder3.getEtag()).thenReturn("345678912"); + when(folder3.getMimeType()).thenReturn("DIR"); + + final String folder4Path = folder3Path + "folder4/"; + final RemoteFile folder4 = Mockito.mock(RemoteFile.class); //new one + when(folder4.getRemotePath()).thenReturn(folder4Path); + when(folder4.getEtag()).thenReturn("456789123"); + when(folder4.getMimeType()).thenReturn("DIR"); + + final String folder5Path = folder3.getRemotePath() + ".folder5/"; + final RemoteFile folder5 = Mockito.mock(RemoteFile.class); //hidden one + when(folder5.getRemotePath()).thenReturn(folder5Path); + when(folder5.getEtag()).thenReturn("567891234"); + when(folder5.getMimeType()).thenReturn("DIR"); + + /* + * Generate files + */ + final RemoteFile file1 = Mockito.mock(RemoteFile.class); //ignored because dir unchanged + when(file1.getRemotePath()).thenReturn(folder1Path + "file1.png"); + when(file1.getMimeType()).thenReturn("any"); + final RemoteFile file2 = Mockito.mock(RemoteFile.class); //new file one, should be added + when(file2.getRemotePath()).thenReturn(folder2Path + "file2.png"); + when(file2.getMimeType()).thenReturn("any"); + final RemoteFile file3 = Mockito.mock(RemoteFile.class); //hidden one: ignored + when(file3.getRemotePath()).thenReturn(folder2Path + ".file3.png"); + when(file3.getMimeType()).thenReturn("any"); + final RemoteFile file4 = Mockito.mock(RemoteFile.class); //new one should be added + when(file4.getRemotePath()).thenReturn(folder4Path + "file4.png"); + when(file4.getMimeType()).thenReturn("any"); + final RemoteFile file5 = Mockito.mock(RemoteFile.class); //ignored because dir is hidden + when(file5.getRemotePath()).thenReturn(folder5Path + "file5.png"); + when(file5.getMimeType()).thenReturn("any"); + + /* + * Generate SyncedFolders + */ + final SyncedFolder syncedFolder1 = new SyncedFolder("any", "local/folder1/", folder1Path, true); + syncedFolder1.setLastEtag(folder1.getEtag()); + syncedFolder1.setId((int) DbHelper.insertSyncedFolder(syncedFolder1, context)); + + final SyncedFolder syncedFolder2 = new SyncedFolder("any", "local/folder1/folder2/", folder2Path, true); + syncedFolder2.setLastEtag(""); + syncedFolder2.setId((int) DbHelper.insertSyncedFolder(syncedFolder2, context)); + + final SyncedFolder syncedFolder3 = new SyncedFolder("any","local/folder3/", folder3Path, true); + syncedFolder3.setLastEtag(folder3.getEtag()); + syncedFolder3.setId((int) DbHelper.insertSyncedFolder(syncedFolder3, context)); + + final SyncedFolder syncedFolderRemoved = new SyncedFolder("any", "local/missingFolder/", TEST_FILE_TREE_ROOT_PATH + "lambda/", true); + syncedFolderRemoved.setLastModified(987654321L); + syncedFolderRemoved.setLastEtag("123456789"); + syncedFolderRemoved.setId((int) DbHelper.insertSyncedFolder(syncedFolderRemoved, context)); + + final ArrayList syncedFolders = new ArrayList<>(); + syncedFolders.add(syncedFolder1); + syncedFolders.add(syncedFolder2); + syncedFolders.add(syncedFolder3); + syncedFolders.add(syncedFolderRemoved); + + fileListerUnderTest = Mockito.spy(new RemoteFileLister(syncedFolders, ocClient)); + + + /* Mock the method depending to network and cloud */ + final RemoteFileLister.RemoteFolderLoader spiedDirLoader = Mockito.spy(fileListerUnderTest.getFolderLoader()); + + final RemoteOperationResult folder1Result = Mockito.mock(RemoteOperationResult.class); + when(folder1Result.getHttpCode()).thenReturn(207); + when(folder1Result.isSuccess()).thenReturn(true); + final ArrayList folder1Data = new ArrayList<>(); + folder1Data.add(folder1); + folder1Data.add(file1); + folder1Data.add(folder2); + when(folder1Result.getData()).thenReturn(folder1Data); + when(spiedDirLoader.readRemoteFolder(syncedFolder1.getRemoteFolder(), ocClient)).thenReturn(folder1Result); + + final RemoteOperationResult folder2Result = Mockito.mock(RemoteOperationResult.class); + when(folder2Result.getHttpCode()).thenReturn(207); + when(folder2Result.isSuccess()).thenReturn(true); + final ArrayList folder2Data = new ArrayList<>(); + folder2Data.add(folder2); + folder2Data.add(file2); + folder2Data.add(file3); + when(folder2Result.getData()).thenReturn(folder2Data); + when(spiedDirLoader.readRemoteFolder(syncedFolder2.getRemoteFolder(), ocClient)).thenReturn(folder2Result); + + final RemoteOperationResult folder3Result = Mockito.mock(RemoteOperationResult.class); + when(folder3Result.getHttpCode()).thenReturn(207); + when(folder3Result.isSuccess()).thenReturn(true); + final ArrayList folder3Data = new ArrayList<>(); + folder3Data.add(folder3); + folder3Data.add(folder4); + folder3Data.add(folder5); + when(folder3Result.getData()).thenReturn(folder3Data); + when(spiedDirLoader.readRemoteFolder(syncedFolder3.getRemoteFolder(), ocClient)).thenReturn(folder3Result); + + + final RemoteOperationResult folder4Result = Mockito.mock(RemoteOperationResult.class); + when(folder4Result.getHttpCode()).thenReturn(207); + when(folder4Result.isSuccess()).thenReturn(true); + final ArrayList folder4Data = new ArrayList<>(); + folder4Data.add(folder4); + folder4Data.add(file4); + when(folder4Result.getData()).thenReturn(folder4Data); + when(spiedDirLoader.readRemoteFolder(folder4.getRemotePath(), ocClient)).thenReturn(folder4Result); + + final RemoteOperationResult folder5Result = Mockito.mock(RemoteOperationResult.class); + when(folder5Result.getHttpCode()).thenReturn(207); + when(folder5Result.isSuccess()).thenReturn(true); + final ArrayList folder5Data = new ArrayList<>(); + folder5Data.add(folder4); + folder5Data.add(file4); + when(folder5Result.getData()).thenReturn(folder5Data); + when(spiedDirLoader.readRemoteFolder(folder5.getRemotePath(), ocClient)).thenReturn(folder5Result); + + final RemoteOperationResult missingFolderResult = Mockito.mock(RemoteOperationResult.class); + when(missingFolderResult.getHttpCode()).thenReturn(404); + when(missingFolderResult.isSuccess()).thenReturn(false); + when(spiedDirLoader.readRemoteFolder(syncedFolderRemoved.getRemoteFolder(), ocClient)).thenReturn(missingFolderResult); + + when(fileListerUnderTest.getFolderLoader()).thenReturn(spiedDirLoader); + } } -- GitLab From 33386fbff19586788c58b0aeaf1a95927758f35e Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 16 Nov 2022 12:14:56 +0100 Subject: [PATCH 15/16] Optimization: rewrite few part of code --- .../e/drive/contentScanner/AbstractFileLister.java | 8 ++++---- .../e/drive/contentScanner/LocalFileLister.java | 2 +- .../e/drive/contentScanner/RemoteFileLister.java | 2 -- .../e/drive/contentScanner/LocalFileListerTest.java | 8 ++++---- .../e/drive/contentScanner/RemoteFileListerTest.java | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 7168b1fd..0fb400e6 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -81,7 +81,7 @@ public abstract class AbstractFileLister { } if (isSomeContentToSync) { - DbHelper.updateSyncedFolders(folders, context); //@ToDo: maybe do this when all contents will be synced. + DbHelper.updateSyncedFolders(folders, context); //@todo: maybe do this when all contents will be synced. } return isSomeContentToSync; } @@ -170,6 +170,8 @@ public abstract class AbstractFileLister { if (subFile == null) continue; final String fileName = getFileName(subFile); + if (fileName == null) continue; + if (isDirectory(subFile)) { final SyncedFolder subSyncedFolder = new SyncedFolder(syncedFolder, fileName, 0L, "");//Need to set lastModified to 0 to handle it on next iteration iterator.add(subSyncedFolder); @@ -198,9 +200,7 @@ public abstract class AbstractFileLister { public List getSyncedFoldersId() { final List result = new ArrayList<>(); for (SyncedFolder syncedFolder : folders) { - //if (syncedFolder.isToSync() ){ //todo: is it usefull ? maybe this propertie can be removed from SyncedFolder - result.add( (long) syncedFolder.getId() ); - //} + result.add((long) syncedFolder.getId()); } return result; } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 785fafa7..95abc72b 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -61,7 +61,7 @@ public class LocalFileLister extends AbstractFileLister { } @Override - protected FolderLoader getFolderLoader() { + protected LocalFolderLoader getFolderLoader() { return new LocalFolderLoader(); } diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index 536e2b60..1e32511c 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -63,7 +63,6 @@ public class RemoteFileLister extends AbstractFileLister { @Override protected String getFileName(@NonNull RemoteFile file) { - System.out.println("File path: " + file.getRemotePath()); return CommonUtils.getFileNameFromPath(file.getRemotePath()); } @@ -118,7 +117,6 @@ public class RemoteFileLister extends AbstractFileLister { */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) public RemoteOperationResult readRemoteFolder(@NonNull String remotePath, @NonNull OwnCloudClient client) { - System.out.println("ReadRemoteFolder: "+remotePath); final ReadFolderRemoteOperation operation = new ReadFolderRemoteOperation(remotePath); return operation.execute(client); } diff --git a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java index c6fc1573..b0b9c720 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/LocalFileListerTest.java @@ -196,7 +196,7 @@ public class LocalFileListerTest { final SyncedFolder folder = new SyncedFolder("any", dirPath, "/remote/path/test", true); - final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.getFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); Assert.assertTrue("Loading remote folder resulting in 404 should return true", loadSuccess); @@ -213,7 +213,7 @@ public class LocalFileListerTest { Assert.assertTrue("Cannot create test folder: " + dirPath, dirCreationSucceed); - final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.getFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); final FolderWrapper wrapper = folderLoader.getFolderWrapper(); @@ -239,7 +239,7 @@ public class LocalFileListerTest { Assert.fail("IO Exception: Can't create new file as content of test folder"); } - final LocalFileLister.LocalFolderLoader folderLoader = (LocalFileLister.LocalFolderLoader) fileListerUnderTest.getFolderLoader(); + final LocalFileLister.LocalFolderLoader folderLoader = fileListerUnderTest.getFolderLoader(); final boolean loadSuccess = folderLoader.load(folder); final FolderWrapper wrapper = folderLoader.getFolderWrapper(); @@ -284,7 +284,7 @@ public class LocalFileListerTest { * SyncedFolderRemoved which correspond to a missing directory */ @Test - public void bigWholeTest() { + public void test_listContentToScan() { Assert.assertTrue("Preparation, of file Tree and syncedFolder has failed", prepareFakeContent()); Assert.assertTrue("File listing failed", fileListerUnderTest.listContentToScan(context)); diff --git a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java index 31ae883f..e7c86ee3 100644 --- a/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java +++ b/app/src/test/java/foundation/e/drive/contentScanner/RemoteFileListerTest.java @@ -362,7 +362,7 @@ public class RemoteFileListerTest { * There is also the missing remote folder, which is expected to be in the result's folder's id */ @Test - public void bigWholeTest() { + public void test_listContentToScan() { prepareFakeContent(); Assert.assertTrue("File listing failed", fileListerUnderTest.listContentToScan(context)); -- GitLab From 06eef32804a189bbdb6f54b76156f5f2c32cd712 Mon Sep 17 00:00:00 2001 From: vincent Bourgmayer Date: Wed, 16 Nov 2022 15:29:17 +0100 Subject: [PATCH 16/16] add new method to update SyncedFolder etag & last modified after listing files --- .../e/drive/contentScanner/AbstractFileLister.java | 3 ++- .../foundation/e/drive/contentScanner/LocalFileLister.java | 5 +++++ .../foundation/e/drive/contentScanner/RemoteFileLister.java | 5 +++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java index 0fb400e6..4515cb31 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/AbstractFileLister.java @@ -41,6 +41,7 @@ public abstract class AbstractFileLister { protected abstract boolean isDirectory(@NonNull T file); protected abstract String getFileName(@NonNull T file); protected abstract FolderLoader getFolderLoader(); + protected abstract void updateSyncedFolder(@NonNull SyncedFolder syncedFolder,@NonNull T folder); /** * This allows to use RemoteOperationResult for RemoteFileLister @@ -121,7 +122,7 @@ public abstract class AbstractFileLister { } else { syncedFolder.setToSync(true); } - + updateSyncedFolder(syncedFolder, currentDirectory.getFolder()); //todo: look where to put in subclasses : syncedFolder.setLastEtag(directory.getEtag()).setToSync(true); diff --git a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java index 95abc72b..378c52ff 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/LocalFileLister.java @@ -65,6 +65,11 @@ public class LocalFileLister extends AbstractFileLister { return new LocalFolderLoader(); } + @Override + protected void updateSyncedFolder(@NonNull SyncedFolder syncedFolder, @NonNull File folder) { + syncedFolder.setLastModified(folder.lastModified()); + } + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) public class LocalFolderLoader implements FolderLoader { private FolderWrapper folder; diff --git a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java index 1e32511c..99f9a257 100644 --- a/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java +++ b/app/src/main/java/foundation/e/drive/contentScanner/RemoteFileLister.java @@ -71,6 +71,11 @@ public class RemoteFileLister extends AbstractFileLister { return new RemoteFolderLoader(); } + @Override + protected void updateSyncedFolder(@NonNull SyncedFolder syncedFolder, @NonNull RemoteFile folder) { + syncedFolder.setLastEtag(folder.getEtag()); + } + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) public class RemoteFolderLoader implements FolderLoader { private static final int HTTP_404 = 404; -- GitLab