Listener Implementation

[cdempsey]

I was looking at the (still in progress) code for implementing
application events and listeners for those interesting happenings
within AR and I just wanted to throw out an idea for people to mull
over. This is my first stab on the project so be gentle... :)

The current implementation of the
org.architecturerules.api.listeners.Listener interface seems to be
doing multiple jobs: configuring/registering itself and handling a
bunch of on* event methods. I suggest we separate those two non-
related concerns into two different interfaces.

I was also concerned with the "on*" methods in the Listener
interface. There isn't anything wrong with them but once we publish
this interface to the world they will be set in stone at that point.
If we later discover some interesting events we will not be able to
simply add them to the interface without breaking code already out in
the wild. We could simply extend the interface but that quickly
breaks down when you have to do that over and over. We'd have
"interface creep".

What I want to throw out into the ring is the idea of having a
simplified Listener interface and creating a class hierarchy of
application events that can be passed to the Listener
implementations. The implementations would be responsible for
handling those events that they are interested in and ignoring those
they aren't. This buys us the ability to create new events as the
years go on without breaking listeners that have already been
written. Thoughts?

[Mike Nereson]

Awesome. I was just working on the java docs for the Listener interface this afternoon.

As you described there is a registerListener(Properties) method that is essentially an initialization method. It takes the Properties defined in the user's Configuration. The Listener may or may not need those Properties in order to initialize itself. So, yes, each Listener would have to know how to initialize itself. There is also the terminateListener method that would be responsible for cleaning up its resources and finishing any job that it was responsible for. The use case for this is the ReportListener will need to actually write the report in the terminate method. The ReportListener will use the properties to determine the path output the report and the type of report to output (ie xml, html, csv (only xml for the upcomming 3.0 milestone)). So thats the use case that I trying to implement.

I like the idea of the Listener hierarchy. Lets talk about that some more. Also, do you have any specific ideas on how configration of the Listener can be abstracted away from the Listener implemntation itself?

... a simplified Listener interface and creating a class hierarchy of

application events that can be passed to the Listener
implementations.  The implementations would be responsible for
handling those events that they are interested in and ignoring those
they aren't.  This buys us the ability to create new events as the

years go on ...

Can you describe this further? You're talking about a new entity that knows its event type and has some arbitrary properties that it may or may not contain. It passes this entity to the Listener implementation, and each implementation either knows about this type of event and may handle it, or does not even know about the type of event and disregards it. Is that right?

Thanks for reviewing that API. Now is the time to do that sort of thing. As you said, its stone-like after its released. However, on that note, I will share that this 3.0 release IS going to break backwards compatability becuase we are moving from com.seventytwomiles.architecturerules to org.architecturerules. Also, the API package was just created, the interfaces used to be right along side their implementations. Breaking backwards compatability sucks, but the migration to org.architecturerules I belive is critical. Also I belive our user base is very small still, so now is really the best time to do this.

Lets layout specific changes that can be made to the Listener interface.

~ Mike Nereson

[cdempsey]

> As you described there is a registerListener(Properties) method that is
> essentially an initialization method. It takes the Properties defined in the
> user's Configuration. The Listener may or may not need those Properties in
> order to initialize itself. So, yes, each Listener would have to know how to
> initialize itself. There is also the terminateListener method that would be
> responsible for cleaning up its resources and finishing any job that it was
> responsible for. The use case for this is the ReportListener will need to
> actually write the report in the terminate method. The ReportListener will
> use the properties to determine the path output the report and the type of
> report to output (ie xml, html, csv (only xml for the upcomming 3.0
> milestone)). So thats the use case that I trying to implement.

Totally with you on that. I was suggesting that it be two separate
interfaces because there will be use-cases where listeners have no
resources to setup or clean up and forcing every listener to take on
that responsibility seems misplaced. Hiding it in an abstract base
class is rough since external developers may want to implement their
own listener hierarchy and forcing them to code away those methods or
use our base classes wouldn't be nice. lol

> Can you describe this further? You're talking about a new entity that knows
> its event type and has some arbitrary properties that it may or may not
> contain. It passes this entity to the Listener implementation, and each
> implementation either knows about this type of event and may handle it, or
> does not even know about the type of event and disregards it. Is that right?

