[
https://issues.apache.org/activemq/browse/CAMEL-1472?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=56743#action_56743
]
Claus Ibsen commented on CAMEL-1472:
------------------------------------
Ashwin
*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 exist.
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 this.
And use isXXXEnabled for those loggers to avoid overhead when that level is not
enabled
> Lucene Component
> ----------------
>
> Key: CAMEL-1472
> URL: https://issues.apache.org/activemq/browse/CAMEL-1472
> Project: Apache Camel
> Issue Type: New Feature
> Reporter: Claus Ibsen
> Assignee: Ashwin Karpe
> Fix For: Future
>
> Attachments: camel-lucene-20100102.patch, camel-lucene-20100102.zip
>
>
> 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.
