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

Add a way to set the Kind from Embeddable #164 #165

Closed
wants to merge 1 commit into from

Conversation

athieriot
Copy link

@athieriot athieriot commented Aug 7, 2017

To explain a bit more about issue #164, we have this use case where we use a Wrapper that embed a generic
object in one of its field.

@Entity
public class Wrapper {

   @Embedded
   private Interface content;

   ...
}

The content can be of any subtype of Interface (some of them will be
outside of our control) and the Kind value can change with any of
these subtypes.

This Pull Request allow a developer to provide an Embeddable class
that will override the Kind of the enclosing Entity.

It will look like this:

@Embeddable
@OverrideKind(kind = "mycontent")
public class MyContent implement Interface {

   ...
}

We have this use case where we use a Wrapper that embed a generic
object in one of its field.

```
@entity
public class Wrapper {

   @Embedded
   private Interface content;

   ...
}
```

The content can be of any subtype of Interface (some of them will be
outside of our control) and the ```Kind``` value can change with any of
these subtypes.

This Pull Request allow a developer to provide an ```Embeddable``` class
that will override the ```Kind``` of the enclosing ```Entity```.

It will look like this:

```
@embeddable
@OverrideKind(kind = "mycontent")
public class MyContent implement Interface {

   ...
}
```
@sai-pullabhotla
Copy link
Owner

@athieriot - thanks for submitting this Pull Request. Have a question -

If you are able to annotate the Embeddable with a OverrideKind, why not annotate it as an Entity? Does the "Wrapper" have any other data elements other than an Embedded Content?

@athieriot
Copy link
Author

athieriot commented Aug 9, 2017 via email

@sai-pullabhotla
Copy link
Owner

Does something like the below works for you?

@MappedSuperClass
class Document {
    //Common Metadata fields
}

@Entity 
class MyContent1 extends Document {
    //Specific fields for MyContent1
}

@Entity 
class MyContent2 extends Document {
    //Specific fields for MyContent2
}

@athieriot
Copy link
Author

athieriot commented Aug 10, 2017 via email

@sai-pullabhotla
Copy link
Owner

@athieriot - I think you should eventually use the MappedSuperClass/Entity for your needs. This PullRequest looks like a temporary solution that addresses your specific needs. For now, I will go ahead and close this PullRequest, this means you would have to use the modified code from your forked repo. Let me know if you have any further questions or comments.

@athieriot
Copy link
Author

Thank you for your feedback, it does make sense from a library point of view.

One other thing we are especially looking at is to be able to use immutable objects.
So we might give a crack at implementing this issue: #49.

If you have any advice of if there is anything wrong with this current PR (Style, tests, doc) that you wouldn't want to see please tell !

@sai-pullabhotla
Copy link
Owner

It would be great to have support for the immutable fields within an entity. Once you have the implementation, we can work out any minor changes. Comments/JavaDoc is important. Use the default Eclipse code formatter settings. I will go ahead and close this now.

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

Successfully merging this pull request may close these issues.

2 participants