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

fix and document annotation module #1281

Open
xenoterracide opened this issue Mar 22, 2021 · 2 comments
Open

fix and document annotation module #1281

xenoterracide opened this issue Mar 22, 2021 · 2 comments
Labels

Comments

@xenoterracide
Copy link

xenoterracide commented Mar 22, 2021

seems like annotation support is hot garbage, mostly it doesn't work as suggested it should (and isn't documented), and it requires you to duplicate every single annotation provided by an upstream framework. These days annotations are pretty much the #1 way that frameworks use to generate behavior, so not having fully working, fully documented first-class support for them doesn't really work (JPA support notwithstanding, at this exact second I was thinking of switching something over for Picocli, and realized what a pain.)

All we should generally have to do is annotate as normal, possibly adding an additional annotation saying where to put them.... in general though I think this entire mess needs to be rethought as to avoid having to have mylib:immutables, e.g. javax.validation:immutables, picocli:immutables just to provide a copy of annotations for every single upstream annotation.

at a thought, I would think that all you might need to add for non-repeatable's is a concrete (non-meta) annotation to say these go on a different generated thing, like the field. Maybe

// I'm changing the access type here from below
@InjectAnnotation( type = Access.class, dest = InjectionAnnotation.Where.Field )
@Access(AccessType.FIELD) 
CountrySubdivisionCode getCode( );

and if you aren't moving it no need to specify an additional annotation.

At the end of this specialized Jackson support shouldn't be necesary.

from comment...


package com.mckesson.dex.annotation.jpa.immutables;

import javax.persistence.AccessType;
import org.immutables.annotate.InjectAnnotation;

@InjectAnnotation(type= javax.persistence.Access.class, target = { InjectAnnotation.Where.ACCESSOR, InjectAnnotation.Where.FIELD })
public @interface Access {

    /**
     * (Required)  Specification of field- or property-based access.
     */
    AccessType value();
}

given this entity:

package com.mckesson.dex.model.principal;

import java.io.Serializable;
import javax.persistence.AccessType;
import javax.persistence.EnumType;
import javax.persistence.ForeignKey;
import javax.persistence.Index;
import javax.persistence.Table;
import javax.validation.constraints.NotNull;
import com.mckesson.dex.annotation.jpa.immutables.Access;
import com.mckesson.dex.annotation.jpa.immutables.Cache;
import com.mckesson.dex.annotation.jpa.immutables.Cacheable;
import com.mckesson.dex.annotation.jpa.immutables.Column;
import com.mckesson.dex.annotation.jpa.immutables.Entity;
import com.mckesson.dex.annotation.jpa.immutables.Enumerated;
import com.mckesson.dex.annotation.jpa.immutables.Id;
import com.mckesson.dex.annotation.jpa.immutables.JoinTable;
import com.mckesson.dex.annotation.jpa.immutables.ManyToOne;
import org.hibernate.annotations.CacheConcurrencyStrategy;
import org.immutables.serial.Serial;
import org.immutables.value.Value;

@Entity
@Cacheable
@Value.Immutable
@Serial.Version( 1L )
@Access(AccessType.FIELD)
@Table(name = "country_subdivision")
@Cache(usage = CacheConcurrencyStrategy.READ_ONLY)
public interface CountrySubdivision extends Serializable {

    @ManyToOne(optional = false)
    @JoinTable(
        name = "jurisdiction_country_subdivision",
        indexes = @Index(name = "idx_jcs_uq", columnList = "jurisdiction,country_subdivision", unique = true),
        joinColumns = @javax.persistence.JoinColumn(name = "country_subdivision"),
        foreignKey = @ForeignKey(name = "mac_juris_juris_fk"),
        inverseJoinColumns = @javax.persistence.JoinColumn(name = "jurisdiction")
    )
    Jurisdiction getJurisdiction();

    @Id
    @NotNull
    @Access(AccessType.PROPERTY)
    @Enumerated(EnumType.STRING)
    @Column(name = "code", length = 3, nullable = false, insertable = false, updatable = false)
    public CountrySubdivisionCode getCode( );
}

it generated this code

package com.mckesson.dex.model.principal;

import com.google.common.base.MoreObjects;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.annotations.Var;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import javax.annotation.CheckReturnValue;
import javax.annotation.Nullable;
import javax.annotation.ParametersAreNonnullByDefault;
import javax.annotation.concurrent.Immutable;
import javax.annotation.concurrent.NotThreadSafe;
import javax.persistence.Access;
import javax.persistence.Cacheable;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Enumerated;
import javax.persistence.Id;
import javax.persistence.JoinTable;
import javax.persistence.ManyToOne;
import org.hibernate.annotations.Cache;
import org.immutables.value.Generated;

/**
 * Immutable implementation of {@link CountrySubdivision}.
 * <p>
 * Use the builder to create immutable instances:
 * {@code ImmutableCountrySubdivision.builder()}.
 */
