Apply Nullability to MongoDb module #10213
Conversation
Related to: spring-projects#10083 Signed-off-by: Jiandong Ma <[email protected]>
...godb/src/main/java/org/springframework/integration/mongodb/inbound/MongoDbMessageSource.java
Show resolved
Hide resolved
...ion-mongodb/src/main/java/org/springframework/integration/mongodb/store/MessageDocument.java
Show resolved
Hide resolved
...mongodb/src/main/java/org/springframework/integration/mongodb/store/MongoDbMessageStore.java
Show resolved
Hide resolved
| @@ -611,49 +615,47 @@ public <S> S read(Class<S> clazz, Bson source) { | |||
| } | |||
|
|
|||
| private MessageWrapper readAsMessageWrapper(Bson source) { | |||
There was a problem hiding this comment.
I can not make this method return nullable, the underlying spring-data-mongodb does not allow to return null.
so I assert the source param never null?
...b/src/main/java/org/springframework/integration/mongodb/outbound/MongoDbOutboundGateway.java
Outdated
Show resolved
Hide resolved
...a/org/springframework/integration/mongodb/outbound/ReactiveMongoDbStoringMessageHandler.java
Outdated
Show resolved
Hide resolved
| @@ -144,6 +148,7 @@ protected MessageBuilderFactory getMessageBuilderFactory() { | |||
| @Override | |||
| public void afterPropertiesSet() { | |||
| if (this.mongoTemplate == null) { | |||
| Objects.requireNonNull(this.mongoDbFactory); | |||
There was a problem hiding this comment.
below MappingMongoConverter and MongoTemplate both use this, so I leave it here.
There was a problem hiding this comment.
Ok. Then NullAway.Init on this connectionFactory property ?
There was a problem hiding this comment.
Or Assert.notNull().
This method is called only once, so having a check is OK.
We use Objects.requireNonNull only if we deliberately know that it is not null.
Probably one day I’ll revise them all into a NullAway suppressions since I’m very picky for performance 😁
There was a problem hiding this comment.
mongoDbFactory can be null, when we give a mongoTemplate via ctor
There was a problem hiding this comment.
Or
Assert.notNull(). This method is called only once, so having a check is OK. We useObjects.requireNonNullonly if we deliberately know that it is not null. Probably one day I’ll revise them all into aNullAwaysuppressions since I’m very picky for performance 😁
ok
...ion-mongodb/src/main/java/org/springframework/integration/mongodb/store/MessageDocument.java
Show resolved
Hide resolved
...mongodb/src/main/java/org/springframework/integration/mongodb/store/MongoDbMessageStore.java
Show resolved
Hide resolved
| new Update().inc(SEQUENCE, 1L), | ||
| FindAndModifyOptions.options().returnNew(true).upsert(true), | ||
| Map.class, this.collectionName); | ||
| Objects.requireNonNull(result); |
There was a problem hiding this comment.
This looks redundant since you use another one in the next operator.
We need to think to get rid off of this at all if we are sure it is never null.
If this is a critical execution path, it would be better to avoid as much as possible extra calls.
There was a problem hiding this comment.
I suppress it with NullAway.
...mongodb/src/main/java/org/springframework/integration/mongodb/store/MongoDbMessageStore.java
Outdated
Show resolved
Hide resolved
…into jspecify-mongodb
Signed-off-by: Jiandong Ma <[email protected]>
|
Merged as: fb12c05 |
|
Here is some clean up after your change: a3476d2. Pay attention to the fix in all those components where we have a choice between template and connection factory. |
Related to: #10083