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 zcmeHQ36LDuS?-?MId*1uMviRDM~p@>K~}8M^nJvMZ5^w_vJUIA6(8f*-LH4Y+TB^r z%xWbJIBkb2D8NoaLMlaENhMW)DX0oZrBZ>S%oPY!f)g+~4A@j*2w(`|rU=K^J@^(8H z`_xqF)b2g^PnCS-OLvs2HLG;!=&7j#QzuHtPaHb3`^4GO;ib$!v9jv}QU78g#DSqJu|fNwZa@EiyeM;vJo-B2|S!% zq(imyO7>3pn#ygufJOr8Rj=iH?ND+XqS~gde~@}jWAzUNHezXqK9=5cWcx}CxBG-J z#8+BRMi=chjHfAuJKvpZGY_)qE#)$6dQC&-twz0Op|7=wMOtL}_vBCmEk9ej!=t5r zckFN{@!m{6@Q-`_Oz%(DgERcqJL{cjmvhy{KQRL_12F?J12F?J12F?J12F?J12F?J z12F?J16LOVlYqk(|BbwNKJi!bg=MdLE73UD(!z1QBn;>7c}0M^|qGmHsVfay_nd)BSx3YbpT7w0WFkDB#{)vR<74j*je)p(iL6=D2s%(0C_%9y!4b8}U1 z(|2eNjS$!x$7^P5v4L|Z%?73O&6#=&U;~c=Emb*=s)9ra`^?MtedOX{78g0NE%HVB zikB(-@^dq#iANKEwt)o1s$vGNA_gAbgj?mbxYDh%Z||Wam17HZelPs$8w9r^?#Oy` z@DOetqR6-hO^u4|GvD~onN5=!#=OjRof-rFdv1$_zYBU9(l7tmsn?31WWIixk^bMP ze{4L@yj`VCYCJohJyD-4HEFAb=U}tqdX86`mzpg)S6OV*MiURg%07CbW@6agPil>Y z89Xbi`;SZ2#rbpd^`-erqdr&d$+OS=%?G~v{tH>?KfM&D-@AzIK5|H_R-fNRcUg_v z1zK_UG+;nq#pTGtLUp#*Y*ldG=fABT$>f>ax;blC743)CaeECOqf8sz29dmD|V;-qrs0Bah zI%g`Ke>>24-ZpHp4GsO~vCG$Ya!@@{ILQ>9!e@$d@n;MFSiGTdq4NxOCAW++?)Jo!r=91^XaY0y@5(P zll0VW$$J9Ndsecsa8HYX7|gc;-`v_v_HCE2qUi z^hBK7&59IziclY?y+0PCCuD2CZdy;R?ARFr?CYFyGxRx2ne|&85TE-t^j~R#= zxW*W8ruaM~I$JLtNwMQ>v(}*V$Luq;`Yc6m@ml_x|LERFE8YO5MGZK_aJlX+gHoFd3n>Y=`4Yo2lEHy-MM ziAvBe+g?nt4VmMJ78NyBAskp%I8ISzMO0+n5(wdlpnC-0Ub-%Ud-4~rt^3l+U{CRF z=MH(?F4RnYu|}}TqHLR zAyHO`q+L7=iFektb&U{;V6~FQfd*R;WXZCLYKof8X|~(-H^0|Ka>Ec3d38wI#lw(z zXGNapY>t=+hsSB6BC3|gNvdj?iZ1H1$OFlr{rep*lIw?%2&+TVE*^%&J8P@%5NwD< z)CA6uOpOSlArT&fv7lR);39cr0Z4M2bIjJc!`&XsbE|{WDIbQVeOfma)8;MP#L!Po z4grN_QQ{1fmt`E|i3{d0K2c1}zXezxsCZab(P+MKp`5woNr&!)0Yz zRaDF7Z5yp&i`2c|7rC*7oBywve1R#-g$E};Jn_wmz4@QcGnoB03+ZJw5wYUdF3l{R)gu@K36xE}SL zv1q^+xejfJtZMO`CRw6xYo?}~ydo(AQDxmWs7~Ezt3UAaKWs`qUAhPSq?{wpogP03 z2+mPSmPNd`8iL4kD#Ei$JkjuI;wi6{%S-iHyKIui1rSqSnyIx~v-lL1pmpi@GqfIc zrl_dyNKbiFC88=B3ZDBaCL5w|h>9e+6n!PTDd{ZTJ;6{%b*}Rm>hE@|V&)^DjifTK z(-!fvTd8Tp-G0S@Cru>r=qJUgvWcKrQ2@J zzf{U49s2GG=B#t4lM1RirwdY9;046`ukLBomkR85)NVb0Pjl<}u9fSFNvOVo_4KJ*MLn0xN_e$5iN(pB!W*1ssHTQb zB3f7%C0_CB`T66h=h9sxdNQZ;dYMNoa)H~po;+GuSGv<;I8ZVUg$3~HW#XO5}V_8vh-3zk7(6dveB%Xb=@=9L#+-dIn{d8c%QNaz&bG-c zo6WZ77OFl~QECw^xR#m(YRWn{t2yTFPoDOMuOp{UmfaD1*MemSP7QjtirtM?d5$z% zHL`fIG9=bFt%R20sUq5h$aq-d9~dia&Js9SDP)+0c+;Ify*l^MmW1;*1zqx~vmUlq z152H=w2HJUGjwHudm1&fKI^7p@eB4LjwmJK++vF~)@Y{U^~!*JA7!9zaHufR4PqIl4U>;SWX;fd!4PFl z!YrLrbX{>7J$lm(3FjUOx0Q42q6gM!)X&lR1q9!(HP>v|6nC81QtwPDp}+4kT(K8c4TgbSaoH-1gaj5VbofjYz7dnU*41g3T+Ip;OtAVR4Ww*WQ4xc6}np zzQo+Xo@0u+!j{Qf*>lPBi65DGB>(C04`#ofEoYq66KN|qlX@jTmUv_A=h*LzZD+sc z{R)ZiTjiX_Q8SM^Z+DWcWa}zScBE{ol2n#pFH7FVyuM0p*IscewY`3X7{+Ywj`8{K|F*~DD<4mUAbq98t@qhFqC2*9(y2)}SD|K6+1owCf?=_e4o#vfR zGJThmBH&Rf&&pvwl0cWddi8#>?$nN3W|<0|COLaMTKnnj~?mSxBs>>6@Z>ij}} z(tKs^b24dnX^tIQBU=kkd}Ub0%D47#`EHk2bXl-4T~$Td5(JaiZ7RqzrmvQOH57%9 zN=GC;eukom9nRg&oDXTh%X7F(j!ZaCl@vwA>$afq7K}NfXj-DeNj4EBB`P_Q^!Uk% zB6g^URTpJU35jA;3rnuVQZT8)Dl4_IbjevpWUNv21YnIIc5uYoMj6%wQU=c^r*eh> zYf)6d{x_EpU4GC}MGtgIme3@;w_1b*hX%nSks)A}*ut`&C>wg@2Sg-20eB-^em`!B zcSO4_oLlhNE7sU3H(}5i)iYH@Qn0Gh)h^?jEKq9M5+*mq#*!hiO^KIuNrrJolZYL) zmqgOzXFQ77{avguhgn3zGP3}%7S9u^5}p^SWe7yFB_S%-NP7HOqlis8@6Hfshm&FP zh9mIRy~J>5`c(@SqUpsNCgAu$XQ|n0)Xp!`W~jAbS?&$D@^iW;tBS3_HpKw z^qt8srhY25ZR}%{X8w=z$HrgGekRLjes1!c#Sa$#JR`S%N00hY`h%lOT!FLQIm@9V z2RaK}yjbOBQRdXBw`G{9zY#)8J{$%@6uGs6=!&ikqEqrcp&*1cGa+_FBYf8&D^&! z6bDpC!VN5Q{93y0km$(j43;!QL2!|$WCLSETXjb?0*H1=BLGBuD3jL$Vn}o(h{2MN zg@S;GTSTvt0Vdk2JE9Rlv`aoRei!qdjp<$2o)&)t2yzt@0A5_f2s$h}ssV;d&IN#Q zJGBH~&(-$=VMuf&h{2Mxp&*3F)8c+t?11QqMgY++c`i3~RlekOs}tF}=x3rW8ifh3 z81<^^Ju@_TM4E~K%Y|ktWl5rD zSu>oI1@A=RKFL-j5 z5&BkIAQ>er&ZXez!wxy?jNFeD>c2CPtsu0FIys>*nOun}-ZXi%VCUbL`^v<}@HPI4 z8HgE(8HgE(8HgE(8HgG9f69PUnaeY-*PQ3-IxsD~;+7}dwk2Xv%$j%wp53OzVRNx8 z>!xTJ1{{mn_Z5EY6?d0YY-WeMmZiXt*y6G8gTlQVh-dHElq@ie z8%_K&|2UP|QM_D1gCjQ4V@r-`>Qoi5b=wr+vLxZ4A*u(pITMKQdGX+;B%1Jw zv}A20FKh1Mt2U0QoC$ARc;0e~Ef|twz!M$2GIUKRibjd(^704YL`%}?5j*L4j+e@u zhCNXmvXd@MUC)iFzpJuY>3{C7Y6n4)B^a0x~74?y$>yZI=QWi+uZQv#!r%a4^`_@Hwn( z&5|Vd2K%XvXn|PAWn;k6GhP&gfPNM95)stdBleF#b?cCw4j-(py|5K`hqXgU_=mN& zLzucK?GPgS?iHXfkLa*Y=stC;fWBPzT?{k!+-1A9V#P57F#|CJ(HRJP(?r0&wZ3U~ zMOS{)?3A=8$6L%MhG(*eGoMYrn);`)-KkrW4=0{xU%y0 zbKZI%qJ1p2Ba!$vodyl}%eO!Hh5T@RWnhKmh$PA)XTeazX$tl@5lx~9OG0^zSKWX# z|8W!SHfIj!8Fz!VXRnU5Su9|(w;XaLJ^orm5p&~A$f^dzt72JHL5xVAAh?Q6O+|yr zmFrjR&_KnaY}bs`7EyMc`YRUY@`wz=*^(*|T|tCfMC8J@cFE=qOA-YHl@Z;F{R`)| z*ulYyMcCUJsbW#|1XL`77~;2>6y`KU;884u)ljJ=$=JDzP0f~ootkdNz75l=V;-1b zyoeWGUyQl|G)nCv>+;tyswga@vMCcAfm0DPPs1)>#2P~oR|OSTIZ<|N`FD@I%{MjB zd{OoVMyX&VJ^so?5hIAUs>3FZaALNNn1GxNOR$dcu!!M{t)5&=e||Xgdq-hHBZ=&6Hj@dS&dH{NaFvFQ{1v)jas;oYl4!{a97+sCw0GaYo?_gk>dMi9 z#r2TjNl{RRiTW{ylq7hcLO@bH7Zi+y2xDp(7JPIxNk)DPEVTC#9~nR#5-BSR;xJJ^ z;*gRe2VxCdcbT91ce=+WR#Qg*B5m6#_M1dbkkH0Qa#1QIF z&=f>RR#6$m6Z#qlkb@L(nMq8 z#{B2^mJ;+`D1K@w8{iAsogE2G*TXlWfJ~t3>P@%6w`a1e2uNmR%MmD zcyt#gtqQV$iG0;mNaT1-qs09fMwC9;4cO(rNFpB`k`M_Tehdz=lVK7?I15arVN8s= zyE2jym*T3v}^fdA1%sJ;q8Z{0LW&c;E-pZuo9? zAoO;Sxoz~e&7o-A1N$Qb_jnE!mM^3b;+*wIDIW=9MBws)tB)w9I9tq*&Fo~De_H-fvsEG-;w(#&pez$COmfGa%91~si%A6 z^V)6c>E8HEbvO2OZ+z~#8+y9E1aP5KFkfE5cJWSA2A{qDu@rK|xoz1_=oVdhr+k?9 z_UX;VI`gY5ch;-3gtIyACIPvs5`gF~D5Owe{do^Y4%vZ^f}m6g0|R(u842z8fvc(w z-TZ$yG9P7%O5xGTk57(IJeB{K{NCIL#$O#TXP?Y`A+se-Q_qimD*5Y4Iq?GXQT8zV zZ&#!OPQ@Cq5%qVu4287_y$J_$kA;c)2kwxPSo@$@GbB_~u@a+k$e!q0jfi2Ih7RKq z`^;Cr_EoHZI6G(uaTW=QLtJ~KAPy7tBMvEPaUw@7kyunm3SSFLH->=CEv8Nob{xT_ zU7Mjj!0vK2T{n`T8)XGw=dS3=SUV*p9UIPg6DfJHS&7QX;fvJ67R**i-^C#)3H!`9 zKJ?#NARaf{h>@sgoE>VaD;yHe#bS)Yas?y90ZWxkz~KeX;+`3L**1r(*fM2i;^}kB><9)ve*Rv{?bcyhYRG;p1bguGa{~ zwmcK}0TNchwkx@o{b%p|ENVVgb*;|DnwRY>iZ@Zv@X~Xcjig{zr73Sn6p#gMj>HnY zrP^4r<}gw!rh&$E^Y1-3Q^IKT*uZEraPJPr7Yu^F+z%^E)IZvUlr)HHh?YSlET_xF z5I8KRAj&r!39xo6>uz4N=W>s`zwU4`q9~p@_^4!M9Ts!h&B?8Cok|GilB4?+FJj+OS0cRm9A#FTziHWMISf=YQ z@a&mSyNDkih4>0a=@pO{$%5aVs2GGG*`VllYXi9fu-1kc>+Zt+v-kbGi}*bw5O?G9 zMtEv2=eR4nGRK{g8g^>qPD4sQ6ZzJ#SzCq6h)Hc#Gz}YDe1Z5!U)tm%K06BW6^#EY zQ;hORkq%!g#gy@Kq8OH&y%LL=x{X2HMf_V2xriSciFgg6F56+B#nlE2RIC$2UCFub93Tg7-r1(oNhA>*-%-OZF2 zG@jRW&E@z7Ec!W_GdTvfUhj>eZ?1?k0uPk-ch>$%l~-leG6hLAMGL9VMNUA&6A@`k z4M7sz{`u6zCtM__hmdT% 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