camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Claus Ibsen (JIRA)" <>
Subject [jira] Commented: (CAMEL-1472) Lucene Component
Date Mon, 04 Jan 2010 19:37:16 GMT


Claus Ibsen commented on CAMEL-1472:


*Having the following comments, in a bit of messy order as its type as I looked at the code:*
1) Why does JettyConfiguration have a LuceneComponent field? I would assume a xxConfiguration
is a simple object that is like a getter/setter that can easily be configured in Spring XML
but also can do some URL parsing as well. It should likely also have a default no-arg ctr
so you can create it from spring XML more easily. Ah you should probably pass the Component
in the {{parseURL}} method so you can use it locally only in this method and leave all the
getter/setter for easy Spring XML config as well.
2) The code for parsing the authority could be a bit smarter, however we only have 2 operations
so I assume it do. 
3) I dont like the default of the indexDir to be {{target/index}} which smells like a maven
folder for unit testing
4) The getMaxHits code could be prettier, eg use that getAndRemoveParameter and then put it
in settings afterards. Then you can use Camel Type convert to get it as a integer and avoid
the ugly JDK converter and type case code
5) The operation parameter can be an enum and Camel can auto convert to it from a String (case
insensitve AFAIR). Just a note.
6) No need to INFO log creating a producer. camel-core have logging for creating consumers/producers
already. And INFO logging is to high and the logging info does not provide good details anyway.
So please remove it.
7) LuceneEndpoint is preferred to have a default no arg ctr as well and have getter/setter
to configure it so you can create that endpoint easily as well. There are some components
which is not possible like that, eg Jetty, Http etc. But this one is simple enough for that.
8) Why is the endpoint *non singleton*? Most endpoints can be singleton out of the box
9) The debug loggin in LucenIndexer looks like TRACE logging to me. Try to mimimize DEBUG
logging a bit and use TRACE for the very verbose stuff
10) I wonder if the body *cannot* be converted to String should the indexer skip the entire
Exchange? Or should some exception be thrown. You can use getMandatoryBody if the body must
11) LuceneProducer. Remove that INFO logging which has no value. camel-core has logging already
for starting and stopping resource in Camel
12) I dont think you need to store endpoint in LuceneProducer as the super got it already
13) Dont use NPE exception but throw an IllegalArgumentException instead if some parameters
or value is missing. 
14) LucenseSearcher again use TRACE logging for that kind of logging
15) Dont use IOConverter directly to just do a new File(folder) to avoid that hard dependency
16) LuceneQueryProcessor - Dont swallow exceptions and dont use e.printStrackTrace(). This
error seems so severe that the exception should be rethrown
17) Remove code that is commented out
18) Why does the search return data as a XML String. Why not have some POJO class with the
search result. And let end user marshal to XML if you *really* need XML. I am not keen on

And use isXXXEnabled for those loggers to avoid overhead when that level is not enabled

> Lucene Component
> ----------------
>                 Key: CAMEL-1472
>                 URL:
>             Project: Apache Camel
>          Issue Type: New Feature
>            Reporter: Claus Ibsen
>            Assignee: Ashwin Karpe
>             Fix For: Future
>         Attachments: camel-lucene-20100102.patch,
> We should add a new component for Apache Lucene integration

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message