Skip to content

HHH-19584 add @CurrentTimestamp(allowMutation) #10462

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,17 @@
* this additional {@code select} never occurs.
*/
SourceType source() default SourceType.DB;

/**
* Specifies whether the value may be mutated by the application program
* during {@linkplain #event events} which do <em>not</em> trigger value
* generation.
* <p>
* For example, a field annotated
* {@code CurrentTimestamp(event=INSERT, allowMutation=true)}
* is generated when a record is inserted and may be manually mutated later.
*
* @since 7.1
*/
boolean allowMutation() default false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
public class CurrentTimestampAnnotation implements CurrentTimestamp {
private org.hibernate.generator.EventType[] event;
private org.hibernate.annotations.SourceType source;
private boolean allowMutation;

/**
* Used in creating dynamic annotation instances (e.g. from XML)
*/
public CurrentTimestampAnnotation(ModelsContext modelContext) {
this.event = new org.hibernate.generator.EventType[] {INSERT, UPDATE};
this.source = org.hibernate.annotations.SourceType.DB;
this.allowMutation = false;
}

/**
Expand All @@ -33,6 +35,7 @@ public CurrentTimestampAnnotation(ModelsContext modelContext) {
public CurrentTimestampAnnotation(CurrentTimestamp annotation, ModelsContext modelContext) {
this.event = annotation.event();
this.source = annotation.source();
this.allowMutation = annotation.allowMutation();
}

/**
Expand All @@ -41,6 +44,7 @@ public CurrentTimestampAnnotation(CurrentTimestamp annotation, ModelsContext mod
public CurrentTimestampAnnotation(Map<String, Object> attributeValues, ModelsContext modelContext) {
this.event = (org.hibernate.generator.EventType[]) attributeValues.get( "event" );
this.source = (org.hibernate.annotations.SourceType) attributeValues.get( "source" );
this.allowMutation = (Boolean) attributeValues.get( "allowMutation" );
}

@Override
Expand All @@ -66,5 +70,12 @@ public void source(org.hibernate.annotations.SourceType value) {
this.source = value;
}

@Override
public boolean allowMutation() {
return allowMutation;
}

public void allowMutation(boolean value) {
this.allowMutation = value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public class CurrentTimestampGeneration implements BeforeExecutionGenerator, OnE
public static final String CLOCK_SETTING_NAME = "hibernate.testing.clock";

private final EnumSet<EventType> eventTypes;
private final boolean allowMutation;

private final CurrentTimestampGeneratorDelegate delegate;
private static final Map<Class<?>, BiFunction<@Nullable Clock, Integer, CurrentTimestampGeneratorDelegate>> GENERATOR_PRODUCERS = new HashMap<>();
Expand Down Expand Up @@ -185,16 +186,19 @@ public class CurrentTimestampGeneration implements BeforeExecutionGenerator, OnE
public CurrentTimestampGeneration(CurrentTimestamp annotation, Member member, GeneratorCreationContext context) {
delegate = getGeneratorDelegate( annotation.source(), member, context );
eventTypes = fromArray( annotation.event() );
allowMutation = annotation.allowMutation();
}

public CurrentTimestampGeneration(CreationTimestamp annotation, Member member, GeneratorCreationContext context) {
delegate = getGeneratorDelegate( annotation.source(), member, context );
eventTypes = INSERT_ONLY;
allowMutation = false;
}

public CurrentTimestampGeneration(UpdateTimestamp annotation, Member member, GeneratorCreationContext context) {
delegate = getGeneratorDelegate( annotation.source(), member, context );
eventTypes = INSERT_AND_UPDATE;
allowMutation = false;
}

private static CurrentTimestampGeneratorDelegate getGeneratorDelegate(
Expand All @@ -216,24 +220,27 @@ static CurrentTimestampGeneratorDelegate getGeneratorDelegate(
context.getDatabase().getDialect(),
basicValue.getMetadata()
);
final Clock baseClock = context.getServiceRegistry()
.requireService( ConfigurationService.class )
.getSetting( CLOCK_SETTING_NAME, value -> (Clock) value );
final Key key = new Key( propertyType, baseClock, size.getPrecision() == null ? 0 : size.getPrecision() );
final CurrentTimestampGeneratorDelegate delegate = GENERATOR_DELEGATES.get( key );
final Clock baseClock =
context.getServiceRegistry().requireService( ConfigurationService.class )
.getSetting( CLOCK_SETTING_NAME, value -> (Clock) value );
final Key key =
new Key( propertyType, baseClock,
size.getPrecision() == null ? 0 : size.getPrecision() );
final var delegate = GENERATOR_DELEGATES.get( key );
if ( delegate != null ) {
return delegate;
}
final BiFunction<@Nullable Clock, Integer, CurrentTimestampGeneratorDelegate> producer = GENERATOR_PRODUCERS.get( key.clazz );
if ( producer == null ) {
return null;
else {
final var producer = GENERATOR_PRODUCERS.get( key.clazz );
if ( producer == null ) {
return null;
}
else {
final var generatorDelegate = producer.apply( key.clock, key.precision );
final var old = GENERATOR_DELEGATES.putIfAbsent( key, generatorDelegate );
return old != null ? old : generatorDelegate;
}
}
final CurrentTimestampGeneratorDelegate generatorDelegate = producer.apply( key.clock, key.precision );
final CurrentTimestampGeneratorDelegate old = GENERATOR_DELEGATES.putIfAbsent(
key,
generatorDelegate
);
return old != null ? old : generatorDelegate;
case DB:
return null;
default:
Expand All @@ -255,6 +262,11 @@ public EnumSet<EventType> getEventTypes() {
return eventTypes;
}

@Override
public boolean allowMutation() {
return allowMutation;
}

Comment on lines +265 to +269
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yrodiere This is the only significant thing here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks, but should we merge this immediately, or do you want someone to have a look at the "manually set the value for inserts" case too?
Asking because that might change the exposed API, if we decide its' better (simpler) to expose a single allowOverride() attribute controlling whether people can manually set the value for both inserts and updates.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks, but should we merge this immediately, or do you want someone to have a look at the "manually set the value for inserts" case too?

Well, nobody actually asked for that yet. (Except you 😉)

Asking because that might change the exposed API, if we decide its' better (simpler) to expose a single allowOverride() attribute controlling whether people can manually set the value for both inserts and updates.

It's not linked directly to inserts vs updates, it depends on whether the client can override generation. So I think it's two different things.

All that said, we can't merge this without a test.

@Override
public Object generate(SharedSessionContractImplementor session, Object owner, Object currentValue, EventType eventType) {
return delegate.generate();
Expand All @@ -276,7 +288,8 @@ public String[] getReferencedColumnValues(Dialect dialect) {
}

interface CurrentTimestampGeneratorDelegate {
// Left out the Generator params, they're not used anyway. Since this is purely internal, this can be changed if needed
// Left out the Generator params, they're not used anyway.
// Since this is purely internal, this can be changed if needed
Object generate();
}

Expand Down