1 2 Previous Next 15 Replies Latest reply on Feb 14, 2012 9:02 AM by Tom Eicher

    RemoteGatewayListener, RemoteFileSystemFactory, RemoteFileSy

    Tom Fennelly Master

      This code is very interesting indeed :)

      I was looking at it wrt JBESB-2524, specifically in relation to how it uses the ftpLocalDir and ftpRemoteDir config settings.

      These settings are used by the FtpImpl and SecureFtpImpl classes, which are RemoteFileSystem implementations. The are constructed from an EPR by the RemoteFileSystemFactory. Both implementations initialise their ftpLocalDir and ftpRemoteDir config settings from the jboessesb-properties.xml. It then loads this into a ConfigTree instance, calls checkParams, which goes and initializes the ftpLocalDir and ftpRemoteDir variables again from the configtree that was just constructed from the config. Huh?

      I hope somebody sees the logic in all of this toing and frowing with the config!!

      Anyway... does anyone know what the real intention is here? ftpLocalDir and ftpRemoteDir are currently being configured from the jbossesb-properties.xml. Was there ever an intention to allow this to be overridden by the EPR ala how it is currently being set in the configtree passed by the RemoteGatewayListener to the ListenerUtil.assembleEpr method?

      As I see it, this code in RemoteGatewayListener is currently doing nothing. Can we just remove it and skip the EPR override that we don't have?

      Also on this FTP code... looks to me like RemoteFileSystem instances are potentially being created for fun by (e.g.) the RemoteFileMessageComposer i.e. for every message. Is this really necessary, or can we just create one for the RemoteFileMessageComposer instance?

        • 1. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
          Tom Fennelly Master

          Ooooppppssss... sorry about the hyperlink :)

          • 2. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
            Kevin Conner Master

             

            "tfennelly" wrote:
            This code is very interesting indeed :)

            I suspect it is no different from the rest of the code ;)

            "tfennelly" wrote:
            These settings are used by the FtpImpl and SecureFtpImpl classes, which are RemoteFileSystem implementations. The are constructed from an EPR by the RemoteFileSystemFactory. Both implementations initialise their ftpLocalDir and ftpRemoteDir config settings from the jboessesb-properties.xml.

            I'll be very surprised if this is the case, are you certain of this? How could any of the ftp/sftp stuff work in that case. As already mentioned on the JIRA issue, it looks like the local dir is not getting passed through but the remote must be getting through otherwise it just wouldn't work.

            As for the local dir, I'm not sure this should even be in the EPR to be honest as it should be specific to the local system. It would be better if this was being taken from jbossesb-properties.xml and defaulting to the value of java.io.tmpdir.

            "tfennelly" wrote:
            It then loads this into a ConfigTree instance, calls checkParams, which goes and initializes the ftpLocalDir and ftpRemoteDir variables again from the configtree that was just constructed from the config. Huh?

            Huh as in you are surprised or huh as in it appears to be doing it in a similar way to the others?

            "tfennelly" wrote:
            I hope somebody sees the logic in all of this toing and frowing with the config!!

            Not really, but it seems to be consistent with the rest of it. But then you know this as well as everyone else :)

            "tfennelly" wrote:
            Anyway... does anyone know what the real intention is here? ftpLocalDir and ftpRemoteDir are currently being configured from the jbossesb-properties.xml.

            Remote must be coming from the config, surely, although local doesn't really make sense.
            "tfennelly" wrote:
            Was there ever an intention to allow this to be overridden by the EPR ala how it is currently being set in the configtree passed by the RemoteGatewayListener to the ListenerUtil.assembleEpr method?

            The remote definitely, and I'm pretty sure this already works. The local doesn't make any sense to me :)

            "tfennelly" wrote:
            As I see it, this code in RemoteGatewayListener is currently doing nothing. Can we just remove it and skip the EPR override that we don't have?

            Which code are you referring to? Can you give line numbers?

            "tfennelly" wrote:
            Also on this FTP code... looks to me like RemoteFileSystem instances are potentially being created for fun by (e.g.) the RemoteFileMessageComposer i.e. for every message. Is this really necessary, or can we just create one for the RemoteFileMessageComposer instance?

            I'm sure there are many improvements to this code, like the others. I suspect the reason for this is more to do with the underlying connections and the lack of recovery in the original donation rather than anything else but file a JIRA for it.

            Lets face it, the original code did not even understand how to construct a proper ftp URL, leaving us in the position where we can't easily fix it without breaking everything out there.

            Kev

            • 3. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
              Tom Fennelly Master

               

              "Kevin.Conner@jboss.com" wrote:
              "tfennelly" wrote:
              These settings are used by the FtpImpl and SecureFtpImpl classes, which are RemoteFileSystem implementations. The are constructed from an EPR by the RemoteFileSystemFactory. Both implementations initialise their ftpLocalDir and ftpRemoteDir config settings from the jboessesb-properties.xml.

              I'll be very surprised if this is the case, are you certain of this? How could any of the ftp/sftp stuff work in that case. As already mentioned on the JIRA issue, it looks like the local dir is not getting passed through but the remote must be getting through otherwise it just wouldn't work.


              FtpImpl and SecureFtpImpl are constructed from an EPR, but the ftpLocalDir and ftpRemoteDir settings are not coming from the EPR. They are being read from the jbossesb-properties.xml. Then... they construct a ConfigTree instance from the data read from the EPR + the data read from the jbossesb-properties.xml. The the configTree is used to initialize everything again and this is what I mean by toing and frowing :)

              So, the code that I think is irrelevant (and should be removed) is the code in the RemoteGatewayListener class (71 to 77). It's setting ftpLocalDir and ftpRemoteDir on a ConfigTree instance (not the same instance I talk about above) and constructing an EPR from the ConfigTree. Bu the EPR doesn't support ftpLocalDir and ftpRemoteDir.

              I was also thinking that setting these on the EPR was a bit strange and that it should be defined at the system level. I didn't say it in the last post, but since you seem to think the same thing then that's cool :)

              • 4. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                Kevin Conner Master

                 

                "tfennelly" wrote:
                FtpImpl and SecureFtpImpl are constructed from an EPR, but the ftpLocalDir and ftpRemoteDir settings are not coming from the EPR.


                This all sounds strange, as you are suggesting that the ftp code doesn't work and we know this is not the case. Let me dig around and see what I find.

                "tfennelly" wrote:
                I was also thinking that setting these on the EPR was a bit strange and that it should be defined at the system level. I didn't say it in the last post, but since you seem to think the same thing then that's cool :)

                The local dir definitely, the remote dir has to be part of the EPR though. Give me 30 mins to go through it and see what is going on.

                Kev

                • 5. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                  Tom Fennelly Master

                   

                  "Kevin.Conner@jboss.com" wrote:
                  "tfennelly" wrote:
                  FtpImpl and SecureFtpImpl are constructed from an EPR, but the ftpLocalDir and ftpRemoteDir settings are not coming from the EPR.


                  This all sounds strange, as you are suggesting that the ftp code doesn't work and we know this is not the case. Let me dig around and see what I find.


                  Something is getting badly list in translation here as I am not trying to imply that this is not working!!!!

                  The values for ftpLocalDir and ftpRemoteDir are coming from jbossesb-properties.xml.

                  • 6. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                    Tom Fennelly Master

                    Oh and go for it... dig to your hearts content :)

                    • 7. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                      Tom Fennelly Master

                      Looking at the history... I think it was because of a commons-httpclient version clash between soapUI and the AS.

                      • 8. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                        Tom Fennelly Master

                        And that soesn't appear to be an issue any longer, so probably no longer any need for the MBean.

                        • 9. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                          Kevin Conner Master

                           

                          "tfennelly" wrote:
                          Something is getting badly list in translation here as I am not trying to imply that this is not working!!!!

                          Something was definitely getting lost as you forgot to mention that the remote dir is actually coming from the configured uri and not that parameter ;)

                          "tfennelly" wrote:
                          The values for ftpLocalDir and ftpRemoteDir are coming from jbossesb-properties.xml.

                          That is certainly true for the local dir but, thankfully, not for remote. The remote dir is only being taken from the jbossesb-properties.xml iff the uri is not set.

                          The behaviour should definitely be local dir from jbossesb-properties.xml, defaulting to java.io.tmpdir, and remote dir from the EPR (through URI in this case).

                          Kev

                          • 10. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                            Kevin Conner Master

                             

                            "tfennelly" wrote:
                            Oh and go for it... dig to your hearts content :)

                            Don't worry I did :)

                            • 11. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                              Tom Fennelly Master

                               

                              "Kevin.Conner@jboss.com" wrote:
                              "tfennelly" wrote:
                              Something is getting badly list in translation here as I am not trying to imply that this is not working!!!!

                              Something was definitely getting lost as you forgot to mention that the remote dir is actually coming from the configured uri and not that parameter ;)


                              Touche!!

                              I glossed over that when looking at it... just looked at the local part. In any case... the code in RemoteGatewayListener would still appear to by irrelevant and (more importantly) misleading, right?

                              • 12. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                                Tom Fennelly Master

                                 

                                "Kevin.Conner@jboss.com" wrote:
                                "tfennelly" wrote:
                                Oh and go for it... dig to your hearts content :)

                                Don't worry I did :)


                                I'll leave all the worrying to you Kev ;)

                                • 13. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                                  Kevin Conner Master

                                   

                                  "tfennelly" wrote:
                                  I glossed over that when looking at it... just looked at the local part. In any case... the code in RemoteGatewayListener would still appear to by irrelevant and (more importantly) misleading, right?

                                  Yep, couldn't agree more.

                                  • 14. Re: RemoteGatewayListener, RemoteFileSystemFactory, RemoteFi
                                    Kevin Conner Master

                                     

                                    "tfennelly" wrote:
                                    I'll leave all the worrying to you Kev ;)

                                    I have plenty of that this morning, my next one is trying to work out why we no longer install the quickstarts into the distribution :)

                                    Kev

                                    1 2 Previous Next