Skip to content

SimpleReactiveMongoRepository#saveAll does not populate @Id property if it is immutable #3609

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

Closed
petitcl opened this issue Mar 25, 2021 · 1 comment
Assignees
Labels
type: bug A general bug

Comments

@petitcl
Copy link
Contributor

petitcl commented Mar 25, 2021

When using the Flux<S> saveAll(Publisher<S> entityStream) method from the SimpleReactiveMongoRepository class, the returned entities do not have their @Id annotated property populated, in the case where the annotated field is immutable.

The @Id annotated property should obviously be populated with the ObjectId generated by mongodb.

This bug does not affect Flux<S> saveAll(Iterable<S> entities) nor Flux<S> insert(Publisher<S> entities). It only occurs if the @Id property is immutable (ie: final and no setter).

I was able to reproduce the bug on the following versions:

  • spring-data-mongodb 3.1.6, spring-data-commons 2.4.6
  • spring-data-mongodb 3.0.7, spring-data-commons 2.3.7
  • spring-data-mongodb 2.2.12, spring-data-commons 2.2.12

Here a sample repository to reproduce this bug: petitcl/spring-data-mongodb-sample-1. This sample shows that the bug occurs for an immutable entity but not for a mutable entity.

After looking at the code of SimpleReactiveMongoRepository in version 2.4.4, it seems to be due to the fact that the original entity reference is returned, instead of the entity returned by the underlying MongoOperations.
This works if the entity is mutable, because the entity reference will be updated with the new id, but it does not work if the entity is immutable, as it would require a new reference with the populated id.

Current Code:

	@Override
	public <S extends T> Flux<S> saveAll(Publisher<S> entityStream) {

		Assert.notNull(entityStream, "The given Publisher of entities must not be null!");

		return Flux.from(entityStream)
			.flatMap(
				entity -> entityInformation.isNew(entity) ?
				mongoOperations.insert(entity, entityInformation.getCollectionName()).then(Mono.just(entity)) :
				mongoOperations.save(entity, entityInformation.getCollectionName()).then(Mono.just(entity))
			);
	}

What I assume it should look like:

	@Override
	public <S extends T> Flux<S> saveAll(Publisher<S> entityStream) {

		Assert.notNull(entityStream, "The given Publisher of entities must not be null!");

		return Flux.from(entityStream)
			.flatMap(
				entity -> entityInformation.isNew(entity) ?
					mongoOperations.insert(entity, entityInformation.getCollectionName()) :
					mongoOperations.save(entity, entityInformation.getCollectionName())
			);
	}

If you validate that this is indeed a bug, I can probably submit a PR to fix it.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 25, 2021
@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 26, 2021
@mp911de mp911de self-assigned this Mar 26, 2021
@mp911de
Copy link
Member

mp911de commented Mar 26, 2021

Thanks for reporting the issue. This is indeed a bug and a leftover from the time where ReactiveMongoOperations.save(…) emitted Void. Feel free to submit a pull request including a test.

petitcl pushed a commit to petitcl/spring-data-mongodb that referenced this issue Mar 26, 2021
Make SimpleReactiveMongoRepository#saveAll(Publisher<S>) return the saved entity references instead of the original references.

Closes spring-projects#3609
petitcl added a commit to petitcl/spring-data-mongodb that referenced this issue Mar 26, 2021
Make SimpleReactiveMongoRepository#saveAll(Publisher<S>) return the saved entity references instead of the original references.

Closes spring-projects#3609
petitcl added a commit to petitcl/spring-data-mongodb that referenced this issue Mar 26, 2021
Make SimpleReactiveMongoRepository#saveAll(Publisher<S>) return the saved entity references instead of the original references.

Closes spring-projects#3609
petitcl added a commit to petitcl/spring-data-mongodb that referenced this issue Mar 26, 2021
Make SimpleReactiveMongoRepository#saveAll(Publisher<S>) return the saved entity references instead of the original references.

Closes spring-projects#3609
petitcl added a commit to petitcl/spring-data-mongodb that referenced this issue Mar 26, 2021
Make SimpleReactiveMongoRepository#saveAll(Publisher<S>) return the saved entity references instead of the original references.

Closes spring-projects#3609
@mp911de mp911de added this to the 3.0.9 (Neumann SR9) milestone Mar 29, 2021
mp911de added a commit that referenced this issue Mar 29, 2021
Omit StreamUtils usage if input is a collection. Remove superfluous Flux.from(…). Simplify test and migrate test to JUnit 5.

See #3609.
Original pull request: #3611.
mp911de pushed a commit that referenced this issue Mar 29, 2021
Make SimpleReactiveMongoRepository#saveAll(Publisher<S>) return the saved entity references instead of the original references.

Closes #3609
Original pull request: #3611.
mp911de added a commit that referenced this issue Mar 29, 2021
Omit StreamUtils usage if input is a collection. Remove superfluous Flux.from(…). Simplify test and migrate test to JUnit 5.

See #3609.
Original pull request: #3611.
mp911de pushed a commit that referenced this issue Mar 29, 2021
Make SimpleReactiveMongoRepository#saveAll(Publisher<S>) return the saved entity references instead of the original references.

Closes #3609
Original pull request: #3611.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants