IndexFileNames

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

IndexFileNames

Doug Cutting
[hidden email] wrote:

> --- lucene/java/trunk/src/java/org/apache/lucene/store/FSDirectory.java (original)
> +++ lucene/java/trunk/src/java/org/apache/lucene/store/FSDirectory.java Mon Jun  6 10:52:12 2005
> @@ -52,8 +52,8 @@
>          if (name.endsWith("."+IndexReader.FILENAME_EXTENSIONS[i]))
>            return true;
>        }
> -      if (name.equals("deletable")) return true;
> -      else if (name.equals("segments")) return true;
> +      if (name.equals(Constants.INDEX_DELETABLE_FILENAME)) return true;
> +      else if (name.equals(Constants.INDEX_SEGMENTS_FILENAME)) return true;
>        else if (name.matches(".+\\.f\\d+")) return true;
>        return false;

This really belongs in the index package.  That way, when we change the
set of files in an index, the changes will be localized.

So this, LuceneFileFilter, Constants.INDEX_* and
IndexReader.FILENAME_EXTENSIONS, should all be moved to a common home in
the index package, like org.apache.lucene.index.IndexFileNames.  Thoughts?

Doug

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: IndexFileNames

Bernhard Messer
Doug Cutting wrote:

> [hidden email] wrote:
>
>> ---
>> lucene/java/trunk/src/java/org/apache/lucene/store/FSDirectory.java
>> (original)
>> +++
>> lucene/java/trunk/src/java/org/apache/lucene/store/FSDirectory.java
>> Mon Jun  6 10:52:12 2005
>> @@ -52,8 +52,8 @@
>>          if (name.endsWith("."+IndexReader.FILENAME_EXTENSIONS[i]))
>>            return true;
>>        }
>> -      if (name.equals("deletable")) return true;
>> -      else if (name.equals("segments")) return true;
>> +      if (name.equals(Constants.INDEX_DELETABLE_FILENAME)) return true;
>> +      else if (name.equals(Constants.INDEX_SEGMENTS_FILENAME))
>> return true;
>>        else if (name.matches(".+\\.f\\d+")) return true;
>>        return false;
>
>
> This really belongs in the index package.  That way, when we change
> the set of files in an index, the changes will be localized.
>
> So this, LuceneFileFilter, Constants.INDEX_* and
> IndexReader.FILENAME_EXTENSIONS, should all be moved to a common home
> in the index package, like org.apache.lucene.index.IndexFileNames.  
> Thoughts?
>
yes, this makes sense. I will try to pack all the spreaded filenames and
extensions together in a new class
org.apache.lucene.index.IndexFileNames. So we have everything in one
place and it will be much easier to maintain or even to change.

Bernhard


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: IndexFileNames

Bernhard Messer
Bernhard Messer wrote:

> Doug Cutting wrote:
>
>> [hidden email] wrote:
>>
>>> ---
>>> lucene/java/trunk/src/java/org/apache/lucene/store/FSDirectory.java
>>> (original)
>>> +++
>>> lucene/java/trunk/src/java/org/apache/lucene/store/FSDirectory.java
>>> Mon Jun  6 10:52:12 2005
>>> @@ -52,8 +52,8 @@
>>>          if (name.endsWith("."+IndexReader.FILENAME_EXTENSIONS[i]))
>>>            return true;
>>>        }
>>> -      if (name.equals("deletable")) return true;
>>> -      else if (name.equals("segments")) return true;
>>> +      if (name.equals(Constants.INDEX_DELETABLE_FILENAME)) return
>>> true;
>>> +      else if (name.equals(Constants.INDEX_SEGMENTS_FILENAME))
>>> return true;
>>>        else if (name.matches(".+\\.f\\d+")) return true;
>>>        return false;
>>
>>
>>
>> This really belongs in the index package.  That way, when we change
>> the set of files in an index, the changes will be localized.
>>
>> So this, LuceneFileFilter, Constants.INDEX_* and
>> IndexReader.FILENAME_EXTENSIONS, should all be moved to a common home
>> in the index package, like org.apache.lucene.index.IndexFileNames.  
>> Thoughts?
>>
sorry for the confusion. On the first look, i thought the new class
IndexFileNames, containing the necessary constant values, fits perfect
into org.apache.lucene.index. After a more detailed look, i get the
feeling that it would be much better to place the new class into
org.apache.store. If done, we can avoid all dependencies within
FSDirectory to org.apache.lucene.index, which is very clean.

