You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lenya.apache.org by ch...@apache.org on 2005/12/13 06:50:05 UTC
svn commit: r356483 -
/lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
Author: chestnut
Date: Mon Dec 12 21:50:01 2005
New Revision: 356483
URL: http://svn.apache.org/viewcvs?rev=356483&view=rev
Log:
fixes broken compilation: IndexConfiguration (Bug 37873)
Thanks to Felix Röthenbacher for the patch
Modified:
lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
Modified: lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
URL: http://svn.apache.org/viewcvs/lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java?rev=356483&r1=356482&r2=356483&view=diff
==============================================================================
--- lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java (original)
+++ lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java Mon Dec 12 21:50:01 2005
@@ -20,10 +20,10 @@
import java.io.File;
import java.io.IOException;
-import org.apache.avalon.excalibur.io.FileUtil;
import org.apache.avalon.framework.configuration.Configuration;
import org.apache.avalon.framework.configuration.ConfigurationException;
import org.apache.avalon.framework.configuration.DefaultConfigurationBuilder;
+import org.apache.commons.io.FilenameUtils;
import org.apache.log4j.Logger;
import org.xml.sax.SAXException;
@@ -155,9 +155,11 @@
* @return A string.
*/
public String resolvePath(String path) {
- if (path.indexOf(File.separator) == 0) {
- return path;
+ File pathName = new File(path);
+ if (pathName.isAbsolute()) {
+ return pathName.getAbsolutePath();
+ } else {
+ return FilenameUtils.concat(this.configurationFilePath, path);
}
- return FileUtil.catPath(this.configurationFilePath, path);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@lenya.apache.org
For additional commands, e-mail: commits-help@lenya.apache.org
Re: svn commit: r356483 - /lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
Posted by Felix Röthenbacher <fe...@wyona.com>.
Hi Antonio
You are not grumpy. See my comments inline
Antonio Gallardo wrote:
> chestnut@apache.org wrote:
>
>> Author: chestnut
>> Date: Mon Dec 12 21:50:01 2005
>> New Revision: 356483
>>
>> URL: http://svn.apache.org/viewcvs?rev=356483&view=rev
>> Log:
>> fixes broken compilation: IndexConfiguration (Bug 37873)
>> Thanks to Felix Röthenbacher for the patch
>>
>>
>> Modified:
>>
>> lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
>>
>>
>> Modified:
>> lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
>>
>> URL:
>> http://svn.apache.org/viewcvs/lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java?rev=356483&r1=356482&r2=356483&view=diff
>>
>> ==============================================================================
>>
>> ---
>> lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
>> (original)
>> +++
>> lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
>> Mon Dec 12 21:50:01 2005
>> @@ -20,10 +20,10 @@
>> import java.io.File;
>> import java.io.IOException;
>>
>> -import org.apache.avalon.excalibur.io.FileUtil;
>> import org.apache.avalon.framework.configuration.Configuration;
>> import org.apache.avalon.framework.configuration.ConfigurationException;
>> import
>> org.apache.avalon.framework.configuration.DefaultConfigurationBuilder;
>> +import org.apache.commons.io.FilenameUtils;
>> import org.apache.log4j.Logger;
>> import org.xml.sax.SAXException;
>>
>> @@ -155,9 +155,11 @@
>> * @return A string.
>> */
>> public String resolvePath(String path) {
>> - if (path.indexOf(File.separator) == 0) {
>> - return path;
>> + File pathName = new File(path);
>> + if (pathName.isAbsolute()) {
>> + return pathName.getAbsolutePath();
>> + } else {
>> + return FilenameUtils.concat(this.configurationFilePath,
>> path);
>> }
>> - return FileUtil.catPath(this.configurationFilePath, path);
>> }
>> }
>>
>>
>>
> Sorry for being grumpy. ;-)
>
> There is no need to create a "new File" inside this method.
> It only have a performance penality.
The method is rarely called, I can't see any performance penalty here.
Basically, creating a File object is nothing bad, it's just an
accessor object, no system resources are involved in this stage.
>Can we don just replace:
>
> return FileUtil.catPath(this.configurationFilePath, path);
>
> with:
>
> return FilenameUtils.concat(this.configurationFilePath, path);
>
> and keep the method as it was?
Quote from
http://jakarta.apache.org/commons/io/apidocs/org/apache/commons/io/FilenameUtils.html
"NOTE: You may be able to avoid using this class entirely simply by
using JDK File objects and the two argument constructor
File(File,String)."
There is no need to use this helper class, it's just an additional
dependency for a plain debug message. So I would suggest to avoid its
usage at all.
There was a check for absolute paths in the original method:
>> public String resolvePath(String path) {
>> - if (path.indexOf(File.separator) == 0) {
>> - return path;
I replaced it with a simpler, more readable statement.
You are right, this check is unnecessary. But we should check wether
this.configurationFilePath is a directory or a file otherwise
we end up with something that will not work.
Best
Felix
>
> WDYT?
>
> Best Regards,
>
> Antonio Gallardo.
>
--
Felix Röthenbacher felix.roethenbacher@wyona.com
Wyona Inc. - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: svn commit: r356483 - /lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
Posted by Antonio Gallardo <ag...@agssa.net>.
chestnut@apache.org wrote:
>Author: chestnut
>Date: Mon Dec 12 21:50:01 2005
>New Revision: 356483
>
>URL: http://svn.apache.org/viewcvs?rev=356483&view=rev
>Log:
>fixes broken compilation: IndexConfiguration (Bug 37873)
>Thanks to Felix Röthenbacher for the patch
>
>
>Modified:
> lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
>
>Modified: lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java
>URL: http://svn.apache.org/viewcvs/lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java?rev=356483&r1=356482&r2=356483&view=diff
>==============================================================================
>--- lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java (original)
>+++ lenya/trunk/src/modules/search/java/src/org/apache/lenya/lucene/IndexConfiguration.java Mon Dec 12 21:50:01 2005
>@@ -20,10 +20,10 @@
> import java.io.File;
> import java.io.IOException;
>
>-import org.apache.avalon.excalibur.io.FileUtil;
> import org.apache.avalon.framework.configuration.Configuration;
> import org.apache.avalon.framework.configuration.ConfigurationException;
> import org.apache.avalon.framework.configuration.DefaultConfigurationBuilder;
>+import org.apache.commons.io.FilenameUtils;
> import org.apache.log4j.Logger;
> import org.xml.sax.SAXException;
>
>@@ -155,9 +155,11 @@
> * @return A string.
> */
> public String resolvePath(String path) {
>- if (path.indexOf(File.separator) == 0) {
>- return path;
>+ File pathName = new File(path);
>+ if (pathName.isAbsolute()) {
>+ return pathName.getAbsolutePath();
>+ } else {
>+ return FilenameUtils.concat(this.configurationFilePath, path);
> }
>- return FileUtil.catPath(this.configurationFilePath, path);
> }
> }
>
>
>
Sorry for being grumpy. ;-)
There is no need to create a "new File" inside this method. It only have
a performance penality. Can we don just replace:
return FileUtil.catPath(this.configurationFilePath, path);
with:
return FilenameUtils.concat(this.configurationFilePath, path);
and keep the method as it was?
WDYT?
Best Regards,
Antonio Gallardo.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org