1 2 Previous Next 18 Replies Latest reply on Feb 27, 2008 9:40 AM by ataylor

    New package structure

    timfox

      I have just committed a refactoring to our new package structure.

      Here is how it goes:

      org.jboss.messaging.core and sub packages.

      Contain the messaging core - this is all the stuff needed to make a fully functioning transactional, reliable messaging system.

      It does not include jms, nor does it have any dependencies on JEE, JNDI etc. It basically will just depend on J2SE and MINA (if not using an invm transport).

      Please be extra careful not to introduce any such dependencies in this layer.

      Inside core, it is further split into modules:

      core/module-name (e.g. core/management)

      this contains the *interfaces* exported by that module. It should only contain interfaces and possible very simple struct like classes.

      The implementation of the interfaces goes in:

      core/module-name/impl

      The idea is that one interface can depend on interfaces from another module, but it *must not* depend on implementations from another module.

      This is currently broken in some modules (e.g. deployment, security etc) and this will need fixing, where code depends directly on concrete classes rather than interfaces. (More on this later).

      The current modules are:

      channelfactory - channel factory stuff (TBD)

      cluster

      config - the Configuration interface and implementations (Basic, File)

      deployers

      filter - core filters

      list - priority list

      logging

      management - the core management interface and implemementation

      memory- the memory manager (TBD)

      messagecounters

      persistence - persistence manager implementations

      this contains subdirectories:

      e.g.

      nullpm - the null persistence manager

      bdbje - the BDB JE persistence manager

      and then we have

      remoting - our remoting layer

      security - core security store interface and implementation

      server - the core server interfaces and implementation

      e.g. messaging server, server connection, server session, queue etc etc

      settings - the hierarchical repository stuff that handles queue and security settings

      version

      client - this contains the core client interfaces and implementation

      Then we have the jms stuff.

      This is in:

      org.jboss.messaging.jms.client -

      This is the client side JMS code

      org.jboss.messaging.jms.server -

      This is the server side JMSServerManager and related stuff.

      Then we have microcontainer stuff which is specific to the bootstrapping of the MC - much of this can probably be removed now.

      In more detail, some modules are currently exposing implementations to other modules, they are:

      1) Config - we need to create a Configuration interface which lives in the config module - currently there is a concrete class in the server module and another concrete class RemotingConfiguration in the remoting module. These should be combined and put in the config module.

      2) Deployers. Currently abstract class Deployer is exported from the module. This should be an interface. Also DeploymentManager should be an interface. We should also avoid singletons throughput JBM, this is to allow multiple JBM instances to co-exist in the same VM without interacting.

      3) messagecounter - currently these are concrete classes - shoud be abstracted out to interface + implementation

      4) remoting - there are various concrete classes in this module which are exported to other modules - should be made interfaces

      5) security

      6) settings - implemtation is used directly

      Other things to note:

      1) much of the stuff in the microcontainer module doesn't seem necessary any more.. can we remove?

      2) If a class is serializable it should be given a serialVersionUID

      3) please observe the order of static, public, private etc methods in a class. I've seen quite a few occasions of say, a provate method inserted into the middle of a load of public methods etc.

      4) Please avoid K & R style of braces:

      e.g.

      if (foo) {
      //do something
      } else if (bar) {
      //do somehting else
      }

      Is not the style we use.

      That's about it for now :)

      If any of the above comments apply to your areas, can you add to your TODO list to correct.

      Cheers.

      It's really starting to take shape now :)




        • 1. Re: New package structure
          ataylor

           

          Then we have microcontainer stuff which is specific to the bootstrapping of the MC - much of this can probably be removed now.


          Ok, I'll tidy this up,we still need ServiceLocator as we are still dependant on org.jboss.security.AuthenticationManager' and we need to find it via jndi when deployed under the App server.

          Also ive left JBMBootstrapServer which bootstraps via the MC, I'm not sure whether we need this or not, it depends on how we want to ship standalone(I'm creating another thread for this).

          • 2. Re: New package structure
            timfox

             

            "ataylor" wrote:


            Ok, I'll tidy this up,we still need ServiceLocator as we are still dependant on org.jboss.security.AuthenticationManager' and we need to find it via jndi when deployed under the App server.



            Any jaas stuff should be optional.

            The core shouldn't have jaas dependencies, but provide a simple non JAAS authentication / authorization manager which just uses a simple xml based role based security for authorization / authentication.

            It should be possible to inject a JAAS based authentication / authorization manager at run time using the MC, so this can be used when running inside AS -but again, this is outside core.

            • 3. Re: New package structure
              ataylor

               

              It should be possible to inject a JAAS based authentication / authorization manager at run time using the MC, so this can be used when running inside AS -but again, this is outside core.


              Ok, so thats what we have already.

              • 4. Re: New package structure
                ataylor

                 

                Please be extra careful not to introduce any such dependencies in this layer.


                The first thing we should do is change the build to compile each component seperately so this cant happen!

                If we want we could do this with each module under core as well, but it would make the build file a little more complex.

                • 5. Re: New package structure
                  timfox

                   

                  "ataylor" wrote:


                  Ok, so thats what we have already.


                  We're almost there.

                  The problem with the current implementation is that injects a org.jboss.security.AuthenticationManager which is a non core class. The core shouldn't be dependent on this.

                  Instead we need to define our own Authentication interface, and inject this instead.

                  Then, we

                  a) Create a simple implementation which authenticates using a simple non JAAS authentication approach - e.g. simple xml user-password

                  b) Create another implementation which is just a wrapper around JAAS stuff - this lives outside core and is what is injected when using AS

                  • 6. Re: New package structure
                    ataylor

                    we actually only call 2 methods so it should be pretty trivial

                    • 7. Re: New package structure
                      ataylor

                       

                      2) Deployers. Currently abstract class Deployer is exported from the module. This should be an interface.


                      It is an interface, 'Deployable' Which is what used by DeploymentManager. Granted the impl is pulled in by MessagingServer so I will change it, but his means adding it to the jbm-beans file so it can handle the lifespan of the object, which also means when running embedded the user has to cerate and start these as well. We have this problem with most of the wired components in MessagingServer

                      • 8. Re: New package structure
                        timfox

                        The class Deployer isn't an interface :)

                        My point was we shouldn't leak implementations from one module to another, just interfaces.

                        • 9. Re: New package structure
                          timfox

                           

                          "ataylor"but his means adding it to the jbm-beans file so it can handle the lifespan of the object, which also means when running embedded the user has to cerate and start these as well. We have this problem with most of the wired components in MessagingServer[/quote wrote:


                          No need to inject them.

                          Just instantiate them directly in the messaging server. So yes, we have a dependency on the implementation for construction, but not thereafter. This also makes unit testing a lot easier.


                          • 10. Re: New package structure
                            timfox

                            Another thing is use of private final fields, and variable modifiers.

                            I've seen (and corrected) a lot of places where just package protected visibility was used for no obvious reason.

                            In most cases fields should be private, and if they're not changed after construction they should be made final too. This helps catch programming errors and also hints to the compiler to make optimisations.

                            Similarly we should make method/compiler params final too. This also helps to catch errors at compile time.

                            I've already applied this to most classes (and incidentally caught a couple of bugs doing so that only showed up after adding the final modifier :), but we should take care to do this in the future.

                            • 11. Re: New package structure
                              timfox

                              Also, singletons are a no-no.

                              This effects deployers and remoting which both use singletons.

                              We need to be able to cope with the possibility of multiple JBM instances to run in the same VM and they should be able to do so without interference.

                              • 12. Re: New package structure
                                ataylor

                                 

                                The class Deployer isn't an interface :)


                                you know what i mean :)

                                Just instantiate them directly in the messaging server. So yes, we have a dependency on the implementation for construction, but not thereafter. This also makes unit testing a lot easier.


                                That was my point, we shouldn't be Dependant on the implementation for anything if we use interfaces. If its pluggable it should be an interface and should be injected, if it isnt, then do we really need an interface?

                                Just playing devils advocate! :)

                                • 13. Re: New package structure
                                  timfox

                                   

                                  "ataylor" wrote:

                                  That was my point, we shouldn't be Dependant on the implementation for anything if we use interfaces. If its pluggable it should be an interface and should be injected, if it isnt, then do we really need an interface?

                                  Just playing devils advocate! :)


                                  It's not all or nothing.

                                  Being dependent on the implementation in one place (at construction) is better than being dependent on the implementation in N places (each place the class is used), so it's an improvement.

                                  Also, other advantages:

                                  1) Makes testing a whole lot easier since can easily create mock / fake implementions

                                  2) Makes code easier to read / understand

                                  3) If user wants to create their own implementation of the interface they can. If it's a concrete class they'd have to extend that class. That may not be possible or desirable.

                                  • 14. Re: New package structure
                                    timfox

                                     

                                    then do we really need an interface?

                                    Just playing devils advocate! :)


                                    Well... if you take your reasoning to it's logical extreme then we'd just have a single main() method with all the code in that. ;)

                                    1 2 Previous Next