Dead on. The listener Interface would basically just be "public void
onEvent(AbstractEvent event)". Implementations of the interface would
be responsible for handling the exact event sub-classes they're
interested in. For example, a listener may have a "private void
process(SourceDirectoryNotFoundEvent event)" method that is calls from
the "onEvent". But if we suddendly added say
"SourceDirectoryInPigLatinEvent" it wouldn't break all the listeners
already written because they'd just ignore it. When the developer
decided they wanted to pay attention to the new event they could.
They just wouldn't be on our time schedule.

> Thanks for reviewing that API. Now is the time to do that sort of thing. As
> you said, its stone-like after its released. However, on that note, I will
> share that this 3.0 release IS going to break backwards compatability
> becuase we are moving from com.seventytwomiles.architecturerules to
> org.architecturerules.

I don't mind breaking backward compat when its needed. Just wanted to
make sure we thought a bit about future compat too. :)

> > written.  Thoughts?- Hide quoted text -
>
> - Show quoted text -

[Mike Nereson]

> As you described there is a registerListener(Properties) method that is
> essentially an initialization method. It takes the Properties defined in the
> user's Configuration. The Listener may or may not need those Properties in
> order to initialize itself. So, yes, each Listener would have to know how to
> initialize itself. There is also the terminateListener method that would be
> responsible for cleaning up its resources and finishing any job that it was
> responsible for.

I was suggesting that it be two separate interfaces because there will be use-cases where listeners have no
resources to setup or clean up and forcing every listener to take on
that responsibility seems misplaced. 

Alright. Lets forget that any Listener interface or implementation exists. Lets design the next hierarchy and layout the requirements for an even listening framework.

org.architecturerules.api.listener

   EventListener
      .onEvent(Event)

   ConfigurableEventListener
      .initialize(Properties)
      .terminate()

org.architecturerules.api.event

   Event
     
org.architecturerules.event
   RuleAddedEvent
   RuleRemovedEvent
   RuleViolatedEvent
   WildcardMatchedEvent
   many many more to come....

I don't like the name ConfigurableEventListener. Also, I'd like to split the initialize and terminate up into their own, because, for example, the ReportListener is only concerned with terminate.
 
   EventListener
      .onEvent(Event)

   InitilizableEventListener
      .initialize(Properties)

   TerminatableEventListener
      .terminate()

Now, since ReportListener didn't get the properties from initialize, they need to be tossed into terminate.

   EventListener
      .onEvent(Event)

   InitilizableEventListener
      .initialize(Properties)

   TerminatableEventListener
      .terminate(Properties)


New issues:

• Find a better name for InitilizableEventListener and TerminatableEventListener.
• Identity other classes that fall under or over the EventListener.

~ Mike Nereson

[Chris Dempsey]

Let me take a deep dive into the ReportListener so that I can understand the use-case better.

[cdempsey]

Nevermind! ReportListener is empty. lol

I started on a rough implementation to make sure that I wasn't crazy
and I've extended the java.util.EventListener and
java.util.EventObject from the JDK as the basis of the listener code.
Extending those into AbstractEvent and AbstractEventListener seems
okay so far.

I'm guessing we're wanting to expose the configuration and tear down
of the listeners to enable us to control when we grab or clean up
resources that we might be holding on to in the listener at a specific
point in time? Say a database connection or a file handle, for
example? If so we may simply want to call the interfaces Configurable
and Finalizable. If we do that we could re-use the interfaces if we
decided to support custom plug-ins to AR in the future.

> >    - Find a better name for InitilizableEventListener and
> >    TerminatableEventListener.
> >    - Identity other classes that fall under or over the EventListener.
>
> > ~ Mike Nereson- Hide quoted text -

[Mike Nereson]

Nevermind!  ReportListener is empty. lol

I prototyped the output of the XML report here http://code.google.com/p/architecturerules/issues/detail?id=60

That, like anything else, is up for review. If anyone has any comments or questions about it.

I foresee the XML report being used by third parties, as well as being used by our Maven site report and so that is the only use case that I have for the report. Users have asked for an XML output. I don't know if they have plans for the output or not.  If we wanted to, we could make a Hudson plugin, which would also read the contents of the XML output. A Hudson plugin would be good for our future. It should help us snag some new users since Hudson is a big hit and pluggable.

