Thanks for taking a look at this Simon.
Why do you reformat all the stuff in Field, is that necessary here at all? I mean its needed eventually but for the deprecation of things it only bloats the patch really doesn't it?
Because for me this issue is about reducing the complexity of these classes and Field is a mess. Making it more readable reduces the complexity. If needs be I will do this in two patches, but I don't feel this issue is resolved till the code in Field is readable.
When you deprecate AbstractField and Fieldable, Field should ideally be a standalone class. So I see that this still needs to subclass Fieldable / AbstractField but could it stand alone now so that we can simply remove the extends / implements on Field once we drop things in 4.0? I think it looks good from looking at the patch though
I don't really understand what you're suggesting here. In 3x where the deprecations will be occurring Field has to continue to extend AbstractField. Yes in 4.0 we can drop that extension but addressing the deprecations is not in the scope of 3x.
I don't like the name getAllFields on Document since it implies that we have a getPartialFields or something. I see that you can not use getFields since it only differs in return type which doesn't belong to the signature though. Maybe we should implement Iterable<Field> here and offer an additional method getFieldsAsList or maybe getFields(List<Field> fields)
Yeah good call. I think implementing Iterable<Field> is best, but it will also require adding a count() method to Document since often people retrieve the List to get the number of fields.
once we have this in what are the next steps towards FieldType? Will we have only one class Field that is backed by a FieldType but still offers the methods it has now? Or doe we have two totally new classes FieldTyps and FieldValue
Once FieldType is in, all the various metadata properties (isIndexed, isStored etc) will be moved to FieldType, leaving Field as what you suggest as FieldValue. Field will contain its type, boost, name, value. If we have Analyzers on FieldTypes, then we will be able to remove the TokenStream from Field.
I wonder if this patch raises tons of deprecation warnings all over lucene where Fieldable was used? In IW we use it all over the place though. We must fix that in this issue too otherwise uwe will go mad I guess
Yeah but not in 3x unfortunately. As it stands people can retrieve the List of Fieldables via getFields() and add whatever implementation of Fieldable they like. Consequently we need to continue to support Fieldable in IW for example. Once this code has been committed I will create a new patch for trunk which moves all of Solr and Lucene over to the Field. I could do this in many places already of course, but that core classes like IW would have to remain as they are.
I will wait for your thoughts on the reformating and then make a new patch.