Adds lazy resource property #899
Conversation
| } | ||
|
|
||
| public Vector<Resource> getLazyResources() { | ||
| // todo |
There was a problem hiding this comment.
What's this todo for?
There was a problem hiding this comment.
sorry, left it by mistake
ShivamPokhriyal
left a comment
ShivamPokhriyal
left a comment
There was a problem hiding this comment.
Looks good to me.
ctsims
left a comment
ctsims
left a comment
There was a problem hiding this comment.
Hey. Biggest feedback (well, for a small issue) is that I think we should be manually making the lazy flag metadata a string with a fixed set of static values rather than a Boolean object (and potentially the real value as well, potentially. We don't get much from making the metadata a boolean but it adds a lot of risk
|
|
||
| private String descriptor; | ||
| protected String descriptor; | ||
| private boolean lazy; |
There was a problem hiding this comment.
I think this means we're also going to need to bump the appdb database version in formplayer
| case META_INDEX_VERSION: | ||
| return version; | ||
| case META_INDEX_LAZY: | ||
| return lazy; |
There was a problem hiding this comment.
I'm a little surprised to see this works with a boolean, btw. My intuition is that we should hard code an explicit string value here, and convert it to/from boolean. We've started making soft assumptions elsewhere that all metadata values are strings, and I'm worried that implicit conversions.
| } | ||
|
|
||
| public Vector<Resource> getLazyResources() { | ||
| return storage.getRecordsForValues(new String[]{Resource.META_INDEX_LAZY}, new Boolean[]{true}); |
There was a problem hiding this comment.
do we use this now that we're mostly iterating on ID's? This feels a bit like a potential bomb call, it'll grab a LOT of records.
There was a problem hiding this comment.
This still gets used by the MM verification task here to skip any lazy resources during MM verification. I profiled a simialr call which gets all resources in the table for cas which was taking .1 - .2 secs for around ~1600 resources so didn't think it as a big deal for the one time MM verfication task. I can make a similar optimization here where we just collect the uris for all lazy resources by looping over the resources one by one. Though do you think if it's worth to optimize that way ?
There was a problem hiding this comment.
Awesome, no need to optimize for .1 / .2 seconds.
| * @return A Vector of Externalizable objects e, such that the fields specified are equal to the corresponding values provided. | ||
| * | ||
| */ | ||
| Vector<E> getRecordsForValues(String[] metaFieldNames, Object[] values); |
There was a problem hiding this comment.
Is this the same signature as the custom implementations in Android AND formplayer? Is good to finally have it committed in code.
|
|
||
| public Resource(int version, String id, Vector<ResourceLocation> locations, String descriptor) { | ||
| this(version, id, locations, descriptor, false); | ||
| this(version, id, locations, descriptor, "false"); |
There was a problem hiding this comment.
Can we make these static accessors? Lots of risk of typos.
Multimedia Changes -
ResourcepropertyMissingMediaExceptionusingMissingMediaExceptionTypecross-request: dimagi/commcare-android#2243