You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/09/07 09:19:55 UTC
[GitHub] [james-project] quantranhong1999 opened a new pull request, #1182: JAMES-3815 Handle possible null values in imapUidTable and messageIdTable
quantranhong1999 opened a new pull request, #1182:
URL: https://github.com/apache/james-project/pull/1182
This NPE likely comes from badly applied [Rework message denormalization migration](https://github.com/apache/james-project/blob/master/upgrade-instructions.md#rework-message-denormalization) which creates in `imapUidTable` and `messageIdTable` a few rows with null `internalDate`, `bodyStartOctet`, `fullContentOctets` and `headerContent`. Now the Cassandra driver 4 code change just triggers that NPE (can not convert a null to Date).
Checked: `bodyStartOctet`, `size`, `headerContent` should be already good from NPE.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #1182: JAMES-3815 Handle possible null values in imapUidTable and messageIdTable
Posted by GitBox <gi...@apache.org>.
vttranlina commented on PR #1182:
URL: https://github.com/apache/james-project/pull/1182#issuecomment-1239222588
Should we fix the NPE of other classes in this pr?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on pull request #1182: JAMES-3815 Handle possible null values in imapUidTable and messageIdTable
Posted by GitBox <gi...@apache.org>.
Arsnael commented on PR #1182:
URL: https://github.com/apache/james-project/pull/1182#issuecomment-1240156114
> What if size is null? HeaderContent? bodyStartOctet ?
Answer in his first comment of the issue:
```
Checked: bodyStartOctet, size, headerContent should be already good from NPE.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on pull request #1182: JAMES-3815 Handle possible null values in imapUidTable and messageIdTable
Posted by GitBox <gi...@apache.org>.
Arsnael commented on PR #1182:
URL: https://github.com/apache/james-project/pull/1182#issuecomment-1240154619
> Finally it is not a pb of badly applied migration but a regression (you) introduced during cassandra driver 4.0 upgrade.
You mean "we"...?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1182: JAMES-3815 Handle possible null values in imapUidTable and messageIdTable
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1182:
URL: https://github.com/apache/james-project/pull/1182#discussion_r965246324
##########
mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageIdDAO.java:
##########
@@ -538,4 +540,36 @@ private ThreadId getThreadIdFromRow(Row row, MessageId messageId) {
}
return ThreadId.fromBaseMessageId(CassandraMessageId.Factory.of(threadIdUUID));
}
+
+ @VisibleForTesting
+ Mono<Void> insertNullInternalDateAndHeaderContent(CassandraMessageMetadata metadata) {
Review Comment:
Imo overkill
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on pull request #1182: JAMES-3815 Handle possible null values in imapUidTable and messageIdTable
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on PR #1182:
URL: https://github.com/apache/james-project/pull/1182#issuecomment-1239273836
> Should we fix the NPE of other classes in this pr?
IMO it depends on which tables and do we allow to write null into that. I will check tmr.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on pull request #1182: JAMES-3815 Handle possible null values in imapUidTable and messageIdTable
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on PR #1182:
URL: https://github.com/apache/james-project/pull/1182#issuecomment-1240367990
Check other possible same cases (add a column then reading that column does not handle null case):
- https://github.com/apache/james-project/blob/master/upgrade-instructions.md#adding-authorized_users-column-to-user-table => handled with mapNotNull
- https://github.com/apache/james-project/blob/master/upgrade-instructions.md#adding-messageid-metadata-to-the-cassandra-attachments => wrapped with Optional.ofNullable
- https://github.com/apache/james-project/blob/master/upgrade-instructions.md#adding-threadid-column-to-message-metadata-tables => handled null with a If
Check comment of Tung:
- CassandraMessageDAO => We dont allow write null and there is no change to `messageV2` table => read just like it is never null is ok
- CassandraMessageDAOV3 => We dont allow write null and there is no change to `messageV3` table => read just like it is never null is ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on pull request #1182: JAMES-3815 Handle possible null values in imapUidTable and messageIdTable
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on PR #1182:
URL: https://github.com/apache/james-project/pull/1182#issuecomment-1240178524
> What if size is null? HeaderContent? bodyStartOctet ?
We already wrap `Optional.ofNullable` when reading them.
Even `bodyStartOctet` and `size` are special cases when the driver does not return the `null` value but defaults 0 instead. May be return actual `null` here will be more exact.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #1182: JAMES-3815 Handle possible null values in imapUidTable and messageIdTable
Posted by GitBox <gi...@apache.org>.
chibenwa merged PR #1182:
URL: https://github.com/apache/james-project/pull/1182
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org