maritz / nohm

node.js object relations mapper (orm) for redis
http://maritz.github.io/nohm/
MIT License
500 stars 64 forks source link

[Help] Model.find() without arguments works? #141

Open zzswang opened 5 years ago

zzswang commented 5 years ago

In the documents, it says:

Finding all ids of a model
Simply calling find() without arguments will retrieve all IDs.

const ids = await SomeModel.find();
// ids = array of ids

But I've tried with following code:

    const vehilce1 = new Vehicle();
    const vehilce2 = new Vehicle();
    const vin1 = "11111";
    const vin2 = "22222";
    const vdoc1 = {
      ns: "/first",
      line: "111",
      modelBrief: "longbus",
    };
    const vdoc2 = {
      ns: "/second",
      line: "222",
      modelBrief: "longbus",
    };

    // set some properties
    vehilce1.property(vdoc1);
    vehilce1.id = vin1;
    await vehilce1.save();
    vehilce2.property(vdoc2);
    vehilce2.id = vin2;
    await vehilce2.save();

    const vs = await Vehicle.find();
    console.log(vs);

The out put is an emtpy array.

If we put a param like const vs = await Vehicle.find({ ns: "/second" });, then it works.

zzswang commented 5 years ago

Modal.sort() without arguments will throw error.

const Vehicle = Nohm.model("YourModelName", {
  properties: {
    ns: { type: "string", index: true },
    line: { type: "string", index: true },
    modelBrief: { type: "string", index: true },
  },
  methods: {},
  client,
});

await Vehicle.sort();

error

 Invalid field in YourModelName.sort() options: 'undefined'
maritz commented 5 years ago

Yep, you are right. It's a problem with the .id = thing I suggested to you in the other issue (#140).

The "idsets" sets are not filled this way. I thought there was a test covering this, but apparently there isn't.

I will try to fix it in the coming days, alternatively PRs are welcome. :-)

The problem is that here it doesn't properly check for numIdExiting vs create cases and then if that were to be fixed, the create method would need to take an optional id. Since this could lead to confusing behavior I would like to add a model configuration option "manualId" or something like that, so that usecases like yours are covered properly without affecting normal id creation scenarios.

maritz commented 5 years ago

The methods affected by this bug are: find, sort and exists.