realm-java icon indicating copy to clipboard operation
realm-java copied to clipboard

Add primary key on migration

Open rshah opened this issue 9 years ago • 8 comments

I want to add new field as primary key to old model (there was no primary key in my model but the contents are unique), but in migration i got this error:

java.lang.IllegalArgumentException: Illegal Argument: Field "projectUniqueKey" cannot be a primary key, it already contains duplicate values: 

This is the new model

class Project extends RealmObject {
   @PrimaryKey @Index
   private String projectUniqueKey;
   ...
}

This is my migration code

RealmObjectSchema projectSchema = schema.get("Project");
        projectSchema.addField("projectUniqueKey", String.class, FieldAttribute.PRIMARY_KEY)
                .transform(new RealmObjectSchema.Function() {
                    @Override
                    public void apply(DynamicRealmObject obj) {
                        obj.set("projectUniqueKey", getUniqueKey());
                    }
                });

I'm using 0.87.3

rshah avatar Jan 27 '16 03:01 rshah

Can you please share the source code of getUniqueKey?

beeender avatar Jan 27 '16 03:01 beeender

Ah, getUniqueKey just pseudo function, but i got exception before calling this line

obj.set("projectUniqueKey", getUniqueKey());

I added some log to print into logcat but never called. Then i changed to addField without setting it as primary and set it later. The exception was gone.

projectSchema.addField("projectUniqueKey", String.class)
                .transform(new RealmObjectSchema.Function() {
                    @Override
                    public void apply(DynamicRealmObject obj) {
                        Log.d("realm", "Set the unique key"); //this line never called in above codes.
                        obj.set("projectUniqueKey", getUniqueKey());
                    }
                });
RealmResults<DynamicRealmObject> projectResults = realm.allObjects("Project");
//Do something with the result
...
//Then add the primary key
projectSchema.addPrimaryKey("projectUniqueKey").addIndex("projectUniqueKey");

rshah avatar Jan 27 '16 06:01 rshah

Thanks a lot! I think it is a bug that if there are objects exist, when adding a field as primary key, the primary key checking will fail because of all primary key will be empty string in this case before transform called.

I am not sure if it can be fixed, but at least a more user friendly message should be printed.

And your workaround should work well. Thanks!

beeender avatar Jan 27 '16 16:01 beeender

Right now it is actually working as intended and your work-around is the correct one. That said, I fully understand your use case, but to generalize it a bit: You are actually asking for something like "Default values" for fields. We have a issue tracking that here: https://github.com/realm/realm-java/issues/777

cmelchior avatar Jan 27 '16 19:01 cmelchior

Another workaround is using renameField then addPrimaryKey, addIndex using new field name. I did rename field from my old @Required field(not needed in new model) to 'projectUniqueKey'. The error also gone.

rshah avatar Jan 27 '16 23:01 rshah

@cmelchior I don't think use the default value to solve this is the way to go. For the PrimaryKey:

  1. it has to be unique, which means we need supply a generator callback with the object as the input
  2. if the pk generator logic exists in the child RealmObject, that means we are coupling RealmObject with migration, and the RealmObject needs to hold version information which should be ideally only exist in the migration block.

beeender avatar Jan 29 '16 06:01 beeender

A generator is just a generalisation of a single default value, and I wouldn't add the generator logic to the ReamClass. A quick idea below:

@PrimaryKey
@DefaultValue(generator = PrimaryKeyGenerator.class)
private String id;

@PrimaryKey( = "42")
@DefaultValue(value = "42")
private String id

public class PrimaryKeyGenerator implements DefaultValueGenerator<String, Foo> {
  @Override
  public String generateValue(Foo object) {
    return UUID.getRandom().toString();
  } 
}

public class SingleValueGenerator implements DefaultValueGenerator<String, Foo> {
  @Override
  public String generateValue(Foo object) {
    return "42"
  } 
}

I am not sure that just holding the PK generator in the migration is a good idea. It sounds like we are trying to solve a really small sub-problem that is covered by other use cases: Default Values and Calculated Fields. If we just try to solve that without thinking about the others, we probably end up having to redesign the API. Right now our migration API also has a small problem in that it doesn't really support annotation parameters. In order to do that we need to add another layer of abstraction. Cocoa calls this RealmProperty (I think).

cmelchior avatar Jan 29 '16 09:01 cmelchior

Working as intended, but we need to document it.

cmelchior avatar Apr 14 '16 09:04 cmelchior