Skip to content

Inheritance from abstract type not working #1682

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

Closed
bytenik opened this issue Dec 23, 2015 · 8 comments
Closed

Inheritance from abstract type not working #1682

bytenik opened this issue Dec 23, 2015 · 8 comments
Assignees

Comments

@bytenik
Copy link
Contributor

bytenik commented Dec 23, 2015

I've got an abstract base class of type Activity and subtypes of a variety of types, including Searched.

For mappings, I fluent-mapped Activity but was forced when fluent-mapping Searched to remap everything mapped on Activity.

On the storage side, I do for example client.Index<Activity>(instanceOfSearched, x => x.Index("Activities").Type(typeof(Searched))).

When I search, using client.Search<Activity>(x => x.Indices("Activity").AllTypes().MatchAll()), I get JsonSerializationExceptions based around an inability to instantiate Activity because it is abstract. It doesn't seem to notice that the stored type is actually a Searched.

@Mpdreamz
Copy link
Member

You need to either specify .ConcreteTypeSelector() to determine the actual type to instantiate based on the response hits or use .Types(typeof(Searched), .....) in which case we build a concrete type selector automagically under the covers. .AllTypes() does no magic like that (like scanning for all subclasses for Activity) because we can not safely assume that's what you want to do.

Hope this helps!

For mappings, I fluent-mapped Activity but was forced when fluent-mapping Searched to remap everything mapped on Activity .

I'm not sure I quite follow? Mind explaining what you meant with that statement ? 😄

@bytenik
Copy link
Contributor Author

bytenik commented Dec 23, 2015

Hah, I didn't realize this but it makes total sense. FWIW docs on how to do inheritance would probably avoid this for someone else. I kind of made this up as I went from various comments you've made on here and StackOverflow.

I was under the impression that if you have a parent-child relationship and the parent type has mappings set up, the child will inherit those mappings. Does that not work for fluent mappings?

@bytenik
Copy link
Contributor Author

bytenik commented Dec 23, 2015

I'm actually doing a Search/Scroll operation and the Scroll has no ability to specify types or a ConcreteTypeSelector. As a result it still fails.

@Mpdreamz
Copy link
Member

Types should inherit mappings when you call MapFromAttributes() and call .Map() on each type individually. Mind sharing some code (opening a new issue even better) ?

https://nest.azurewebsites.net/nest/search/basics.html has some docs on the covariance albeit very terse 😄

The Scroll() might be a total oversight on our part will investigate!

@bytenik
Copy link
Contributor Author

bytenik commented Dec 23, 2015

The following is the mapping logic called on a CreateIndexDescriptor. I used ActorCreated here just because it was earlier in my code. I use CommonForActivity right now because I had to keep repeating the same basic mappings.

                .AddMapping<Activity>(m => m
                    .Dynamic(false)
                    .Properties(props => CommonForActivity(props)
                    )
                )
                .AddMapping<ActorCreated>(m => m
                    .Dynamic(false)
                    .Properties(props => CommonForActivity(props)
                        .String(s => s
                            .Name(x => x.CreatedIdentity)
                            .Index(FieldIndexOption.NotAnalyzed)
                        )
                    )
                )

CommonForActivity:

static PropertiesDescriptor<T> CommonForActivity<T>(PropertiesDescriptor<T> existing)
    where T : Activity
{
    return existing.Date(d => d
            .Name(x => x.Timestamp)
            .Index()
        );
}

@Mpdreamz
Copy link
Member

This is expected behaviour, it would be hard for us to stitch two mappings together I think, abstracting too shared methods is the way to go.

Open to suggestions though!

@bytenik
Copy link
Contributor Author

bytenik commented Dec 23, 2015

👍 Nah that's fine, just wanted to verify. I agree stitching would be strange, it just wasn't clear from the docs.

The scroll thing is unfortunately a major blocker for me. If you don't have time to fix this ASAP, can you point me in the right direction so I can fix and pull request?

@Mpdreamz
Copy link
Member

Mpdreamz commented Jan 6, 2016

This is now also implemented in 2.0

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

No branches or pull requests

3 participants