Why not creating a new public final class
org.apache.lucene.store.IndexFileNames and move LuceneFileFilter,
Constants.INDEX_*, SegmentMerger.COMPOUND_EXTENSIONS,
SegmentMerger.VECTOR_EXTENSIONS and IndexReader.FILENAME_EXTENSIONS to it.

Does it sound ok ?

Bernhard

>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: IndexFileNames

Doug Cutting
Bernhard Messer wrote:
> sorry for the confusion. On the first look, i thought the new class
> IndexFileNames, containing the necessary constant values, fits perfect
> into org.apache.lucene.index. After a more detailed look, i get the
> feeling that it would be much better to place the new class into
> org.apache.store. If done, we can avoid all dependencies within
> FSDirectory to org.apache.lucene.index, which is very clean.

I think that's an illusion: the store package would actually become more
dependent on the index package.  If someone changes the set of files in
an index then the changes will not be localized to the index package.
Nothing outside of the index package should know anything about the
internal structure of an index.

If insteaed the index package exposes a public API that permits other
packages to inquire whether particular file names belong to an index
then only a small dependency on what should be a stable API is exposed.
  Changes to index structure can be made without changing anything
outside of the index package.

> Why not creating a new public final class
> org.apache.lucene.store.IndexFileNames and move LuceneFileFilter,
> Constants.INDEX_*, SegmentMerger.COMPOUND_EXTENSIONS,
> SegmentMerger.VECTOR_EXTENSIONS and IndexReader.FILENAME_EXTENSIONS to it.

I still think this class should be in the index package.  I'm not
convinced that anything other than the FileFilter needs to be public.

Doug

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: IndexFileNames

Bernhard Messer
Doug Cutting wrote:

> Bernhard Messer wrote:
>
>> sorry for the confusion. On the first look, i thought the new class
>> IndexFileNames, containing the necessary constant values, fits
>> perfect into org.apache.lucene.index. After a more detailed look, i
>> get the feeling that it would be much better to place the new class
>> into org.apache.store. If done, we can avoid all dependencies within
>> FSDirectory to org.apache.lucene.index, which is very clean.
>
>
> I think that's an illusion: the store package would actually become
> more dependent on the index package.  If someone changes the set of
> files in an index then the changes will not be localized to the index
> package. Nothing outside of the index package should know anything
> about the internal structure of an index.
>
> If insteaed the index package exposes a public API that permits other
> packages to inquire whether particular file names belong to an index
> then only a small dependency on what should be a stable API is
> exposed.  Changes to index structure can be made without changing
> anything outside of the index package.
>
>> Why not creating a new public final class
>> org.apache.lucene.store.IndexFileNames and move LuceneFileFilter,
>> Constants.INDEX_*, SegmentMerger.COMPOUND_EXTENSIONS,
>> SegmentMerger.VECTOR_EXTENSIONS and IndexReader.FILENAME_EXTENSIONS
>> to it.
>
>
> I still think this class should be in the index package.  I'm not
> convinced that anything other than the FileFilter needs to be public.

I finished the changes and commited the changes. There are two new
classes in package org.apache.lucene.index.
org.apache.lucene.index.IndexFileNames contains common lucene related
filenames and extensions, the scope of the class itself and it's members
are package. org.apache.lucene.index.IndexFileFilter is public and used
in FSDirectory to decide whether a file belongs to an lucene index and
can be deleted.

Bernhard


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: IndexFileNames

Doug Cutting
Bernhard Messer wrote:
> I finished the changes and commited the changes. There are two new
> classes in package org.apache.lucene.index.
> org.apache.lucene.index.IndexFileNames contains common lucene related
> filenames and extensions, the scope of the class itself and it's members
> are package. org.apache.lucene.index.IndexFileFilter is public and used
> in FSDirectory to decide whether a file belongs to an lucene index and
> can be deleted.

This looks great!

Thanks,

Doug

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]