magibney commented on a change in pull request #513:
URL: https://github.com/apache/solr/pull/513#discussion_r788339026
##########
File path:
solr/core/src/java/org/apache/solr/response/transform/TransformerFactory.java
##########
@@ -40,6 +41,47 @@ public void init(NamedList<?> args) {
public abstract DocTransformer create(String field, SolrParams params,
SolrQueryRequest req);
+ /**
+ * The {@link FieldRenamer} interface should be implemented by any {@link
TransformerFactory} capable of generating
+ * transformers that might rename fields, and should implement {@link
#create(String, SolrParams, SolrQueryRequest, Map, Set)}
+ * in place of {@link #create(String, SolrParams, SolrQueryRequest)} (with
the latter method
+ * overridden to throw {@link UnsupportedOperationException}).
+ *
+ * {@link DocTransformer}s returned via {@link #create(String, SolrParams,
SolrQueryRequest, Map, Set)}
+ * will be added in a second pass, allowing simplified logic in {@link
TransformerFactory#create(String, SolrParams, SolrQueryRequest)}
+ * for non-renaming factories.
+ *
+ * {@link #create(String, SolrParams, SolrQueryRequest, Map, Set)} must
implement extra logic to be aware of
+ * preceding field renames, and to make subsequent {@link FieldRenamer}
transformers aware of its own field renames.
+ *
+ * It is harmless for a {@link DocTransformer} that does _not_ in practice
rename fields to be returned from a
+ * factory that implements this interface (e.g., for conditional renames?);
but doing so opens the possibility of
+ * {@link #create(String, SolrParams, SolrQueryRequest, Map, Set)} being
called _after_ fields have been renamed,
+ * so such implementations must still check whether the field with which
they are concerned has been renamed ...
+ * and if it _has_, must copy the field back to its original name. This
situation also demonstrates the
+ * motivation for separating the creation of {@link DocTransformer}s into
two phases: an initial phase involving
+ * no field renames, and a subsequent phase that implement extra logic to
properly handle field renames.
+ */
+ public interface FieldRenamer {
Review comment:
I considered this (and I guess I'm still considering it). Making this a
subclass would clarify the intent, and the expense of some (albeit not
particularly useful?) flexibility. I'll think about this in conjunction with
the above feedback about the "rename checking" in the `[geo]` transformer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