I'm guessing we're wanting to expose the configuration and tear down
of the listeners to enable us to control when we grab or clean up
resources that we might be holding on to in the listener at a specific
point in time?  Say a database connection or a file handle, for
example? 

Yes, ReportListener will output to a File. It could stream the output to the file as the events occur, in which case the file is created and opened in the ReportListener initalization, or it could do one major file write in the terminate method, in which initialize is not needed for the ReportListener. Either one works for me.

 

If so we may simply want to call the interfaces Configurable
and Finalizable.  If we do that we could re-use the interfaces if we
decided to support custom plug-ins to AR in the future.

Thats good for reusability, but make requires the interfaces to be more generic. We need to pass the Properties to both methods.

 

~ Mike Nereson

[Chris Dempsey]

I prototyped the output of the XML report here http://code.google.com/p/architecturerules/issues/detail?id=60

That, like anything else, is up for review. If anyone has any comments or questions about it.

I foresee the XML report being used by third parties, as well as being used by our Maven site report and so that is the only use case that I have for the report. Users have asked for an XML output. I don't know if they have plans for the output or not.  If we wanted to, we could make a Hudson plugin, which would also read the contents of the XML output. A Hudson plugin would be good for our future. It should help us snag some new users since Hudson is a big hit and pluggable.

 

Good job!  I'll wander over and take a look at it ASAP.

 

Yes, ReportListener will output to a File. It could stream the output to the file as the events occur, in which case the file is created and opened in the ReportListener initalization, or it could do one major file write in the terminate method, in which initialize is not needed for the ReportListener. Either one works for me.

 

I think following the principle of least surprise and the general idea that a method does one thing developers wouldn't expect file writes to happen within a "terminate" method.  I suppose it would be like waiting to issue SQL statements to the database on a connection.close() call.

Thats good for reusability, but make requires the interfaces to be more generic. We need to pass the Properties to both methods.

 

We could generify the interfaces (Configurable<T>, Finalizable<T>) and let implementations choose the structure for passing in the configuration / tear down data.  Just thinking out loud here but shouldn't the listener implementation know how to clean up after itself (it knows it has files to clean up, for example)?  If so, then Finalizable wouldn't need parameters at all.  It would just be a hook method we would call from the main AR processing flow to give listeners time to clean up after themselves before we shutdown.

[Mike Nereson]

Just thinking out loud here but shouldn't the listener implementation know how to clean up after itself (it knows it has files to clean up, for example)?  If so, then Finalizable wouldn't need parameters at all.  It would just be a hook method we would call from the main AR processing flow to give listeners time to clean up after themselves before we shutdown.

Depends. ReportListener needs to know where to output the report to and what format to output as. That information comes in Properties. So, if finalize writes the file, the initialize doesn't do anything so initialize would not be neccessary. If initialize(Properties) is not used, then ReportListener still needs to get the Properties to know where to output the file too. The other option is to implement initalize(Properties) just to store a reference to the properties.

On the other hand, if the file output is streamed as the events occur, then we will need initialize, and finalize will just close the file.

I think following the principle of least surprise and the general idea that a method does one thing developers wouldn't expect file writes to happen within a "terminate" method.  I suppose it would be like waiting to issue SQL statements to the database on a connection.close() call.

I like the idea of streaming the data out to the file. And it sounds like you'd agree?

[Chris Dempsey]

I like the idea of streaming the data out to the file. And it sounds like you'd agree?

 

Yup!  I totally agree that is the way to go.

[cdempsey]

Is there anything you'd like help on with the listeners, Mike? Or
should I take a look at some of the issues first?

On Aug 31, 5:54 pm, "Chris Dempsey" <cdal...@gmail.com> wrote:
> > I like the idea of streaming the data out to the file. And it sounds like
> > you'd agree?
>
> Yup!  I totally agree that is the way to go.
>
>
>
> On Sun, Aug 31, 2008 at 3:56 PM, Mike Nereson <mikenere...@gmail.com> wrote:
> >   Just thinking out loud here but shouldn't the listener implementation

> >> know how to clean up after itself (*it knows it has files to clean up,
> >> for example*)?  If so, then Finalizable wouldn't need parameters at all.

