chibenwa commented on PR #1111: URL: https://github.com/apache/james-project/pull/1111#issuecomment-1210142364
> Help please? I will try to look at it in the coming week. > Effectively, have the messages lie about what they actually have in their content byte array. No they are supposed to tell what the underlying data is regardless of what they did actually load. > Tests basically want BODY and FULL to be the same thing in regard to findInMailbox(). Good catch. Maybe we should just **drop** Body fetch type and use **FULL** instead, which would simplify the code, and avoid some confusions. We don't use it (FetchType.BODY) so it effectively looks like accidental complexity we might want get rid of... That would simplify your issue? > The deleteMessages... tests are doing something strange during AbstractMessageMapper.updateFlag(). They first retrieve messages with fetch type METADATA and the save them via InMemoryMessageMapper.save(), which copies the truncated message and thus winds up with a broken message in the store?? So, this mechanism actually relies on MessageMappers to ignore the fetch type??? What you describe looks like a behavior specific to the in-memory implementation (ignoring fetch type, etc..). So ok memory mailbox, implemented for testing purposes, might benefit from a rework... > turns out honoring the fetch type breaks MemoryMessageMapperTest (and MemoryMessageWithAttachmentMapperTest), in different ways Do we care about memory honoring the fetch type stuff? Ok, it makes it closer to real implementations (cassandra / JPA) meaning catching some issues earlier in the development cycle, but integration tests (here MPT IMAP tests?) still get us covered. While making memory fetch type aware the way Cassandra implem is would be a contribution I would support, I do not know if I would consider it 'essential' > save them via InMemoryMessageMapper.save(), which copies the truncated message and thus winds up with a broken message in the store?? So, this mechanism actually relies on MessageMappers to ignore the fetch type??? We could adapt "save" behaviour to actually update message metadata instead of replacing it, if the message is already present? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
