Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dynamic template support for properties #577

Closed
wants to merge 1 commit into from
Closed

dynamic template support for properties #577

wants to merge 1 commit into from

Conversation

mhaeussler-magari
Copy link
Contributor

To support dynamic templates you need an abstract property, which can handle any array result.
Scalar types do not work here, so I introduced the property type "object" like in earlier versions.

Now you can do something like:

/**
 * @ES\Document(
 *      type="myType",
 *      dynamicTemplates={{
 *          "myDynamicTemplateDefinition": {
 *              "path_match": "myField.*",
 *              "mapping": {
 *                  "type": "string",
 *                  "index": "not_analyzed"
 *              }
 *          }
 *      }}
 * )
 */
class MyDocument
{
    /**
     * @var array
     *
     * @ES\Property(name="myField", type="object")
     */
    private $myProperty = array();
}

this supports dynamic templates
@saimaz saimaz added the question label Mar 2, 2016
@mhaeussler-magari
Copy link
Contributor Author

I have no idea why one test is failing, maybe you can give me a hint to fix this.

  1. ONGR\ElasticsearchBundle\Tests\Unit\Service\ManagerTest::testSetCommitModeException
    Failed asserting that exception message 'The commit method must be either refresh, flush or none.' contains 'The commit method must be either refresh or flush'.

@mhaeussler-magari
Copy link
Contributor Author

@saimaz You are right, that the index creation is working. But when I try to push data to it I get the following exception:

Elasticsearch\Common\Exceptions\BadRequest400Exception: {"error":{"root_cause":[{"type":"mapper_parsing_exception","reason":"failed to parse [myField]"}],"type":"mapper_parsing_exception","reason":"failed to parse [myField]","caused_by":{"type":"illegal_argument_exception","reason":"unknown property [SOME-DYNAMIC-STUFF]"}},"status":400}

In my use case I want to put an array with key, value pairs into myField.

@mhaeussler-magari
Copy link
Contributor Author

@saimaz Oh and I just saw that you added an options property to the document definition. When I try this I got the following exception:

[Doctrine\Common\Annotations\AnnotationException]
[Creation Error] The annotation @es\Document declared on class ..\Document\MyDocument does not have a property named "options". Available properties: type, enabled, all, dynamic, dynamicTemplates, transform, dynamicDateFormats

@saimaz
Copy link
Member

saimaz commented Mar 2, 2016

This is probably because you are using some 0.* version. My example refers to the latest version. Currently 1.0.0-alpha.7

@saimaz
Copy link
Member

saimaz commented Mar 2, 2016

If you want to store key value array in some field, you have to define it as object in the dynamic template or document as well. This is how elasticsearch works :).

@saimaz
Copy link
Member

saimaz commented Mar 2, 2016

Sorry, my bad. options are in properties not in the document.

your:

/**
 * @ES\Document(
 *      type="myType",
 *      dynamicTemplates={{
 *          "myDynamicTemplateDefinition": {
 *              "path_match": "myField.*",
 *              "mapping": {
 *                  "type": "string",
 *                  "index": "not_analyzed"
 *              }
 *          }
 *      }}
 * )
 */

is correct.

The problem is with the way how you are trying to use elasticsearch IMO. Can you shortly explain what you want to do with your document?

@mhaeussler-magari
Copy link
Contributor Author

@saimaz Thank you for your fast response. I think I described my problem not well enough.

What I want to do:
Create a field in elasticsearch which has a dynamic mapping, so I can push data with dynamic keys, which I do not want to define hard coded in the mapping definition.
E.g.
myField.dynamicKey1 = someValue
myField.dynamicKey2 = someOtherValue
and so on. In my use case their can be houndreds of keys, which I do not know, because they are provieded by an external system.

The definition of the dynamic template is working fine, means the mapping is correctly created in elasticsearch.

The problem is that I can not define a property with dynamic content with ongr any more.
Normaly I would create an embedded annotation to another class with concret property definitions. In my case I just do not know the names of the properties, because they are dynamic.

In some older versions of ongr the "object" type was available WITHOUT using a concret class with concrete properties. That worked fine.
I do not know about an equivalent with the 1.0.0-alpha.7 version.

So I reintroduced the "object" type for properties WITHOUT using a concret class. Ongr puts the whole array result from the dynamic field into this property, which is fine for me.

I hope this explanation makes my problem more clear :)

@saimaz
Copy link
Member

saimaz commented Mar 2, 2016

Totally clear. I will play a bit with the dynamic templates later to see if there is a quick way to resolve the problem.

Maybe the solution could be to create an Embeded object relation, but in that object you could use magic setters and getters. Maybe that could be the solution.

@saimaz saimaz added bug and removed question labels Mar 2, 2016
@mhaeussler-magari
Copy link
Contributor Author

Thank you :)

Yeah I thought about this solution as well. Would be more elegant.
As far as I understood some changes in \ONGR\ElasticsearchBundle\Result\Converter::assignArrayToObject would be necessary. Actually it depends on the metadata with the concrete defined properties with setters/getters.

@saimaz
Copy link
Member

saimaz commented Mar 2, 2016

Well, you can extend also a Converter change functionality to support your behaviour and then override it's definition in service.yml.

@mhaeussler-magari
Copy link
Contributor Author

Do I understand you right, that my actual proposal is not the way the problem should be solved?

I can try another solution. To be honest, I do not think that I can build a quick fix by implementing the Embeded solution ;)

@saimaz
Copy link
Member

saimaz commented Mar 2, 2016

I'm not saying it's not right. From the first view it looks not very good. I just need some time to think about how to solve it. My suggestions above was how tou could temporary solve the problem in your project.

@mhaeussler-magari
Copy link
Contributor Author

Ok, you have a point.

I will close this pull request and will look forward to your implementation.
So far I can use my workaround.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants