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

custom value mappings for @Enumerated #47

Closed
lukasj opened this issue Feb 11, 2013 · 20 comments · Fixed by #463
Closed

custom value mappings for @Enumerated #47

lukasj opened this issue Feb 11, 2013 · 20 comments · Fixed by #463

Comments

@lukasj
Copy link
Contributor

lukasj commented Feb 11, 2013

In mapping a pre-existing schema, it is rarely possible to map a given set of constants using an @Enumerated:

  • Ordinals are non-sequential
  • Names are incompatible with Java Enum Constants
  • Names cannot be aliased to something more useful or conventional for Java Naming standards

Limitation is because specification states the name() will be used of an enum.
A typical example: Database uses 'N/A', which can never be mapped to an enum. Intuition will let us look for a solution to map this to 'NOT_AVAILABLE', both a valid name for an Enum Constant, and also conventional naming.

On top of this, an enum often has a representation in the Database, and a constant has usually other additional attributes that could be useful.
Better than using the name() of an @Enumerated, we should map the enumerated similar as an @entity, and use the @id.

@lukasj
Copy link
Contributor Author

lukasj commented Feb 11, 2013

@glassfishrobot Commented
Reported by jd3714att

@lukasj
Copy link
Contributor Author

lukasj commented Nov 26, 2014

@glassfishrobot Commented
jd3714att said:
Would this not be more elegant than using a @converter to solve the problem.

@Enumerated
public enum Language {
  ENGLISH_US("en-US"),
  ENGLISH_BRITISH("en-BR"),
  FRENCH("fr"),
  FRENCH_CANADIAN("fr-CA");
  @ID
  private String code;
  @Column(name="DESCRIPTION")
  private String description;

  Language(String code) {
    this.code = code;
  }

  public String getCode() {
    return code;
  }

  public String getDescription() {
    return description;
  }
}

@lukasj
Copy link
Contributor Author

lukasj commented Dec 16, 2014

@glassfishrobot Commented
hajolemcke said:
This looks great. And it would solve the problem to have a separate converter for each enumeration because a generic converter can not be build type save.

@lukasj
Copy link
Contributor Author

lukasj commented Apr 28, 2015

@glassfishrobot Commented
rbygrave said:
I don't think @column(name="DESCRIPTION") is required. The only requirement is to mark the field (as you have done with @id) or mark a getter method that returns the value to be stored in the DB.

So instead annotate the getCode() method like:

@DbValue
public String getCode() { return code; }

or

@Id
public String getCode() { return code; }

Another alternative would be to create an annotation that acts just like @XmlEnumValue does for xml mapping ... so create a @DbEnumValue such that:

@Enumerated
public enum Language {
  @DbEnumValue("en-US")
  ENGLISH_US,

  @DbEnumValue("en-BR")
  ENGLISH_BRITISH,

  @DbEnumValue("fr")
  FRENCH,

  @DbEnumValue("fr-CA")
  FRENCH_CANADIAN;

@lukasj
Copy link
Contributor Author

lukasj commented Nov 23, 2015

@glassfishrobot Commented
pbenedict said:
This kind of custom transformation can now be expressed with Converters. I've done it myself for several situations. Converters, not enums that transform themselves into a DB value, is the correct way to separate out model code from integration code.

@lukasj
Copy link
Contributor Author

lukasj commented Nov 23, 2015

@glassfishrobot Commented
rbygrave said:

Converters, not enums that transform ...
For me I'd say its a balance between pragmatism and 'purity'. The @DbValue annotation on a method approach mirrors a similar approach Jackson takes for JSON and personally I think it represents significantly less work than writing a Converter. So yes, I'll agree to disagree with you there on pragmatism over purity grounds.

@lukasj
Copy link
Contributor Author

lukasj commented Nov 23, 2015

@glassfishrobot Commented
pbenedict said:
Yes, it is less work. The approach you mention I have used for many years; finally turned away from it for several reasons. It began to breakdown the moment I had many ways of encoding an Enum for multiple systems: perhaps an existing database requires code XYZ but a new technology requires code ABC. All those things are possible which is why I think the "purity" aspect (integration code) is a better design because it frees you up from coupling concerns.

@lukasj
Copy link
Contributor Author

lukasj commented Nov 23, 2015

@glassfishrobot Commented
pbenedict said:
In closing, I should mention that you could create a nearly-universal Enum-to-String attribute converter that accomplishes your reusability requirements. You already have the value in your example code; just move it into the enum constructor; provide a standard interface that exposes the code; finally do the conversion in the Converter.

Example:

public interface Coded {
  String code();
}

@Enumerated
public enum Language implements Coded {
  ENGLISH_US("en-US"),
  ENGLISH_BRITISH("en-BR"),
  FRENCH("fr"),
  FRENCH_CANADIAN("fr-CA");

  private String code;

  private Language(String code) {
      this.code = code;
  }

  // To database: converter uses this
  // From database: converter loops values() and compares DB column against code()  
  public String code() { return code; }
}

// Reusable universal logic to convert back and forth public abstract class YourAbstractCodedConverter<X extends Enum<X> & Coded> implements AttributeConverter<X,String> { 
}

// Strongly typed for JPA public class YourLocaleCodedConverter extends YourAbstractCodedConverter<Language> {
  // empty }

@lukasj
Copy link
Contributor Author

lukasj commented Nov 24, 2015

@glassfishrobot Commented
pbenedict said:
@rbygrave, I reverse my opinion. I went to demonstrate one thing and have concluded another. After writing out the code above, it's clear the code is boilerplate and more complex than is necessary. Thus, I now support this ticket and have voted it up. This should be allowed via an annotation.

The pattern above clearly keeps out the integration (JPA) from the model code. This is very important when the Enum is part of a public API (where no such dependency should be required), but this is not always everyone's goal. Sometimes Enums are dedicated to JPA ... or, as you said, you don't prefer to keep such a "pure" separation of concerns.

@lukasj
Copy link
Contributor Author

lukasj commented Nov 24, 2015

@glassfishrobot Commented
jd3714att said:
@pbenedict, in regards to "pure" separation of concerns, who says that separation is not there? We only have Java Identifier (the enum instances) vs Internal Code, which neither has to be the actual vendor specific backing presentation. That separation should still be possible, although this is not what I intended. So to that regards I would also avoid using names that include "Xml", or "Db".

@lukasj
Copy link
Contributor Author

lukasj commented Mar 1, 2016

@glassfishrobot Commented
rbygrave said:
FYI

Just for information purposes I have implemented this in my ORM.

All that is required is to annotate the method used to return the value stored in the database (the getCode() method in the example below). For my ORM I use @DbEnumValue. The ORM loops the enum entries and builds its map of DB to Enum values to there is no need for anything else (no other annotations required etc).

The original example becomes:

public enum Language {

  ENGLISH_US("en-US"),
  ENGLISH_BRITISH("en-BR"),
  FRENCH("fr"),
  FRENCH_CANADIAN("fr-CA");

  private String code;

  Language(String code) {
    this.code = code;
  }

  @DbEnumValue  
  public String getCode() {
    return code;
  }

}

Note: The @DbEnumValue annotation has a storage() attribute that can be either DbEnumType.VARCHAR or DbEnumType.INTEGER.

Cheers, Rob.

@lukasj
Copy link
Contributor Author

lukasj commented Mar 16, 2016

@glassfishrobot Commented
jaguild said:
Work in this area just cannot happen fast enough; this is a huge omission even when you do control the schema.

Our schemas tend to have FK constraints from data tables to "enum" tables that contain fixed control data. I don't even care if we have to write some boilerplate in the style that @pbenedict suggested above...that seems OK to me, but I'd like to add that any improvements to enum support should allow mapping an enum to these kinds of dedicated control data tables:

@Enumerated(table="LANGUAGE_TYPE")
public enum Language implements Coded {
  ENGLISH_US("en-US"),
  ENGLISH_BRITISH("en-BR"),
  FRENCH("fr"),
  FRENCH_CANADIAN("fr-CA");

  @EnumeratedColumn(name="code")
  private String code;

  private Language(String code) {
      this.code = code;
  }

  // To database: converter uses this
  // From database: converter loops values() and compares DB column against code()  
  public String code() { return code; }
}

This still requires us to duplicate the control data values in the table as enum constants and keep the two in sync, but at least it gives us the strong typing of values found in the LANGUAGE_TYPE table. I think it could otherwise work like @pbenedict implies in his code sample.

What would be really awesome is if the JPA provider could scan the table on startup and somehow update the enum constant code values with the data from the @EnumeratedColumn to avoid having to duplicate and hardcode the same values from the database table into in the Java enum type itself. I don't know if that last idea is possible due to how enum types are implemented in the first place.

But, the provider could at least maintain an internal map of enum type constants to actual rows on a default enum.getCode() == LANGUAGE_TYPE.code policy so FK column updates could occur correctly when an entity property that is mapped to an @Enumerated type is changed. Regarding a point made by @jd3714att in the original issue description, some kind of optional name mapper could be specified to provide a different matching policy allowing the provider to handle dealing with values found in the target table that violate Java enum constant naming rules.

