molecuel / mongolastic

Mongolastic - Mongoose middleware for elasticsearch integration
MIT License
8 stars 4 forks source link

Nested Mapping Support #4

Closed skimmmer closed 10 years ago

skimmmer commented 10 years ago

I have added nested mapping support for subdocs in schemas. Let me know if I need to add/edit a test for the change.

DominicBoettger commented 10 years ago

Hello skimmer,

thanks a lot for your contribution! Could you please be so kind and fix the errors in the unit tests ( showing up when running npm test). It would also be great if you could add a few lines of code to the mapping test case. Then, of course, i will merge your code.

Best regards Dominic

skimmmer commented 10 years ago

Hi @DominicBoettger, do you still need the unit tests and fixes?

DominicBoettger commented 10 years ago

Saw that the errors were our fault. Fixed them. I implement a unit test for your stuff now. Will be only one line i think...

skimmmer commented 10 years ago

Okay, thanks!

DominicBoettger commented 10 years ago

Hi,

hope you can help me. Can you give me a example how you define you subschema? Just want to be sure to understand it correctly.

Thanks Dominic

skimmmer commented 10 years ago

Hi @DominicBoettger ,

Here’s an example, in this case “deck” is the main collection which each document has sub-documents of “card”:

var card = new mongoose.Schema({
    title: {
        type: String,
        default: '',
        trim: true,
        elastic: { mapping: { type: 'string'}}
    },
    created: {
        type: Date,
        default: Date.now,
        elastic: { mapping: { type: 'string', index: 'not_analyzed'}}
    }
});

var deck = new mongoose.Schema({
    title: {
        type: String,
        default: '',
        trim: true,
        elastic: { mapping: { type: 'string'}}
    },
    author: {
        type: String,
        default: "Anonymous",
        elastic: { mapping: { type: 'string', index: 'not_analyzed'}}
    },
    cards: [card],
    created: {
        type: Date,
        default: Date.now,
        elastic: { mapping: { type: 'string', index: 'not_analyzed'}}
    }
});

Let me know if you have any questions.

DominicBoettger commented 10 years ago

After merging your stuff there is something heavily broken with the mappings. They look quite different than before. Especially mappings of reference fields seem to be broken.

DominicBoettger commented 10 years ago

Hi,

i reverted the branch to the last commit after realizing that the output of the unit tests heavily differs from the expected output.

But i think only the injected stuff for the geo field was missing. I also realized that the rendering for mappings of the ref fields is missing ( this is my fault and i should fix this in the future ). I think we are not using field which are not part of the mongoose model at the moment.... Not sure if we will need that feature, but would be great if your code will support this.

Can you please check this and add a unit test?

That would be great! Thanks for your contribution.

Best regards Dominic

skimmmer commented 10 years ago

Okay, I haven't really used ref fields before and I'm not sure the correct way it is represented in the elasticsearch mapping, but I will look into it. I will let you know when I have a more stable version with some unit tests.

Thanks!