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

BeforeConvertCallback and auditing not invoked on nested object #1899

Closed
jiallombardo opened this issue Sep 28, 2024 · 1 comment
Closed
Assignees
Labels
status: invalid An issue that we don't feel is valid

Comments

@jiallombardo
Copy link

jiallombardo commented Sep 28, 2024

Assume the following classes (with Lombok @Data annotations):

public interface PersistableEntity {
    long getId();
    void setId(long id);
}

@Data
public class Base implements PersistableEntity {
    @Id
    long baseId;
    String name; 
    @CreatedDate
   Timestamp created;

    @MappedColumn(idColumn="baseId", keyColumn="baseId")
    SequentialSet<Sub> items;

    @Override
    long getId() {
        return baseId;
    }

    @Override
    void setId(long id) {
        this.baseId = id;
    } 
}

@Data
public class Sub implements PersistableEntity{
    @Id
    long subId;
    long baseId;
    String data;
    @CreatedDate
    Timestamp created;

    @Override
    long getId() {
        return subId;
    } 
    @Override
    void setId(long id) {
        this.subId = id;
    } 
}

We also have a BeforeConvertCallback for each type, which gets the id from a PostgreSQL sequence.

@Log4j2
@Component
public class AbstractBeforeConvertCallback<T extends PersistableEntity> implements BeforeConvertCallback<T> {

    private static final String SEQUENCE_ID_REQUEST = "SELECT nextval('%s')";

    protected final JdbcTemplate jdbcTemplate;

    @Override
    public @NotNull T onBeforeConvert(@NotNull T value) {
        log.debug("Converting entity {}", value);
        if (value.getId() == DEFAULT_ID) {
            value.setId(getNextId());
        }

        return value;
    }

    private Long getNextId() {
        return jdbcTemplate.queryForObject(String.format(SEQUENCE_ID_REQUEST, getSequence()), Long.class);
    }
}

@Component
public class BaseBeforeConvertCallback extends AbstractBeforeConvertCallback<Base> {

    private static final String SEQUENCE = "sq_base";

    public BaseBeforeConvertCallback(JdbcTemplate jdbcTemplate) {
        super(jdbcTemplate);
    }

    @Override
    protected String getSequence() {
        return SEQUENCE;
    }
}

@Component
public class SubBeforeConvertCallback extends AbstractBeforeConvertCallback<Sub> {

    private static final String SEQUENCE = "sq_sub";

    public SubBeforeConvertCallback(JdbcTemplate jdbcTemplate) {
        super(jdbcTemplate);
    }

    @Override
    protected String getSequence() {
        return SEQUENCE;
    }
}

Let's consider a test example (in Groovy) for BaseRepository:

def saveObject = new Base(baseId: 1, name: 'SomeName', items: [new Sub(subId: 1, data: 'data')]
baseRepository.save(saveObject)

This results in an erroneous query being generated for the sub item. Here is a part of the resulting stacktrace (let's assume that the next value generated for the Base sequence was 5):

Batch entry 0 INSERT INTO "sub" ("data", "base_id", "sub_id", "created") VALUES (('data'), (5), (NULL), (NULL))

So, what I assume is happening here:

  1. The Base entity is being properly saved
  2. The repository then attempts to save the Sub entity, but fails, since the BeforeConvertCallback and the auditing logic are not invoked for it.

I'm not sure, whether the repository is intended to work like this at all (although the fact that it attempts to save the mapped entities certainly hints that it might have been intended), but this seems like an oversight. Or, perhaps, it was a structural issue on my part?

Thank you.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 28, 2024
@schauder
Copy link
Contributor

This behaviour is intended and documented.

Events and callbacks get only triggered for aggregate roots. If you want to process non-root entities, you need to do that through a listener for the containing aggregate root.

This design is based on the DDD practice not to access elements of an aggregate from outside the aggregate.

@schauder schauder self-assigned this Sep 30, 2024
@schauder schauder added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 30, 2024
@schauder schauder closed this as not planned Won't fix, can't repro, duplicate, stale Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants