yahoo / squidb

SquiDB is a SQLite database library for Android and iOS
https://github.com/yahoo/squidb/wiki
Apache License 2.0
1.31k stars 132 forks source link

Update function #257

Closed MFlisar closed 7 years ago

MFlisar commented 7 years ago

This is a question, though it's too long to write a good formatted gitter post...

I'm using an extended insertOrUpdate method like here: https://github.com/yahoo/squidb/issues/226 that looks like following:

public <T extends TableModel> Result insertOrUpdate(T item, IUpdateBeforeUpdate<T> updateBeforeUpdate, Property<?>... logicalKey) {
    // Validate the arguments
    if (logicalKey == null || logicalKey.length == 0) {
        throw new IllegalArgumentException("No logical key specified");
    }
    for (Property<?> col : logicalKey) {
        if (!item.containsNonNullValue(col)) {
            throw new IllegalArgumentException("Item did not have a value to compare for logical key column " + col);
        }
    }

    // Build a criterion for checking for existing models with the given logical key
    Criterion existingCriterion = logicalKey[0].eq(item.get(logicalKey[0]));
    for (int i = 1; i < logicalKey.length; i++ ) {
        existingCriterion = existingCriterion.and(logicalKey[i].eq(item.get(logicalKey[i])));
    }

    boolean success;
    Result result = null;
    Table t = getTable(item.getClass());
    T existing = (T) fetchByCriterion(item.getClass(), existingCriterion);
    if (existing == null)
    {
        success = insertRow(item);
        if (success)
            result = Result.Insert;
        else
            result = Result.Error;
        L.d(L.G_APP_UPDATE, "NEW ROW: %s", item);
    }
    else
    {
        if (updateBeforeUpdate != null)
            updateBeforeUpdate.update(item);
        existing.setPropertiesFromValuesStorage(item.getSetValues(), t.getProperties());
        success = updateRow(existing);
        if (success) { // This is not necessarily required, but it mimics the bookkeeping that persist() does
            item.setRowId(existing.getRowId());
            item.markSaved();
            result = Result.Update;
        }
        else
            result = Result.Error;
    }
    return result;
}

My updateBeforeUpdate paramater does only do following: it calls item.clearValue(field) for some user fields that MAY already exist so that those already existing values are not overwritten. So far so good.

Problem

After calling such an update function with a full loaded model, the model is not useable anymore, because it may have fields that were cleared.

What I want

I want to call the updateOrInsert function and keep all fields but be able to skip some fields from being persisted when calling this functions (BUT keep them in the model if they are in there.) Can I do this? Or do I have to extract the fields, save them in a list, save my model and reset the fields?

sbosley commented 7 years ago

Sure, there are many ways to do this. Probably the easiest is to simply call setPropertiesFromValuesStorage using only the properties you are interested in, which I imagine you could provide using your IUpdateBeforeUpdate object:

existing.setPropertiesFromValuesStorage(item.getSetValues(), updateBeforeUpdate.whichPropertiesToSave());

If you do this though, note that the contract of markSaved will be somewhat broken. It will still put the skipped values into the databaseValues of the model, which is interpreted to mean "these were the last known values to exist in the database" -- which of course if some set values were skipped, they are not. Probably it's more "correct" to instead make item take on only the values from existing after the update has occurred, e.g. using item.clear(); item.readPropertiesFromValuesStorage(existing.getDatabaseValues()). Of course, you will have to decide what is the most "correct" for your use case. Hope that helps!

MFlisar commented 7 years ago

Those set/getPropertiesFromValuesStorage are a bit confusing imho, I don't get their meaning by reading the name just as the AbstractModel setValues and values field as well. I think something like get/setPersistetValues() and persistedValues and notYetPersistedValues or similar would reflect the purposes a little more, just my personal opinion though...

I think I get the idea now and can solve my problem with the tipps, thanks