@Generated(from = "CountrySubdivision", generator = "Immutables")
@SuppressWarnings({"all"})
@ParametersAreNonnullByDefault
@javax.annotation.Generated("org.immutables.processor.ProxyProcessor")
@Immutable
@CheckReturnValue
public final class ImmutableCountrySubdivision implements CountrySubdivision {
  @Access
  private final Jurisdiction jurisdiction;
  @Column
  @Enumerated
  @Access
  private final CountrySubdivisionCode code;

  private ImmutableCountrySubdivision(
      Jurisdiction jurisdiction,
      CountrySubdivisionCode code) {
    this.jurisdiction = jurisdiction;
    this.code = code;
  }

  /**
   * @return The value of the {@code jurisdiction} attribute
   */
  @Entity@ManyToOne@JoinTable@Cacheable@Cache@Access
  @Override
  public Jurisdiction getJurisdiction() {
    return jurisdiction;
  }

  /**
   * @return The value of the {@code code} attribute
   */
  @Column@Entity@Cacheable@Enumerated@Cache@Id@Access
  @Override
  public CountrySubdivisionCode getCode() {
    return code;
  }

  /**
   * Copy the current immutable object by setting a value for the {@link CountrySubdivision#getJurisdiction() jurisdiction} attribute.
   * A shallow reference equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value A new value for jurisdiction
   * @return A modified copy of the {@code this} object
   */
  public final ImmutableCountrySubdivision withJurisdiction(Jurisdiction value) {
    if (this.jurisdiction == value) return this;
    Jurisdiction newValue = Objects.requireNonNull(value, "jurisdiction");
    return new ImmutableCountrySubdivision(newValue, this.code);
  }

  /**
   * Copy the current immutable object by setting a value for the {@link CountrySubdivision#getCode() code} attribute.
   * A value equality check is used to prevent copying of the same value by returning {@code this}.
   * @param value A new value for code
   * @return A modified copy of the {@code this} object
   */
  public final ImmutableCountrySubdivision withCode(CountrySubdivisionCode value) {
    if (this.code == value) return this;
    CountrySubdivisionCode newValue = Objects.requireNonNull(value, "code");
    if (this.code.equals(newValue)) return this;
    return new ImmutableCountrySubdivision(this.jurisdiction, newValue);
  }

  /**
   * This instance is equal to all instances of {@code ImmutableCountrySubdivision} that have equal attribute values.
   * @return {@code true} if {@code this} is equal to {@code another} instance
   */
  @Override
  public boolean equals(@Nullable Object another) {
    if (this == another) return true;
    return another instanceof ImmutableCountrySubdivision
        && equalTo((ImmutableCountrySubdivision) another);
  }

  private boolean equalTo(ImmutableCountrySubdivision another) {
    return jurisdiction.equals(another.jurisdiction)
        && code.equals(another.code);
  }

  /**
   * Computes a hash code from attributes: {@code jurisdiction}, {@code code}.
   * @return hashCode value
   */
  @Override
  public int hashCode() {
    @Var int h = 5381;
    h += (h << 5) + jurisdiction.hashCode();
    h += (h << 5) + code.hashCode();
    return h;
  }

  /**
   * Prints the immutable value {@code CountrySubdivision} with attribute values.
   * @return A string representation of the value
   */
  @Override
  public String toString() {
    return MoreObjects.toStringHelper("CountrySubdivision")
        .omitNullValues()
        .add("jurisdiction", jurisdiction)
        .add("code", code)
        .toString();
  }

  /**
   * Creates an immutable copy of a {@link CountrySubdivision} value.
   * Uses accessors to get values to initialize the new immutable instance.
   * If an instance is already immutable, it is returned as is.
   * @param instance The instance to copy
   * @return A copied immutable CountrySubdivision instance
   */
  public static ImmutableCountrySubdivision copyOf(CountrySubdivision instance) {
    if (instance instanceof ImmutableCountrySubdivision) {
      return (ImmutableCountrySubdivision) instance;
    }
    return ImmutableCountrySubdivision.builder()
        .from(instance)
        .build();
  }

  private static final long serialVersionUID = 1L;

  /**
   * Creates a builder for {@link ImmutableCountrySubdivision ImmutableCountrySubdivision}.
   * <pre>
   * ImmutableCountrySubdivision.builder()
   *    .jurisdiction(com.mckesson.dex.model.principal.Jurisdiction) // required {@link CountrySubdivision#getJurisdiction() jurisdiction}
   *    .code(com.mckesson.dex.model.principal.CountrySubdivisionCode) // required {@link CountrySubdivision#getCode() code}
   *    .build();
   * </pre>
   * @return A new ImmutableCountrySubdivision builder
   */
  public static ImmutableCountrySubdivision.Builder builder() {
    return new ImmutableCountrySubdivision.Builder();
  }