> >> It would just be a hook method we would call from the main AR processing
> >> flow to give listeners time to clean up after themselves before we shutdown.
>
> > Depends. ReportListener needs to know where to output the report to and
> > what format to output as. That information comes in Properties. So, if
> > finalize writes the file, the initialize doesn't do anything so initialize
> > would not be neccessary. If initialize(Properties) is not used, then
> > ReportListener still needs to get the Properties to know where to output the
> > file too. The other option is to implement initalize(Properties) just to
> > store a reference to the properties.
>
> > On the other hand, if the file output is streamed as the events occur, then
> > we will need initialize, and finalize will just close the file.
>

> >  I think following the principle of least surprise<http://en.wikipedia.org/wiki/Principle_of_least_surprise>and the general idea that a method does one thing developers wouldn't expect

> >> file writes to happen within a "terminate" method.  I suppose it would be
> >> like waiting to issue SQL statements to the database on a connection.close()
> >> call.
>
> > I like the idea of streaming the data out to the file. And it sounds like

> > you'd agree?- Hide quoted text -

[Mike Nereson]

Chris,

You've invested a good amount of time into reviewing the current listener setup, and clearly have a good idea as to how you'd implement an even better setup. I have already thrown up some code which should demonstrate what I was going for, and you've seen the XML report which is the current end goal of the listeners. I think you are more than well enough equiped with the requirements and experience to implement the even listener.

I have plenty of other tasks that I could be working on and have to particular preference. I think we'd be pretty well off if you took on the even listeners while I hit up one of the other issues or finish up some unfinished tasks (like default-architecture-rules.xml and proper tests for reading properties.)

So yes, if you want to take on the listeners, please have at it.

~ Mike Nereson

[Chris Dempsey]

Alrighty.  I'll start moving on the listeners then. :)

[Mike Nereson]

We talked about streaming the xml report. This tool can do just that for us...

http://www.ociweb.com/mark/programming/WAX.html

However, its not hosted in any maven repo, so I'm not sure it is the BEST tool for us.


~ Mike Nereson

[Chris Dempsey]

I took a look at WAX.  Other than hating the name it looks nice.  :)  One of the oddities I noticed is the API names.  It just seems strange to call "start" instead of "element" or "attr" instead of "attribute".  Not a big deal really and certainly not a reason not to use it.  I've been kicking around writing something like this for work. Currently, our XML generation is all StringBuilder-based which is fast but way ugly code.

Nice find!

[Mike Nereson]

I took a look at WAX.  Other than hating the name it looks nice.  :) 

WAX = Writing API for XML.  Could be worse, but could be a whole lot better to. I
 

One of the oddities I noticed is the API names.  It just seems strange to call "start" instead of "element" or "attr" instead of "attribute".  Not a big deal really and certainly not a reason not to use it. 

I looked at their Google code site. They have no wiki, no issues and one revision. I wonder if its just one developer who doesn't have a community around him to help point out proper naming conventions like this.

I've been kicking around writing something like this for work.

If it is just one guy, he could use your help - But we need your help too, so don't go too far ( :  The domain of WAX seems like a good area to get into if one is looking to join an open source project .
 

Currently, our XML generation is all StringBuilder-based which is fast but way ugly code.

I think that's how most people are still doing it. Its either a StringBuilder or a complex heavy-weight XML library.

I am not sure we should use WAX though, because its not hosted in any Maven repository, so our maven users, which hopefuly most are, would not be able to get the dependency directly with maven. Ah, the woes of software development.



~ Mike Nereson

[Chris Dempsey]

Well, I'm buttoning up the changes on the internal XML generation code and could reproduce it for AR after I'm done writing all the tests for the listeners/events.

I agree with your concerns surrounding WAX.  I had not gotten to the point where I was going to look in a Maven repo for it but was a bit disappointed that I could not browse the source for WAX on-line.

[Mike Nereson]

Same here. I went to browse the source. I assume that a pretty common first step to investigating a new open source project that you have some interest it. The Google code page is completely empty.

I was turned on to WAX by a friend in St Louis who attends the STL Java User Group. I guess this guy is presenting this project there this week or so. Good for him. It should get him some more attention.

~ Mike Nereson