Hi Shenglin
One thing which you are doing well is introducing subresources, given
that both MBean and MBeanCotellection have become the ones.
Understanding how subresources work is good in itself. I can see you
have unique ids allocated to MBeans now. So there's some progress.
IMHO some more code cleanup is needed.
1. At this stage please have only a ***single*** subresource,
intoroduce a dedicated class called MBeanResource which should have
only a single member, MBean instance. This MBeanResource should only
have right now setMBean(MBean) and getMBean methods. getMBean method
should be annotated with @GET.
2. MBean and MBeanCollection should have no static methods, and no
JAX-RS resource methods at all. These are plain JAXB beans only.
3. JMXServer should have:
- /mbean/{id}
Note, Path value should be '/mbean/{id}'
You should use a captured {id}, such as 0, 1, 2, etc (as opposed
to manually parsing path segments) for locating MBean, set this MBean
on a new MBeanResource instance and return MBeanResource,
MBeanResource.getMBean will complete the processing. we will think
later on how to keep MBeanResources between requests. The reason I'd
like a dedicated MBeanResource wrap MBean is because possible
modifications, update notifications, etc may need to be dealt with and
thus IMHO it can be cleaner to keep MBean as a plain JAXB bean and
have a dedicated JMX-aware handler around it as opposed to making
MBean both JAXB and JMX-aware at the same time.
- /list
- /{property}/{value}
Please note this method, /{property}/{value}, is on JMXServer. Remove
@Path("") and replace it with /{property}/{value}.
- /objectname/{objectname}
This method should iterate over the existing MBeanCollection only
without quering the instrumentation manager again.
3. All the methods should use the same code for accessing the map -
which is what you already do. Some cleanup is needed there too, but
for now please address the above
thanks, Sergey
On Thu, Jun 9, 2011 at 4:05 AM, Shenglin Qiu <dabaip...@hotmail.com> wrote:
> Hi Sergey:
>
> Sorry for replying a little bit late.
> But I reverted the code as you mentioned /mbean/{id} must not be changed.
>
> Code has be synced to github:
> https://github.com/dabaipang/services
>
> /**
> * Specification:
> * Function 1:
> * Jmx Server:
> * sub-resource locator
> * http://localhost:8080/services/jmx/mbean/0
> * http://localhost:8080/services/jmx/mbean/1
> * ...
> *
> *
> * Function 2:
> * Note: search by bus.id, bus.id will be changed on every time server
> had been bounced.
> * http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
> *
> http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,*
> * ...
> *
> http://localhost:8080/services/jmx/objectname/org.apache.cxf:port=%22CustomerServiceImpl%22,*
> *
> http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,port=%22SoapPort%22,type=Bus.Service.Endpoint,*
> *
> http://localhost:8080/services/jmx/objectname/org.apache.cxf:type=Bus.Service.Endpoint,*
> *
> http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,service=%22%7Bhttp%3A%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl%22,type=Bus.Service.Endpoint,*
> *
> http://localhost:8080/services/jmx/objectname/org.apache.cxf:service=%22%7Bhttp%3A%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl%22,*
> *
> * Function 3:
> * Search by service:
> *
> http://localhost:8080/services/jmx/service/%7Bhttp:%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl
> *
> http://localhost:8080/services/jmx/service/%7Bhttp:%2F%2Fserver.gsoc.apache.org%2F%7DUserServiceImpl
> *
> * Note, above queries are searching these 2, but replace by url encoding
> from:
> * {http://server.gsoc.apache.org/}UserServiceImpl
> * {http://server.gsoc.apache.org/}CustomerServiceImpl
> *
> * Search by type:
> * http://localhost:8080/services/jmx/type/Bus.Service.Endpoint
> *
> * Search by port:
> * http://localhost:8080/services/jmx/port/UserServiceImpl
> * http://localhost:8080/services/jmx/port/CustomerServiceImpl
> * http://localhost:8080/services/jmx/port/SoapPort
> *
> * Search by domain
> * http://localhost:8080/services/jmx/domain/org.apache.cxf
> *
> * Function 4:
> * http://localhost:8080/services/jmx/list
> *
> * */
>
> Source is also attached.
> Thank you very much on the reviewing, I am very dedicated to what you have
> mentioned. Though there was a time in last 2 days, I reformatted the code a
> little too much:)
>
>
> Regards:
> Shenglin Qiu
>
>
>> Date: Wed, 8 Jun 2011 10:23:58 +0100
>> Subject: Re: Expose MBeans in CXF
>> From: sberyoz...@gmail.com
>> To: dabaip...@hotmail.com
>> CC: dev@cxf.apache.org
>>
>> Hi Shenglin
>>
>> I'm not sure if I'm confusing you with the comments I'm making...The
>> source has become cleaner, good progress there,
>> but I'm not sure what are you doing there at all now.
>> Why MBeanCollection has become a subresource ? What happened to our
>> /mbean/{id} handler ? Recall, you had a subresource locator method
>> dealing with '/mbean/{id}' and you were delegating to MBean to finish
>> the processing and the only thing I suggested is to have a dedicated
>> MBeanResource handler which I guess should wrap an individual MBean
>> and work with it (get its state for now, etc)
>>
>> What does
>>
>> http://localhost:8080/services/jmx/property/id/0
>>
>> mean ?
>>
>> it has to be
>>
>> http://localhost:8080/services/jmx/mbean/0
>> http://localhost:8080/services/jmx/mbean/1
>>
>> and MBeanResource needs to deal with these URIs.
>>
>> Please check my previous email.
>> have only
>> 1. list all MBeans in org.apache.cxf domain
>> /list
>> 2. find MBeans in org.apache.cxf domain which have a given attribute value
>> /{attribute}/{value}
>> 3. find MBeans in org.apache.cxf domain their object names
>> /objectname/{objectname}
>> 4. get the represenation of the individual MBean (and later update it,
>> register event listeners, etc)
>> /mbean/{id}
>>
>> // we haven't discussed it yet - need to be done
>> 5. Return the counter statistics
>>
>> /counters
>>
>> 6. Counters for a specific endpoint
>>
>> /counters/{servicename}
>>
>> Sergey
>>
>> On Wed, Jun 8, 2011 at 5:46 AM, Shenglin Qiu <dabaip...@hotmail.com>
>> wrote:
>> > Hi Sergey:
>> >
>> > I attached the src and due to I am working on the mvn project only sync
>> > to
>> > svn, so I will take some time and sync this to github.
>> >
>> > And here is an update on the request:
>> >
>> > Self-defined demo inbound service:
>> > http://localhost:8080/services/greeter?wsdl
>> > http://localhost:8080/services/customerservice/customers
>> > http://localhost:8080/services/customerservice/customer/firstname/Lois
>> > http://localhost:8080/services/customerservice/customer/id/1
>> > http://localhost:8080/services/userservice/users
>> > http://localhost:8080/services/userservice/user/1
>> > /**
>> > * Specification:
>> > * Function 1:
>> > * Jmx Server:
>> > * sub-resource locator
>> > * http://localhost:8080/services/jmx/property/id/0
>> > * http://localhost:8080/services/jmx/property/id/1
>> > * ...
>> > *
>> > *
>> > * Function 2:
>> > * Note: search by bus.id, bus.id will be changed on every time
>> > server
>> > had been bounced.
>> > * http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>> > *
>> >
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,*
>> > * ...
>> > *
>> >
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:port=%22CustomerServiceImpl%22,*
>> > *
>> >
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,port=%22SoapPort%22,type=Bus.Service.Endpoint,*
>> > *
>> >
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:type=Bus.Service.Endpoint,*
>> > *
>> >
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,service=%22%7Bhttp%3A%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl%22,type=Bus.Service.Endpoint,*
>> > *
>> >
>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:service=%22%7Bhttp%3A%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl%22,*
>> > *
>> > * Function 3:
>> > * Search by service:
>> > *
>> >
>> > http://localhost:8080/services/jmx/property/service/%7Bhttp:%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl
>> > *
>> >
>> > http://localhost:8080/services/jmx/property/service/%7Bhttp:%2F%2Fserver.gsoc.apache.org%2F%7DUserServiceImpl
>> > *
>> > * Note, above queries are searching these 2, but replace by url
>> > encoding
>> > from:
>> > * {http://server.gsoc.apache.org/}UserServiceImpl
>> > * {http://server.gsoc.apache.org/}CustomerServiceImpl
>> > *
>> > * Search by type:
>> > *
>> > http://localhost:8080/services/jmx/property/type/Bus.Service.Endpoint
>> > *
>> > * Search by port:
>> > * http://localhost:8080/services/jmx/property/port/UserServiceImpl
>> > *
>> > http://localhost:8080/services/jmx/property/port/CustomerServiceImpl
>> > * http://localhost:8080/services/jmx/property/port/SoapPort
>> > *
>> > * Function 4:
>> > * http://localhost:8080/services/jmx/list
>> > *
>> > * */
>> >
>> > I will ping you tomorrow morning.
>> >
>> > Thank you.
>> > Regards:
>> >
>> > Shenglin Qiu
>> >
>> >
>> >> Date: Wed, 1 Jun 2011 15:32:30 +0100
>> >> Subject: Re: Expose MBeans in CXF
>> >> From: sberyoz...@gmail.com
>> >> To: dev@cxf.apache.org
>> >>
>> >> Hi Shenglin
>> >>
>> >> It's a step in the right direction, thanks, but JMXServer still needs
>> >> to be cleaned up quite a bit.
>> >> - as I said earlier you have 4 big functions basically duplicating
>> >> each other. We can't have a method per every attribute a given MBean
>> >> may have. Have a single function only, max two (one capturing all the
>> >> attributes, another one - dedicated to objectname/{value}).
>> >> - Introduce MBeanResource subresource instead of overloading MBean, it
>> >> will be cleaner and easier to work with
>> >> - as suggested earlier, update the domain MBean and/or MBeanAttribute
>> >> such that you could drop a big chunk of code in JMXServer where
>> >> individual attribute values are set, currently in getMbeanMap (if
>> >> (attrName.equals("service") then/else)). That is because a number of
>> >> attributes is basically unlimited, even though we have some well-known
>> >> ones
>> >>
>> >> - getMBeanMap - this is so complex I can't understand what it does. I
>> >> do understand it returns a thread-safe Map - but I'm finding it
>> >> difficult to understand how the method achieves that. Please remove
>> >> all those synchronized blocks and have ConcurrentHashMap. Have atomic
>> >> integer counter, synchronized exlicitly (in a block) if you prefer,
>> >> and try to get a simple and effective function implemented. Don't make
>> >> the logic of that function dependent on a n
>> >> I don't understand why you use "list" for creating MBean hrefs;
>> >>
>> >> - there's no need for having Mbean.id and MBean.href because
>> >> MBean.href is identifying a given MBean uniquely and thus you could
>> >> use that calculated href as a key.
>> >>
>> >> Thanks, Sergey
>> >>
>> >>
>> >> On Tue, May 31, 2011 at 5:36 PM, Shenglin Qiu <dabaip...@hotmail.com>
>> >> wrote:
>> >> >
>> >> > Hi Sergey:
>> >> >
>> >> > Project has been synced to Github:
>> >> >
>> >> > Browser:
>> >> > https://github.com/dabaipang/services
>> >> >
>> >> >
>> >> > Git address:
>> >> > https://dabaip...@github.com/dabaipang/services.git
>> >> > or
>> >> > g...@github.com:dabaipang/services.git
>> >> >
>> >> >
>> >> > Thank you.
>> >> >
>> >> >
>> >> > Regards:
>> >> > Shenglin Qiu
>> >> >
>> >> >
>> >> >
>> >> >> From: dabaip...@hotmail.com
>> >> >> To: sberyoz...@gmail.com
>> >> >> CC: dev@cxf.apache.org
>> >> >> Subject: RE: Expose MBeans in CXF
>> >> >> Date: Tue, 31 May 2011 11:23:38 -0400
>> >> >>
>> >> >>
>> >> >> Hi Sergey:
>> >> >>
>> >> >> > > Function 1: (0 - 5 is continuous, no gap)
>> >> >> > > Jmx Server:
>> >> >> > > sub-resource locator
>> >> >> > > http://localhost:8080/services/jmx/mbean/0
>> >> >> > > http://localhost:8080/services/jmx/mbean/1
>> >> >> >
>> >> >> > The id allocation (0, 1, etc) has to be thread safe
>> >> >>
>> >> >> Done with:
>> >> >> private synchronized Map<String, MBean> getMBeansMap(){
>> >> >> ....
>> >> >> }
>> >> >> My reason not using synchronized block but using synchronized method
>> >> >> is:
>> >> >> this guarantees the order on mbeansMap's initialization, so when
>> >> >> it's
>> >> >> called by a lot of requests, the followings will have to wait, unit
>> >> >> the
>> >> >> first one finished, after that, following requests will see
>> >> >> mbeansMap has
>> >> >> been initialized, and simply return mbeansMap without initializing
>> >> >> it again.
>> >> >>
>> >> >>
>> >> >> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>> >> >> > or
>> >> >> > http://localhost:8080/services/jmx/objectname/org.apache.cxf
>> >> >> >
>> >> >> > should also work
>> >> >> >
>> >> >> > >
>> >> >>
>> >> >> http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>> >> >> is working.
>> >> >>
>> >> >> > > Due to the URL duplication with /mbean/{id}, I can't put
>> >> >> > > {searchtype}/{query}, so I put things like:
>> >> >> >
>> >> >> > Personally I'd prefer to keep a URI as minimal as possible.
>> >> >> >
>> >> >> > /mbean/{id}
>> >> >> > and
>> >> >> > {searchtype}/{query}
>> >> >> >
>> >> >> > do not duplicate each other given that /mbean/{id} is more
>> >> >> > specific
>> >> >> > than {searchtype}/{query}.
>> >> >> > Likewise /objectname/{objectname} is more specific than
>> >> >> > {searchtype}/{query}, so consider dropping /search/ part,
>> >> >> > we may have a dedicated /search handler later on.
>> >> >>
>> >> >> Done by dedicating each request per method:
>> >> >> @GET
>> >> >> @POST
>> >> >> @Path("/list")
>> >> >> public MBeanCollection list()
>> >> >>
>> >> >> @Path("/mbean/")
>> >> >> public MBean locateMBean()
>> >> >>
>> >> >>
>> >> >> @GET
>> >> >> @POST
>> >> >> @Path("/type/{query}")
>> >> >> public MBeanCollection searchMBeansByType(@PathParam("query")
>> >> >> String query)
>> >> >>
>> >> >>
>> >> >> @GET
>> >> >> @POST
>> >> >> @Path("/port/{query}")
>> >> >> public MBeanCollection searchMBeansByPort(@PathParam("query")
>> >> >> String query)
>> >> >>
>> >> >>
>> >> >> @GET
>> >> >> @POST
>> >> >> @Path("/service/{query}")
>> >> >> public MBeanCollection searchMBeansByService(@PathParam("query")
>> >> >> String query)
>> >> >>
>> >> >>
>> >> >> @GET
>> >> >> @POST
>> >> >> @Path("/objectname/{objectname}")
>> >> >> public MBeanCollection getComponent(@PathParam("objectname")
>> >> >> String objectname)
>> >> >>
>> >> >> I remembered you mentioned to handle Exceptions once, currently I am
>> >> >> still 'throws' -ing them out, I will make the try catch block.
>> >> >> Should I also add a new object which display the error or simply
>> >> >> return
>> >> >> empty?
>> >> >>
>> >> >>
>> >> >> Thank you very much.
>> >> >>
>> >> >>
>> >> >> Regards:
>> >> >> Shenglin Qiu
>> >> >>
>> >> >>
>> >> >> > Date: Tue, 31 May 2011 10:35:36 +0100
>> >> >> > Subject: Re: Expose MBeans in CXF
>> >> >> > From: sberyoz...@gmail.com
>> >> >> > To: dev@cxf.apache.org
>> >> >> >
>> >> >> > Hi Shenglin
>> >> >> >
>> >> >> > Good progress, some comments below
>> >> >> > >
>> >> >> > > Function 1: (0 - 5 is continuous, no gap)
>> >> >> > > Jmx Server:
>> >> >> > > sub-resource locator
>> >> >> > > http://localhost:8080/services/jmx/mbean/0
>> >> >> > > http://localhost:8080/services/jmx/mbean/1
>> >> >> >
>> >> >> > The id allocation (0, 1, etc) has to be thread safe
>> >> >> >
>> >> >> > >
>> >> >> > >
>> >> >> > > Function 2:
>> >> >> > > Note: search by bus.id, bus.id will be changed on every time
>> >> >> > > server
>> >> >> > > had been
>> >> >> > > bounced.
>> >> >> > >
>> >> >> > >
>> >> >> > > http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,*
>> >> >> > > ...
>> >> >> > that is ok, a user does not have to specify them as you
>> >> >> > demonstrated
>> >> >> > with queries like
>> >> >> >
>> >> >> > >
>> >> >> > >
>> >> >> > > http://localhost:8080/services/jmx/objectname/org.apache.cxf:port=%22CustomerServiceImpl%22,*
>> >> >> > I guess
>> >> >> >
>> >> >> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>> >> >> > or
>> >> >> > http://localhost:8080/services/jmx/objectname/org.apache.cxf
>> >> >> >
>> >> >> > should also work
>> >> >> >
>> >> >> > >
>> >> >> > >
>> >> >> > > I am only now use 2 methods to fulfill:
>> >> >> > > 1 method for service/{serivce}, port/{port}, type/{type}
>> >> >> > > 1 method for objectname/{objectname}
>> >> >> >
>> >> >> > OK
>> >> >> >
>> >> >> > >
>> >> >> > > Due to the URL duplication with /mbean/{id}, I can't put
>> >> >> > > {searchtype}/{query}, so I put things like:
>> >> >> >
>> >> >> > Personally I'd prefer to keep a URI as minimal as possible.
>> >> >> >
>> >> >> > /mbean/{id}
>> >> >> > and
>> >> >> > {searchtype}/{query}
>> >> >> >
>> >> >> > do not duplicate each other given that /mbean/{id} is more
>> >> >> > specific
>> >> >> > than {searchtype}/{query}.
>> >> >> > Likewise /objectname/{objectname} is more specific than
>> >> >> > {searchtype}/{query}, so consider dropping /search/ part,
>> >> >> > we may have a dedicated /search handler later on.
>> >> >> >
>> >> >> > The other thing I forgot to mention, please remove all those
>> >> >> > System.out.println, ping me please if you need some help with
>> >> >> > setting
>> >> >> > up a remote debugging session
>> >> >> >
>> >> >> > Thanks, Sergey
>> >> >> >
>> >> >> > >
>> >> >> > > Thank you so much!
>> >> >> > >
>> >> >> > > Regards:
>> >> >> > > Shenglin Qiu
>> >> >> > >
>> >> >>
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Sergey Beryozkin
>> >>
>> >> Application Integration Division of Talend
>> >> http://sberyozkin.blogspot.com
>> >
>>
>>
>>
>> --
>> Sergey Beryozkin
>>
>> Application Integration Division of Talend
>> http://sberyozkin.blogspot.com
>
--
Sergey Beryozkin
Application Integration Division of Talend
http://sberyozkin.blogspot.com
