From f3406088f30f72a308e169a11f6f168cc8286897 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sat, 9 Sep 2017 11:02:02 -0700 Subject: [PATCH] Use a KeyPath to generalize code for checking for tags, attachments, and authors changes. --- Frameworks/Database/ArticlesTable.swift | 157 +++++++----------- .../Extensions/ArticleStatus+Database.swift | 2 +- ToDo.ooutline | Bin 2970 -> 3097 bytes 3 files changed, 62 insertions(+), 97 deletions(-) diff --git a/Frameworks/Database/ArticlesTable.swift b/Frameworks/Database/ArticlesTable.swift index 2866f9f40..4db09de41 100644 --- a/Frameworks/Database/ArticlesTable.swift +++ b/Frameworks/Database/ArticlesTable.swift @@ -339,57 +339,57 @@ private extension ArticlesTable { } - func updateRelatedObjects(_ parsedItems: [String: ParsedItem], _ articles: [String: Article]) { - - // Update the in-memory Articles when needed. - // Save only when there are changes, which should be pretty infrequent. - - assert(Thread.isMainThread) - - var articlesWithTagChanges = Set
() - var articlesWithAttachmentChanges = Set
() - var articlesWithAuthorChanges = Set
() - - for (articleID, parsedItem) in parsedItems { - - guard let article = articles[articleID] else { - continue - } - - if article.updateTagsWithParsedTags(parsedItem.tags) { - articlesWithTagChanges.insert(article) - } - if article.updateAttachmentsWithParsedAttachments(parsedItem.attachments) { - articlesWithAttachmentChanges.insert(article) - } - if article.updateAuthorsWithParsedAuthors(parsedItem.authors) { - articlesWithAuthorChanges.insert(article) - } - } - - if articlesWithTagChanges.isEmpty && articlesWithAttachmentChanges.isEmpty && articlesWithAuthorChanges.isEmpty { - // Should be pretty common. - return - } - - // We used detachedCopy because the Article objects being updated are main-thread objects. - - articlesWithTagChanges = Set(articlesWithTagChanges.map{ $0.detachedCopy() }) - articlesWithAttachmentChanges = Set(articlesWithAttachmentChanges.map{ $0.detachedCopy() }) - articlesWithAuthorChanges = Set(articlesWithAuthorChanges.map{ $0.detachedCopy() }) - - queue.update { (database) in - if !articlesWithTagChanges.isEmpty { - tagsLookupTable.saveRelatedObjects(for: articlesWithTagChanges.databaseObjects(), in: database) - } - if !articlesWithAttachmentChanges.isEmpty { - attachmentsLookupTable.saveRelatedObjects(for: articlesWithAttachmentChanges.databaseObjects(), in: database) - } - if !articlesWithAuthorChanges.isEmpty { - authorsLookupTable.saveRelatedObjects(for: articlesWithAuthorChanges.databaseObjects(), in: database) - } - } - } +// func updateRelatedObjects(_ parsedItems: [String: ParsedItem], _ articles: [String: Article]) { +// +// // Update the in-memory Articles when needed. +// // Save only when there are changes, which should be pretty infrequent. +// +// assert(Thread.isMainThread) +// +// var articlesWithTagChanges = Set
() +// var articlesWithAttachmentChanges = Set
() +// var articlesWithAuthorChanges = Set
() +// +// for (articleID, parsedItem) in parsedItems { +// +// guard let article = articles[articleID] else { +// continue +// } +// +// if article.updateTagsWithParsedTags(parsedItem.tags) { +// articlesWithTagChanges.insert(article) +// } +// if article.updateAttachmentsWithParsedAttachments(parsedItem.attachments) { +// articlesWithAttachmentChanges.insert(article) +// } +// if article.updateAuthorsWithParsedAuthors(parsedItem.authors) { +// articlesWithAuthorChanges.insert(article) +// } +// } +// +// if articlesWithTagChanges.isEmpty && articlesWithAttachmentChanges.isEmpty && articlesWithAuthorChanges.isEmpty { +// // Should be pretty common. +// return +// } +// +// // We used detachedCopy because the Article objects being updated are main-thread objects. +// +// articlesWithTagChanges = Set(articlesWithTagChanges.map{ $0.detachedCopy() }) +// articlesWithAttachmentChanges = Set(articlesWithAttachmentChanges.map{ $0.detachedCopy() }) +// articlesWithAuthorChanges = Set(articlesWithAuthorChanges.map{ $0.detachedCopy() }) +// +// queue.update { (database) in +// if !articlesWithTagChanges.isEmpty { +// tagsLookupTable.saveRelatedObjects(for: articlesWithTagChanges.databaseObjects(), in: database) +// } +// if !articlesWithAttachmentChanges.isEmpty { +// attachmentsLookupTable.saveRelatedObjects(for: articlesWithAttachmentChanges.databaseObjects(), in: database) +// } +// if !articlesWithAuthorChanges.isEmpty { +// authorsLookupTable.saveRelatedObjects(for: articlesWithAuthorChanges.databaseObjects(), in: database) +// } +// } +// } // MARK: Save New Articles @@ -412,63 +412,30 @@ private extension ArticlesTable { // MARK: Update Existing Articles - // TODO: use a keypath instead of separate functions. Fix code duplication. - - func articlesWithTagChanges(_ updatedArticles: Set
, _ fetchedArticles: [String: Article]) -> Set
{ + func articlesWithRelatedObjectChanges(_ comparisonKeyPath: Keypath>, _ updatedArticles: Set
, _ fetchedArticles: [String: Article]) -> Set
{ return updatedArticles.filter{ (updatedArticle) -> Bool in if let fetchedArticle = fetchedArticles[updatedArticle.articleID] { - return updatedArticle.tags != fetchedArticles.tags + return updatedArticle[keyPath: comparisonKeyPath] != fetchedArticle[keyPath: comparisonKeyPath] } assertionFailure("Expected to find matching fetched article."); return true } } - func articlesWithAttachmentChanges(_ updatedArticles: Set
, _ fetchedArticles: [String: Article]) -> Set
{ + func updateRelatedObjects(_ comparisonKeyPath: Keypath>, _ updatedArticles: Set
, _ fetchedArticles: [String: Article], _ lookupTable: DatabaseLookupTable, _ database: FMDatabase) { - return updatedArticles.filter{ (updatedArticle) -> Bool in - if let fetchedArticle = fetchedArticles[updatedArticle.articleID] { - return updatedArticle.attachments != fetchedArticles.attachments - } - assertionFailure("Expected to find matching fetched article."); - return true - } - } - - func articlesWithAuthorChanges(_ updatedArticles: Set
, _ fetchedArticles: [String: Article]) -> Set
{ - - return updatedArticles.filter{ (updatedArticle) -> Bool in - if let fetchedArticle = fetchedArticles[updatedArticle.articleID] { - return updatedArticle.authors != fetchedArticles.authors - } - assertionFailure("Expected to find matching fetched article."); - return true - } - } - - func updateRelatedTags(_ updatedArticles: Set
, _ fetchedArticles: [String: Article], _ database: FMDatabase) { - - let articlesWithChanges = articlesWithTagChanges(updatedArticles, fetchedArticles) + let articlesWithChanges = articlesWithRelatedObjectChanges(comparisonKeyPath, updatedArticles, fetchedArticles) if !articlesWithChanges.isEmpty { - tagsLookupTable.saveRelatedObjects(for: articlesWithChanges.databaseObjects(), in: database) + lookupTable.saveRelatedObjects(for: articlesWithChanges.databaseObjects(), in: database) } } - func updateRelatedAttachments(_ updatedArticles: Set
, _ fetchedArticles: [String: Article], _ database: FMDatabase) { + func saveUpdatedRelatedObjects(_ updatedArticles: Set
, _fetchedArticles: [String: Article], _ database: FMDatabase) { - let articlesWithChanges = articlesWithAttachmentChanges(updatedArticles, fetchedArticles) - if !articlesWithChanges.isEmpty { - attachmentsLookupTable.saveRelatedObjects(for: articlesWithChanges.databaseObjects(), in: database) - } - } - - func updateRelatedAuthors(_ updatedArticles: Set
, _ fetchedArticles: [String: Article], _ database: FMDatabase) { - - let articlesWithChanges = articlesWithAuthorChanges(updatedArticles, fetchedArticles) - if !articlesWithChanges.isEmpty { - authorsLookupTable.saveRelatedObjects(for: articlesWithChanges.databaseObjects(), in: database) - } + updateRelatedObjects(\Article.tags, updatedArticles, fetchedArticles, tagsLookupTable, database) + updateRelatedObjects(\Article.authors, updatedArticles, fetchedArticles, authorsLookupTable, database) + updateRelatedObjects(\Article.attachments, updatedArticles, fetchedArticles, attachmentsLookupTable, database) } func saveUpdatedArticles(_ updatedArticles: Set
, _ fetchedArticles: [String: Article], _ database: FMDatabase) { @@ -480,7 +447,7 @@ private extension ArticlesTable { func articlesWithParsedItems(_ parsedItems: Set, _ feed: Feed) -> Set
{ - // These Articles don’t get cached. Background-queue only. + assert(!Thread.isMainThread) let feedID = feed.feedID return Set(parsedItems.flatMap{ articleWithParsedItem($0, feedID) }) } @@ -507,8 +474,6 @@ private extension ArticlesTable { // Drop parsedItems that we can ignore. - assert(Thread.isMainThread) - var d = [String: ParsedItem]() for (articleID, parsedItem) in parsedItems { diff --git a/Frameworks/Database/Extensions/ArticleStatus+Database.swift b/Frameworks/Database/Extensions/ArticleStatus+Database.swift index 2774c2dff..3752fca62 100644 --- a/Frameworks/Database/Extensions/ArticleStatus+Database.swift +++ b/Frameworks/Database/Extensions/ArticleStatus+Database.swift @@ -12,7 +12,7 @@ import Data extension ArticleStatus { - convenience init(articleID: String, dateArrived: Date, row: FMResultSet) { + init(articleID: String, dateArrived: Date, row: FMResultSet) { let read = row.bool(forColumn: DatabaseKey.read) let starred = row.bool(forColumn: DatabaseKey.starred) diff --git a/ToDo.ooutline b/ToDo.ooutline index a095913db673d54d07deea48ca9ce084775c559d..1718e4fa036cd472e3696b24c40acf3740b5c85f 100644 GIT binary patch literal 3097 zcmZ{mS2P@o7KW$jy(QyDZwVQM5sVT89? z0x2I^J}=XGD=cV$%l|w$Z4tubm)*~u8Z%)&&7{3UAcz1 z|8PxO%^0~(LH6@?Copeev5NEga@ejNidpuMfo>{M(mUq>2=22ey}F$v)1J`d(Zms| zvpNfGdm}tjMEP26DL@7WaM~nl^^nM=Vu=$1R9S5hEwcK%GyO?Kl=h`K39TuY*E~HOzAm%Ujnl!u_C+-K{T5;0;&l z8pFf7rV~pUUiMFWNi$y`K^JgbOwv&aI{dfLpJ;`;!(Q7is|piTKp;weMwl*@i3_ux zMkZYNO+PSMz^HuUF2X}($JtvorWA;v66OkIO)`G=(e#V?UWYeRRJ{Cb3oTD13MnPN}=!34K=naS59GV!Ak(y_u5AP9X?PJ>ApJ(CKTA zc9i-^6ron>VH%2y|179vmYaiq&=wiOG;MkVtYFt)Qk}kD&r-v;Xr;MfJ}TuoT~I__ zUS+5SN$R1Sv@FVp(ognUA{~p~h=))Izc{7J*e(5(Um|I@%sfr3)vK?-KCr;K<0*rM zqsWY$y+qwuTn_Cn>vochRGoJ;bXANeD}Y)7$!XH`F$!C=KNCI+HmfgO_b3I!eJ8Fb^I^hW6GE13B(`ro7 z=iR>LKKIN^*(@O-TqW5}=8`5w4qc(Q5{H}vbapilx(NJ~&`CKWAzjl*-pO386eoNr z#yFl~`}J{`$!uZu$Xuqh&%T&OccZU|X+up--;Z=Hy|1G*Cal^fqf)_5Z8=t{r)%9N za5J8lbj3mxsdw6QMyFHWEQh|&FN)~XJT?2M4*i<@^qMtkg1!X)*ps4C{z+Bf`=5by zFX+s5=-iM|ZE+WanYg&&ONIAK=XshsWokM{B_p{WoX=B|t7ly|O?ParyMEQF8E>W% zi{>~RL0sI>Ww`L#8nCqAchyKWlWW_C5~}{Bu?iCq;w!K%pESF7^Om>*yZXUlb=`El zmm;$8;*XF(#C3dD>JI)I-p9L8_Vz@FTHsB5prwKChkUM`+EaZ^K$yZQBJ*2z8`6oT zPp+`iou&9lyDTxuaEhZLBPr)cV%LV?n*8}mEuHbxUY-7yt@t!nOaLHev15bLu3d<3 zDS9>J=|+ZuSlJF9FV;l+*+;hf+OhSM$L=DOI;wt&JNe+4C~ptRC{<<2>;1;*zRwdw z3N!MjHot@hJQ@@DP9e$4c)XnF(qD?4HGYr z&#spa7Kv?FF?&kWyUew-Gj;NS;Bg`mT&M^7dm%7ChbsL`wN9S23>Y$tu=uL*Ra;75 zN_xJ{O5B{9sVb zu8K|l+-}-6vUoP1mnc``mkW|Uh}5lNMKyG3Day67c0Jr>h-tkvwokGLwjwG(y>b%v zKBms-A>LZts=B$@yx7`IFIm`qd|WV8O<=A_fhYyJN45uXC&)QX6+m*^xW9{TmLJT_ zm^bD!Nz*=p@2nDbCyx@0sGKU=Bg|^LfLOjeMwXxf_A1c`AaX@LV z0KMJFaBAVD?X}?~9If@x7QAHniw&va0qIQbVv3MzkX-*xvkuYri5s`yyE@U|;jZ7i zMM4G3yRG6^vx`~>Zj}T<z^`r6yh?Wi-0^~n%JCSpGZG!orl$As_Ql#t z?D!#RtY#rPB?{m{T@;J4@NpZPZHYE`*S8JhLGM^7eHlLBXO%!VbFa;XQZuU)<7Hli z3-XR3)WNGKN9~xckG{G-rD8EGucXa(L#<2U{`2-r^FIQg;_1Hq`PO2#=EB#dj7f&! zSUePBH})JPK_}Zvik#mgi%bscc!TI+htiwjE0L-DRZDG!g$M6w{JObmS|MpmAL%_c zKXWP$?!O*8U>q_aCB0SN6wGE*R8WLboloI&t6ZNc4uN0i10M>Q$7R>dM6sQ*flR#5 z!smkQuTA>xfhWa>!e(%Hxj*@wxBfvi;=zq?vm!Vh7Dc9&xOK^yM#6OJL> zkCIJrWmHH*%V+7rK1(klmn69pt})Tb73e1MSEG~f2V-=9Zt8RvM`FYsHBfLkLF->N zuqF7)-syl!vE4g#b>X_MwwC2}CKDqzYa#xe_~JB{Y*fO_+pg!n%9Pfky0cZ{>s`J) zHtCV0K#ld(p^Qx44(NE-ZFhLKIu`RXAtPRJl1w5j;3!7>iqvr7IeIl3RIElrFAC9G zN8oiTP2bkr-zTTXX|2v3YE@fkMmXaF_)QCFyy96)rcaEi9HgvCc))Y-1QB2i0 zK%+u1yx~L~^U#$<_x*jOv)iTfcX<~N0Li-KnJmCJDNMWi*(maxG4O|Mj{HOO*i?6o zlKX@IqEbl>Mw7NQTVviDkz9vsEb0BVxgfHWyf9QsYHn-E9gJjs((bepsZ|onQ~SAhXe`6Q@POu<+~DN&+$pW0 z4iE$Y|9>O@jUWIBxcXgxZU5`%|4jaq-2Wz7sQ<>Xp$<94Ur*5Q_WeB*{^A_)Z@9+L A-T(jq literal 2970 zcmZ{mXE+;-7RM8zX01l;*wxybh#jg{(HbppQCo}_G5Xf3QBg(Io}o6e_pB`_ics@K zTC{d38j9QZe!1_x&vVcDaL#ki^Z)ky@G~(0k%IvM02Sbq`z-?~*f1xL3IL#%1_0=O zUmc!#A)UOCz7m0+9y8X~z6(m6UHjDsKM?`W+zy;{q}Mg|M*73hcIfFX0gtC($6=gI z&8CdI9&NyCKeg6njR5-WREbCsb2o^E9C(+0l@J8gGuJ9%2t>WBnJ_?OxggS|wEfJs zZO#1t>DG-hZKmR{3MO-B;g5b9$gxI6fEuHLmewoaBKE@=HbAy4K28bG_56KigiE}Hgr=AUaQCrZxN z2&Tc<1QyU6xU|!BL6SvXrxLH1h{K(j+}u`)WG+g+Kci^#1wnVHm`P7=OY!yQ4*}v8 ziKhH%3^w#;?H2VOjZW!VJ?!wW$0RL{`A=Esw`76^fG~J$FXd6%ChhP(Gn!VLj1^Y*iprVjiuS&RB2d=r`HJ7W#_LXRw_FI`>wH z$<;-PDMZLyWX|jBB57tnKSDSeU<%L$knla_ii&vznRXy~U{g~WYpU0`5n0awO{zv| z_#f$6s7yL#ne-2pXUK9H#p3`&4cXZm;8@hlT9)4UVYk6IMU9;+@G4=G5jW+OC8p{vY zdB(#>W^e~GcHC5sgAXFL-=EM(rMo6mA25Fe+jaD1e;J^+!1_9Wd6-B*ieOPMo|q?E z?`6oh+b@QfU`<)`n_-?`m#h;KUTn^_Al)9PtjkVkzIS3NE>3nXoFE=wi|{hvuH8@g z5<@a^{3Isr8pL!hZm_t4?vm3wb-tRe5u;;;WftGC%pb5>u6gtDvokvVLbn>$WW%7D z*n^1C+^&#nsr^8aG(Gt}bcickmDQ~*#kDMi#p-QbAKE*}3g(7!-HqiJ_m4?kV9j?5 zb>L~Mw0)I!Q6qa&f~7j`_jWSB|NKP1L##^6Vik99xa@O<-}HT)-Uz+sBBZ)g{HI^(VVl7w~%kChp7f&ZV4Qj1%(+ zw3Ik0XtteVB_NRRrRX6m(330>H>QX4iyU@m|9bc?EhUu5EiL@lApqSGJqGDcre*6{ zi9aP}xT`fbl<)5~{g7B1S?01_a$lRNblMT8p4TbQQnQ2b#|(gJF7S_3Ccxv+_}#t+ zv351l%hA{@;0;BM>xqz@J%#P5NXD{BTN@h~pif{+`%oeXzKF5~GW;cwc3Qu=B2mOf z*2C!Xpp_IS!$`Uj)!{psqI!YgB9`H%tesakP|i%lB*Px671`VMg-y-oiZ7QDLXsM^ zvm5wS|EK-dMwIs0w*dGdpT<A$t%D_UM3|q{jmNJOpjSCJ%%##zGo+{S z1|51?s37J+P3(E+z8aUGmZVRAoVp<$>_w)!iLjJeibrtWmPW7_1FTtnxkSLYFbfTW zMAg?)Zl8Jhlss7wkPpetj&D)yiSm;Pq3gQgcUcaL>axnnI(_AM^eBF5IA)ldFIzLM zt3QDimk{JBCjOvMPc&o*=Vwk(uV++saS}2xz>2eK`@Sxn$STbZ6BEdF5!A_6kpfRD zjJR5k3c1f(&gjpSXFg_TiObB`NC^}d6Quppmp7$~|4oO_>D$_)2J>}Fog%B@U?D&vQ?(Kpqs&BmvgM9SqeFZL1zVtKWZXs0 z=nAlZzyuf%7wAnGSiY|**s16-AC;XrDOKtI3xQ0fV9?YF-ZVqOPBeh|{NygWjPS1Y z)$jgYm|wp8R~i#9NWS+E^?K^X1tR!EJ4Ge3&TgYsaIPFoa^RVt#Kac#J2ZS%q@m=~ zUVUjzlRM7^B#X{#@VRz{=-kjAd`GTFJ5rBBO>p8Xq4y9)awGakauq;U%D-KLIu&&P z0^mcjc~{G^Huz93>E?=H8Lr;JiAzSFV~aeyQ=Q^c8GEvKy#xo9uqJ69+)CTjxaDfz ziJP0&LbpE!hV^;bwl$3PS1NDSb2hK}jqhx#D>LoKCxI>DT>+B*h$9~R?N=|xLzWje zdq!ty>J~B1+wUQc9;53fdm9ZZ)9mdZI2}(5cUNe_F*)>q7Ehe_owImR*loVEg%d`2 z81rn-Ldn2HMQ?Id`b&a?gVww1nq$v1IC{4E3S{zMCZiklI1@q!0A!E@0JOhN#>vqQ z`ON3vA`_Qn`fNc89JNAXNyWGr+VLtdiLeabM#Pq<(4T}G2Bo$5?cn{xkPD$0(5tG7 z^uXA?&Xv~9TwG_Yr^qD7TNbM*rBX#HG8Q)pGcLmX1qw;j`7kRwbgvwGFPZHpJE6aI zN46+ythnl-kbS`s7VedwNMPzuRPD`$7>Yk*$!5jc7BKzFURk?H3y@ zM5Q08jXZtpk*qN?Qd~C_d`)n%yrs&>Fr{cVL&c{RxRsNpChGZ>N+D?ZH`6fiRCMa3Cltx&;txO1+nw1b=-lfA0Ss|FSW>?+tqc z(~=OMmx5KgMdgTDE4zo&B=g$3Wg0bxVMt-TKz|cM?9*I7_|kE_+ZM<>J{?6uM|?eGN;h z^OLzH{N#LZ+8&?Yf#?)|Ia7Z$ae4qaZ&Wui0FnVf|6hrJvjhMFu720w+dl>Qf0O@< e;D3@MzZLr*K$;j(T>JY;_Pc$5Po}?t1^f&DIgvL2