Skip to content

Commit

Permalink
GH-207 - Fix callback generation for aggregates.
Browse files Browse the repository at this point in the history
In case of annotation based callbacks to flip the is new state of an aggregate we currently implement MutablePersistable but do not actually generate an implementation for the method declared by that interface. This doesn't seem to cause any problems currently as nobody is ever invoking that method (as the flipping is implemented through the annotation-based callbacks).

We now avoid implementing MutablePersistable for on-entity callbacks and fall back to only implementing Persistable, a getId() and isNew() method (the latter already generated before).
  • Loading branch information
odrotbohm committed Dec 23, 2023
1 parent ba7479e commit 1fffe61
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Arrays;
import java.util.stream.Stream;

import org.jmolecules.spring.data.MutablePersistable;
import org.junit.jupiter.api.Test;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.data.domain.Persistable;
Expand All @@ -43,7 +44,9 @@ class JMoleculesSpringDataJpaTests {

@Test // #28
void implementsPersistable() {
assertThat(Persistable.class).isAssignableFrom(SampleAggregate.class);
assertThat(new SampleAggregate(id))
.isNotInstanceOf(MutablePersistable.class)
.isInstanceOf(Persistable.class);
}

@Test // #28
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import javax.persistence.PrePersist;
import javax.persistence.Transient;

import org.jmolecules.spring.data.MutablePersistable;
import org.junit.jupiter.api.Test;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.data.domain.Persistable;
Expand All @@ -44,7 +45,9 @@ class JMoleculesSpringDataJpaTests {

@Test // #28
void implementsPersistable() {
assertThat(Persistable.class).isAssignableFrom(SampleAggregate.class);
assertThat(new SampleAggregate(id))
.isNotInstanceOf(MutablePersistable.class)
.isInstanceOf(Persistable.class);
}

@Test // #28
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
@RequiredArgsConstructor(staticName = "of")
class PersistableImplementor {

static final String GET_ID_METHOD = "getId";
static final String IS_NEW_METHOD = "isNew";
static final String MARK_NOT_NEW_METHOD = "__jMolecules__markNotNew";
static final String IS_NEW_FIELD = "__jMolecules__isNew";
Expand All @@ -73,40 +74,56 @@ JMoleculesType implementPersistable(JMoleculesType type) {
ElementMatcher<? super InDefinedShape> isIdField = candidate -> !candidate.getName()
.equals(field.getName());

return type.implement(MutablePersistable.class, type.getTypeDescription(), field.getType())
.mapBuilder(this::generateCallbacks)
.mapBuilder(it -> !it.hasField(named(IS_NEW_FIELD)), this::generateIsNewField)
return type.map(it -> implementPersistable(it, field))
.mapBuilder(__ -> options.hasCallbackAnnotations(), this::generateAnnotationCallbacks)
.mapBuilder(it -> !it.hasMethod(isEquals()), it -> generateEquals(it, isIdField))
.mapBuilder(it -> !it.hasMethod(isHashCode()), it -> generateHashCode(it, isIdField))
.mapBuilder(it -> !it.hasMethod(hasMethodName(IS_NEW_METHOD)), this::generateIsNewMethod);
.mapBuilder(it -> !it.hasMethod(isHashCode()), it -> generateHashCode(it, isIdField));

}).orElse(type);
}

private Builder<?> generateCallbacks(Builder<?> builder) {
private JMoleculesType implementPersistable(JMoleculesType type, InDefinedShape field) {

if (options.callbackInterface != null) {
Generic fieldType = field.getType();

builder = builder.defineMethod(MARK_NOT_NEW_METHOD, void.class, Visibility.PUBLIC)
.intercept(FieldAccessor.ofField(IS_NEW_FIELD).setsValue(false))
.require(createCallbackComponent(builder.toTypeDescription()));
}
if (options.hasCallbackInterface()) {

if (type.isAssignableTo(MutablePersistable.class)) {
return type;
}

type = type.implement(MutablePersistable.class, type.getTypeDescription(), fieldType)
.mapBuilder(it -> it.defineMethod(MARK_NOT_NEW_METHOD, void.class, Visibility.PUBLIC)
.intercept(FieldAccessor.ofField(IS_NEW_FIELD).setsValue(false))
.require(createCallbackComponent(it.toTypeDescription())));
} else {

if (type.isAssignableTo(Persistable.class)) {
return type;
}

if (options.callbackAnnotations.length > 0) {
builder = new LifecycleMethods(builder, options.callbackAnnotations)
.apply(__ -> Advice.to(NotNewSetter.class),
() -> FieldAccessor.ofField(IS_NEW_FIELD).setsValue(false));
type = type.implement(Persistable.class, fieldType);
}

return builder;
return type
.mapBuilder(it -> !it.hasField(named(IS_NEW_FIELD)), this::generateIsNewField)
.mapBuilder(it -> !it.hasMethod(hasMethodName(GET_ID_METHOD)), it -> generateGetIdMethod(it, field))
.mapBuilder(it -> !it.hasMethod(hasMethodName(IS_NEW_METHOD)), this::generateIsNewMethod);
}

private Builder<?> generateAnnotationCallbacks(Builder<?> builder) {

return new LifecycleMethods(builder, options.getCallbackAnnotations())
.apply(__ -> Advice.to(NotNewSetter.class),
() -> FieldAccessor.ofField(IS_NEW_FIELD).setsValue(false));
}

private Builder<?> generateIsNewField(Builder<?> builder) {

// Add isNew field
builder = builder.defineField(IS_NEW_FIELD, boolean.class, Visibility.PRIVATE)
.value(true)
.annotateField(getAnnotation(options.isNewPropertyAnnotation));
.annotateField(getAnnotation(options.getIsNewPropertyAnnotation()));

// Tweak constructors to set the newly introduced field to true.
Junction<MethodDescription> isConstructor = ElementMatchers.isConstructor();
Expand All @@ -117,6 +134,12 @@ private Builder<?> generateIsNewField(Builder<?> builder) {
.visit(Advice.to(IsNewForwarder.class).on(isWither));
}

private Builder<?> generateGetIdMethod(Builder<?> builder, InDefinedShape field) {

return builder.defineMethod(GET_ID_METHOD, field.getType(), Visibility.PUBLIC)
.intercept(FieldAccessor.ofField(field.getName()));
}

private Builder<?> generateIsNewMethod(Builder<?> builder) {

return builder.defineMethod(IS_NEW_METHOD, boolean.class, Visibility.PUBLIC)
Expand All @@ -139,7 +162,7 @@ private Builder<?> generateHashCode(Builder<?> builder, ElementMatcher<? super I
private DynamicType createCallbackComponent(TypeDescription typeDescription) {

Generic callbackType = Generic.Builder
.parameterizedType(new TypeDescription.ForLoadedType(options.callbackInterface), typeDescription).build();
.parameterizedType(new TypeDescription.ForLoadedType(options.getCallbackInterface()), typeDescription).build();

return new ByteBuddy(ClassFileVersion.JAVA_V8)
.subclass(callbackType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
*/
package org.jmolecules.bytebuddy;

import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.Value;
import lombok.With;

import java.lang.annotation.Annotation;
Expand All @@ -26,16 +27,27 @@
/**
* @author Oliver Drotbohm
*/
@With
@AllArgsConstructor
@RequiredArgsConstructor(staticName = "of")
@Value
@AllArgsConstructor(access = AccessLevel.PRIVATE)
class PersistableOptions {

final @NonNull Class<? extends Annotation> isNewPropertyAnnotation;
Class<?> callbackInterface;
@NonNull Class<? extends Annotation> isNewPropertyAnnotation;
@With Class<?> callbackInterface;
Class<? extends Annotation>[] callbackAnnotations;

@SuppressWarnings("unchecked") Class<? extends Annotation>[] callbackAnnotations = (Class<? extends Annotation>[]) Array
.newInstance(Class.class, 0);
@SuppressWarnings("unchecked")
public static PersistableOptions of(Class<? extends Annotation> isNewPropertyAnnotation) {
return new PersistableOptions(isNewPropertyAnnotation, null, (Class<? extends Annotation>[]) Array
.newInstance(Class.class, 0));
}

boolean hasCallbackInterface() {
return callbackInterface != null;
}

boolean hasCallbackAnnotations() {
return callbackAnnotations.length > 0;
}

@SafeVarargs
public final PersistableOptions withCallbackAnnotations(Class<? extends Annotation>... callbackAnnotations) {
Expand Down

0 comments on commit 1fffe61

Please sign in to comment.