8 Replies Latest reply on May 2, 2005 4:13 PM by mikezzz

    Minor fixes to HEAD and integration of Nokia branch

    acoliver

      I've made minor fixes to head. I also did a bit of a cut and paste of the nokia branch to get that working. There is still some weird problem with the database being created with the bodytype having a not null constraint. Maybe I'm loopy but I haven't figured out WHO is creating the database since it obviously isn't the cmp container, I'm assuming its a hibernate thing but I couldn't find anything mapping to the entity mailbox table. I got everything working correctly with Hypersonic by manually recreating the table. We definitely have a little housekeeping to do in the next milestone release. I'm going to be around more for awhile so I'll be pitching in and working with some new volunteers.

      Anyone know how the ENTITYMESSAGE table is getting created with a NOT NULL on the bodytype (which apparently the entity mailbox stuff doesn't use)?

        • 1. Re: Minor fixes to HEAD and integration of Nokia branch
          acoliver

          crap it looks like we're loosing the last chunk. I'll look into it, I think I have more to do. May need to move the header stuff and all. It seems that we're doing the dot stuffing already.

          • 2. Re: Minor fixes to HEAD and integration of Nokia branch

            You've merged against an old revision. Check revision 1.5 of LocalMessage.java. BodyType is commented out, but is present in 1.4. Same with MessageBean.java.

            I'm making some changes around the Mailbox->MailBody integration that I will commit soon. Fixes for JBMAIL-54 have kinda pushed toward some refactoring (stuff we talked about offline). Basically the Mailbox will hold the store id explicitly rather than a serialized object. When this change goes through the bodyType will be reintroduced and the NOT NULL constraint will be correct as there will always be a body type. Let me know when you are finished so I can merge (and test) before committing.

            However I am also confused as to why it gets the not null constraint, without us explicitly putting it there. Will do some investigation.

            Cheers,
            Mike.

            • 3. Re: Minor fixes to HEAD and integration of Nokia branch

              Correction LocalMessage.java revision 1.4 has bodyType commented out, revision 1.3 has it in.

              • 4. Re: Minor fixes to HEAD and integration of Nokia branch
                acoliver

                yeah but it screws up because of the not null constraint

                • 5. Re: Minor fixes to HEAD and integration of Nokia branch

                  I have fixed the Nokia branch integration for simple mail bodies. The problem was Jamesesque nested InputStreams where still being used. Unfortunately stored mail bodies are broken until I merge the parseBody functionality into store. Will do some refactoring to make sure this code is shared. Stored mail bodies are now disabled in the jboss-service.xml file until this is fixed. Will have this fixed for M3.

                  The bodyType not null constraint is probably some type of CMP weirdness as you can not have a int value that is null. However this is also fixed, as the changes I have made to the mailbox/store integration are such that the bodyType field should always be populated.

                  Mike.

                  • 6. Re: Minor fixes to HEAD and integration of Nokia branch

                    Stored mail bodies now work with after removing the nested InputStreams. I have added a IOUtil.dotStuffingCopy method that will copy a dotStuffed stream to a normal output stream. Currently this is only used for stored mail bodies. Simple mail bodies are still using the code from the Nokia branch. Eventually I will refactor it so both use the new copying method, but I want to do some benchmarking first to check that the new util method performs just as well. In theory the new copy method should be slightly faster as it does 1 less copying step than the nokia code. There may be a negitive impact on the performance of the store, as the new copying method runs differently (not likely to be a terrible hit, but not optimal). The buffering within the blob output stream makes some assumptions about the way the copying is done. I will fix this soon.

                    Mike.

                    • 7. Re: Minor fixes to HEAD and integration of Nokia branch
                      acoliver

                      duh I should have known about the not null int thingy. Excellent Mike. I need to check what the previous numbers were with Thomas Diesler were and I'll give you them I just remember being pleasently shocked. After this I'm thinking our main bottleneck will be the stupid JavaMail+JAF thingy. I'm going to open a dialog up with the JAMES guys to see if there is room for a collaboration on a complete replacement for JavaMail.

                      • 8. Re: Minor fixes to HEAD and integration of Nokia branch

                        A quick note that I have created an interface called Copier and a couple of implemenations (SimpleCopier, DotStuffingCopier and DotUnstuffingCopier). SMTP uses the DotUnstuffingCopier for both Simple and Stored mail bodies (having them both use the same code is a good thing). DotStuffingCopier is for POP, but I haven't integrated it yet.

                        This code supercedes the Nokia branch code. Some micro-benchmarking that I have done shows the Copier interface to be marginally quicker and more memory efficient than the Nokia code, around 2-5% (but I had to beat it to death with a stick in order to get it that fast). The Copier interface is also more reusable, however not as elegant, from a design perspective, as the nested streams. It is not possible aggregate Copier imeplematations, each one needs to be implemented seperately. I could further wax philisophical on abstraction vs performance, but I will stop there before people start falling asleep.

                        Mike.