ScanStore: Store threats data in db#1806
Conversation
Scan API responds with a site's threats within the scan state response. With the current API structure, "Fetch Threat" and "Fetch Threats" APIs are not required by the app, which might have wanted a dedicated ThreatStore. To keep things simple, we're deleting the store and rest client for threats and retaining just the ThreatModel, ThreatMapper and corresponding custom serializers. Scan module can now have separate tables for storing scan state and threats and need not interact with a separate ThreatStore.
malinajirka
left a comment
There was a problem hiding this comment.
Thanks @ashiagr! Looks great, it feels definitely better to keep it within the ScanStore - good call!
Note: There's scope of optimisation: we can probably pass response threat model to the ThreatSqlUtils to avoid json conversion while saving special objects. Lmkwyt.
I personally wouldn't worry about it. We don't expect thousands of threats anyway - more of 1-3 threats. I'd keep it as is unless we encounter performance issues while testing. Thanks for thinking about it though.
|
@malinajirka Hello there! It seems that you created a It happens that I just released a hotfix 1.7.1 on top of 1.7.0 – to bring back a fix we patched back in 1.6.26.1 but forgot to merge back to develop, so 1.7.1 now includes this old patch on top of 1.7.0. It's already end of my (long) day so I won't have time to fix that myself before my AFK, but I'll let you in the hands of @jkmassel to decide what to do with that Personally I recommend either one of those options to fix this:
In either case it might be worth considering deleting the 1.7.0-beta-1 GitHub pre-release, or at least edit its description to mention it's not appropriate release to use, and to use 1.8.0-beta-N instead. |
|
👋 @AliSoftware,
Thank you @jkmassel for creating I'll delete |
Parent wordpress-mobile/WordPress-Android#13326
This PR
ThreatStore,ThreatAction,ThreatRestClientas threats can be managed within theScanStoredue to how APIs are structured (threats returned as part of Scan data) and removing them helps in avoiding complexity betweenThreatStore&ScanStorerelationship - 2682769ThreatContext,Extension,List<Rows>are stored as json strings - 17dfbc7, 52e3142To test
ScanStoreTest,ThreatSqlUtilsTestcover important flows and run properly.Note: There's scope of optimisation: we can probably pass response threat model to the
ThreatSqlUtilsto avoid json conversion while saving special objects. Lmkwyt.