Skip to content

Omit sequence generation for non-new entities and entities with provided identifiers #2005

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Omit sequence generation for non-new entities and entities with provided identifiers #2005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,6 @@ private <T> RootAggregateChange<T> createInsertChange(T instance) {
}

private <T> RootAggregateChange<T> createUpdateChange(EntityAndPreviousVersion<T> entityAndVersion) {

RootAggregateChange<T> aggregateChange = MutableAggregateChange.forSave(entityAndVersion.entity,
entityAndVersion.version);
new RelationalEntityUpdateWriter<T>(context).write(entityAndVersion.entity, aggregateChange);
Expand Down
115 changes: 65 additions & 50 deletions .../main/java/org/springframework/data/jdbc/core/mapping/IdGeneratingBeforeSaveCallback.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.Map;
import java.util.Optional;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.data.jdbc.repository.config.AbstractJdbcConfiguration;
Expand All @@ -16,59 +17,73 @@
import org.springframework.util.Assert;

/**
* Callback for generating ID via the database sequence. By default, it is registered as a
* bean in {@link AbstractJdbcConfiguration}
* Callback for generating ID via the database sequence. By default, it is registered as a bean in
Copy link
Member

Choose a reason for hiding this comment

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

Actually, we do want to avoid dependency inversions, this class should ideally now know anything about AbstractJdbcConfiguration.

* {@link AbstractJdbcConfiguration}
*
* @author Mikhail Polivakha
*/
public class IdGeneratingBeforeSaveCallback implements BeforeSaveCallback<Object> {

private static final Log LOG = LogFactory.getLog(IdGeneratingBeforeSaveCallback.class);

private final RelationalMappingContext relationalMappingContext;
private final Dialect dialect;
private final NamedParameterJdbcOperations operations;

public IdGeneratingBeforeSaveCallback(
RelationalMappingContext relationalMappingContext,
Dialect dialect,
NamedParameterJdbcOperations namedParameterJdbcOperations
) {
this.relationalMappingContext = relationalMappingContext;
this.dialect = dialect;
this.operations = namedParameterJdbcOperations;
}

@Override
public Object onBeforeSave(Object aggregate, MutableAggregateChange<Object> aggregateChange) {

Assert.notNull(aggregate, "The aggregate cannot be null at this point");

RelationalPersistentEntity<?> persistentEntity = relationalMappingContext.getPersistentEntity(aggregate.getClass());
Optional<SqlIdentifier> idSequence = persistentEntity.getIdSequence();

if (dialect.getIdGeneration().sequencesSupported()) {

if (persistentEntity.getIdProperty() != null) {
idSequence
.map(s -> dialect.getIdGeneration().createSequenceQuery(s))
.ifPresent(sql -> {
Long idValue = operations.queryForObject(sql, Map.of(), (rs, rowNum) -> rs.getLong(1));
PersistentPropertyAccessor<Object> propertyAccessor = persistentEntity.getPropertyAccessor(aggregate);
propertyAccessor.setProperty(persistentEntity.getRequiredIdProperty(), idValue);
});
}
} else {
Copy link
Member

Choose a reason for hiding this comment

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

There are other files using space formatting , such as IdGeneratingBeforeSaveCallbackTest. As regular contributor I bet you can do better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I've imported the spring data code formatter into Intelij. I'll double-check my formatting, thanks

if (idSequence.isPresent()) {
LOG.warn("""
It seems you're trying to insert an aggregate of type '%s' annotated with @TargetSequence, but the problem is RDBMS you're
working with does not support sequences as such. Falling back to identity columns
"""
.formatted(aggregate.getClass().getName())
);
}
}

return aggregate;
}
private static final Log LOG = LogFactory.getLog(IdGeneratingBeforeSaveCallback.class);

private final RelationalMappingContext relationalMappingContext;
private final Dialect dialect;
private final NamedParameterJdbcOperations operations;

public IdGeneratingBeforeSaveCallback(RelationalMappingContext relationalMappingContext, Dialect dialect,
NamedParameterJdbcOperations namedParameterJdbcOperations) {
this.relationalMappingContext = relationalMappingContext;
this.dialect = dialect;
this.operations = namedParameterJdbcOperations;
}

@Override
public Object onBeforeSave(Object aggregate, MutableAggregateChange<Object> aggregateChange) {

Assert.notNull(aggregate, "The aggregate cannot be null at this point");

RelationalPersistentEntity<?> persistentEntity = relationalMappingContext.getPersistentEntity(aggregate.getClass());

if (!persistentEntity.hasIdProperty()) {
return aggregate;
}

// we're doing INSERT and ID property value is not set explicitly by client
if (persistentEntity.isNew(aggregate) && !hasIdentifierValue(aggregate, persistentEntity)) {
return potentiallyFetchIdFromSequence(aggregate, persistentEntity);
} else {
return aggregate;
}
}

private boolean hasIdentifierValue(Object aggregate, RelationalPersistentEntity<?> persistentEntity) {
Object identifier = persistentEntity.getIdentifierAccessor(aggregate).getIdentifier();

if (persistentEntity.getIdProperty().getType().isPrimitive()) {
return identifier instanceof Number num && num.longValue() != 0L;
} else {
return identifier != null;
}
}

private Object potentiallyFetchIdFromSequence(Object aggregate, RelationalPersistentEntity<?> persistentEntity) {
Optional<SqlIdentifier> idSequence = persistentEntity.getIdSequence();

if (dialect.getIdGeneration().sequencesSupported()) {
idSequence.map(s -> dialect.getIdGeneration().createSequenceQuery(s)).ifPresent(sql -> {
Long idValue = operations.queryForObject(sql, Map.of(), (rs, rowNum) -> rs.getLong(1));
PersistentPropertyAccessor<Object> propertyAccessor = persistentEntity.getPropertyAccessor(aggregate);
propertyAccessor.setProperty(persistentEntity.getRequiredIdProperty(), idValue);
});
} else {
if (idSequence.isPresent()) {
LOG.warn("""
It seems you're trying to insert an aggregate of type '%s' annotated with @TargetSequence, but the problem is RDBMS you're
Copy link
Member

Choose a reason for hiding this comment

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

What is @TargetSequence?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was an old name of the @Sequence annotation, sorry, I've missed it out 🙈

working with does not support sequences as such. Falling back to identity columns
""".formatted(aggregate.getClass().getName()));
}
}

return aggregate;
}
}
3 changes: 2 additions & 1 deletion ...a/org/springframework/data/jdbc/repository/AbstractJdbcRepositoryLookUpStrategyTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.springframework.data.jdbc.testing.EnabledOnDatabase;
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
import org.springframework.data.repository.CrudRepository;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;

/**
Expand All @@ -40,7 +41,7 @@
abstract class AbstractJdbcRepositoryLookUpStrategyTests {

@Autowired protected OnesRepository onesRepository;
@Autowired NamedParameterJdbcTemplate template;
@Autowired NamedParameterJdbcOperations template;
@Autowired RelationalMappingContext context;

void insertTestInstances() {
Expand Down
Loading
Loading