  /**
   * Builds instances of type {@link ImmutableCountrySubdivision ImmutableCountrySubdivision}.
   * Initialize attributes and then invoke the {@link #build()} method to create an
   * immutable instance.
   * <p><em>{@code Builder} is not thread-safe and generally should not be stored in a field or collection,
   * but instead used immediately to create instances.</em>
   */
  @Generated(from = "CountrySubdivision", generator = "Immutables")
  @NotThreadSafe
  public static final class Builder {
    private static final long INIT_BIT_JURISDICTION = 0x1L;
    private static final long INIT_BIT_CODE = 0x2L;
    private long initBits = 0x3L;

    private @Nullable Jurisdiction jurisdiction;
    private @Nullable CountrySubdivisionCode code;

    private Builder() {
    }

    /**
     * Fill a builder with attribute values from the provided {@code CountrySubdivision} instance.
     * Regular attribute values will be replaced with those from the given instance.
     * Absent optional values will not replace present values.
     * @param instance The instance from which to copy values
     * @return {@code this} builder for use in a chained invocation
     */
    @CanIgnoreReturnValue 
    public final Builder from(CountrySubdivision instance) {
      Objects.requireNonNull(instance, "instance");
      jurisdiction(instance.getJurisdiction());
      code(instance.getCode());
      return this;
    }

    /**
     * Initializes the value for the {@link CountrySubdivision#getJurisdiction() jurisdiction} attribute.
     * @param jurisdiction The value for jurisdiction 
     * @return {@code this} builder for use in a chained invocation
     */
    @CanIgnoreReturnValue 
    public final Builder jurisdiction(Jurisdiction jurisdiction) {
      this.jurisdiction = Objects.requireNonNull(jurisdiction, "jurisdiction");
      initBits &= ~INIT_BIT_JURISDICTION;
      return this;
    }

    /**
     * Initializes the value for the {@link CountrySubdivision#getCode() code} attribute.
     * @param code The value for code 
     * @return {@code this} builder for use in a chained invocation
     */
    @CanIgnoreReturnValue 
    public final Builder code(CountrySubdivisionCode code) {
      this.code = Objects.requireNonNull(code, "code");
      initBits &= ~INIT_BIT_CODE;
      return this;
    }

    /**
     * Builds a new {@link ImmutableCountrySubdivision ImmutableCountrySubdivision}.
     * @return An immutable instance of CountrySubdivision
     * @throws java.lang.IllegalStateException if any required attributes are missing
     */
    public ImmutableCountrySubdivision build() {
      if (initBits != 0) {
        throw new IllegalStateException(formatRequiredAttributesMessage());
      }
      return new ImmutableCountrySubdivision(jurisdiction, code);
    }

    private String formatRequiredAttributesMessage() {
      List<String> attributes = new ArrayList<>();
      if ((initBits & INIT_BIT_JURISDICTION) != 0) attributes.add("jurisdiction");
      if ((initBits & INIT_BIT_CODE) != 0) attributes.add("code");
      return "Cannot build CountrySubdivision, some of required attributes are not set " + attributes;
    }
  }
}

I also tried code="[[*]]" and it generated it as @Accessvalue = AccessType.FIELD` with the right import, but that's not a valid annotation usage. If I do this

@InjectAnnotation(type= javax.persistence.Access.class, code="@Access([[*]])", target = { InjectAnnotation.Where.ACCESSOR, InjectAnnotation.Where.FIELD })

it generates the proper annotation on the field but leaves out the import.

version 2.8.2

Originally posted by @xenoterracide in #903 (comment)

@elucash
Copy link
Member

elucash commented Apr 17, 2021

the corrected usage for your case would be

@InjectAnnotation(type= javax.persistence.Access.class, code="([[*]])", target = { InjectAnnotation.Where.ACCESSOR, InjectAnnotation.Where.FIELD })

or

@InjectAnnotation(code="@javax.persistence.Access([[*]])", target = { InjectAnnotation.Where.ACCESSOR, InjectAnnotation.Where.FIELD })

@elucash
Copy link
Member

elucash commented Apr 17, 2021

Another consideration is to create multi-annotation injection and precondition it on the presence of the annotation itself

@InjectAnnotation(
    type = Access.class,
    target = {InjectAnnotation.Where.FIELD, InjectAnnotation.Where.ACCESSOR},
    ifPresent = true
)
@InjectAnnotation(
    type = SomeOtherAnnotation.class,
    target = {InjectAnnotation.Where.FIELD},
    ifPresent = true
)
public @interface MyAssessEtc {
}

notice that the code= is not specified, so it just copy original annotations with all their attributes. No need to create a bunch of custom annotations. In this way you can create "annotation injection library" annotation(s) and place it('em) on the type or even package (package-info.java, need to check but it will probably work too).

@Value.Immutable
@MyAccessEtc
public interface AnnotatedEntity extends Serializable {
  @Access(AccessType.FIELD)  //<-- will get copied to the field and accessor
  String getJurisdiction();
  @Access(AccessType.PROPERTY) //<-- will get copied to the field and accessor
  String getCode();
}

If you have multiple rules which can clash and inject many of the "same" or conflicting annotations, you can use InjectAnnotation.deduplicationKey to limit to a single "closest" rule.

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

No branches or pull requests

2 participants