@lukasj
Copy link
Contributor Author

lukasj commented Apr 27, 2016

@glassfishrobot Commented
neilstockton said:
What if the enum is part of some other library? With the proposal the user would have no way of defining to persist something other than the ordinal/string even though methods may exist. Any "solution" needs to have an orm.xml alternative.

@lukasj
Copy link
Contributor Author

lukasj commented Apr 27, 2016

@glassfishrobot Commented
pbenedict said:
Neil, see all my comments above in which I first objected to this feature. In my objection, I had such a use case in mind that is like yours. However, for those who actually own their enum and don't need that level of abstraction, which @rbygrave was pointing out, it is overkill.

@lukasj
Copy link
Contributor Author

lukasj commented May 20, 2016

@glassfishrobot Commented
neilstockton said:
Well I still "object" (FWIW) because it is unlikely to happen in that way due to the fact that current enum persistence is specified in the class holding the enum via @Enumerated (and not on the enum).

To support persistence of return value of method (on the enum, like above examples) it easily doable to extend the current @Enumerated to add a new EnumType and add an attribute for the method name (on the enum that will be persisted). That way you also can have XML specification of the method to use (rather than the above that only work with annotations, and not everybody thinks that's a "good idea").

@Enumerated(value=EnumType.METHOD, methodName="getCode")
Language lang;

@lukasj
Copy link
Contributor Author

lukasj commented May 5, 2017

@glassfishrobot Commented
This issue was imported from java.net JIRA JPA_SPEC-47

@lukasj
Copy link
Contributor Author

lukasj commented Aug 31, 2018

@gavinking gavinking changed the title Usability of @Enumerated and @MapKeyEnumerated custom value mappings for @Enumerated Aug 13, 2023
@gavinking
Copy link
Contributor

gavinking commented Aug 13, 2023

So this very long discussion is quite old now, and doesn't seem to have ever reached much of a firm conclusion.
So let me give my take on this.

As pointed out by others, there's quite a lot of overlap with AttributeConverter here. However, there's one thing that AttributeConverters aren't really set up to handle, and that's DDL generation. For a JPA @Enumerated field, we want to automatically generate a CHECK constraint or a database-native ENUM type. To do that, we need to know upfront what values are possible.

So there's indeed a good case to me made for allowing explicit "remapping" of enum values.

Just off the top of my head, I can think of two ways to do this. The first is the one that seems to have been proposed here:

enum Status {
    OPEN, CLOSED, CANCELLED;

    @EnumeratedValue
    int toInt() {
        switch (this) {
            case OPEN: return 0;
            case CLOSED: return 1;
            case CANCELLED: return -1;
            default: throw new RuntimeException();
        }
    }
}

The problem with this is it makes the enumerated values implicitly dynamic. There's no way to force toInt() to return the same int value for a given enum value.

But there's a different way, which was mentioned in this comment:

enum Status {
    @EnumeratedIntValue(0)
    OPEN, 
    @EnumeratedIntValue(1)
    CLOSED, 
    @EnumeratedIntValue(-1)
    CANCELLED;
}

That's shorter, and more "correct". It's the solution I've already proposed that we implement in Hibernate.

The downside to this approach is that we would need two annotations (@EnumeratedIntValue and @EnumeratedStringValue or whatever).

Anyway, I think the second approach would be thing worth having in JPA.

@gavinking
Copy link
Contributor

gavinking commented Aug 13, 2023

There's no way to force toInt() to return the same int value for a given enum value.

Well, excuse me, that was wrong: actually @jaguild above already proposed to stick the annotation on a final field of the enum, like this:

enum Status {
    OPEN(0), CLOSED(1), CANCELLED(-1);

    @EnumeratedValue
    final int intValue;

    Status(int intValue) {
        this.intValue = intValue;
    }
}

And that works too. The advantage is that only one annotation is needed; the disadvantage is that I need to add a field to my enum. Not sure which is better.

@gavinking
Copy link
Contributor

And that works too.

In #463 I propose this solution. I've also reworked the section on @Enumerated to actually nail down what STRING and ORDINAL mean, since their semantics were left essentially undefined by the spec!

gavinking added a commit to gavinking/jpa-api that referenced this issue Aug 13, 2023
gavinking added a commit to gavinking/jpa-api that referenced this issue Aug 13, 2023
@lukasj lukasj added this to To do in 3.2.0 via automation Aug 21, 2023
3.2.0 automation moved this from To do to Done Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
3.2.0
Done
Development

Successfully merging a pull request may close this issue.

2 participants