[ 
https://issues.apache.org/jira/browse/SOLR-6645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Fabio Piro updated SOLR-6645:
-----------------------------
    Description: 
Hello good people.

It is understandable that the priority of SolrJ is to provide a stable API for 
java and not a rich-feature client, I'm well aware of that. On the other hand 
more features nowadays mean most of the time Spring Solr Data. Although I 
appreciate the enrichment work of that lib, sometimes depending on its 
monolithic dependencies and magic is not a valid option.

So, I was thinking that the official DocumentObjectBinder could benefit from 
some love, and I had implemented a listener pattern for the annotations. 

You can register your annotations and they relate listeners in the binder, and 
it will invoke the corresponding method in the listener on getBean and on 
toSolrInputDocument, therefore granting the chance to do something during the 
ongoing process.

Changes are:

* [MOD] */beans/DocumentObjectBinder*:  The new logic and a new constructor for 
registering the annotations
* [ADD] */impl/AccessorAnnotationListener*: Abstract utility class with the 
former get(), set(), isArray, isList, isContainedInMap etc...
* [ADD] */impl/FieldAnnotationListener*: all the rest of DocField for dealing 
with @Field
* [ADD] */AnnotationListener*: the base listener class
* [MOD] */SolrServer*: added setBinder (this is the only tricky change, I hope 
it's not a problem).

It's all well documented and the code is very easy to read. Tests are all 
green, it should be 100% BC and the performance impact is void (the logic flow 
is exactly the same as now, and I only changed the bare essentials and nothing 
more, anyway).

Some Examples (they are not part of the pull-request):

The long awaited @FieldObject in 4 lines of code:

https://issues.apache.org/jira/browse/SOLR-1945

{code:java}
public class FieldObjectAnnotationListener extends 
AccessorAnnotationListener<FieldObject> {

    public FieldObjectAnnotationListener(AnnotatedElement element, FieldObject 
annotation) {
        super(element, annotation);
    }

    @Override
    public void onGetBean(Object obj, SolrDocument doc, DocumentObjectBinder 
binder) {
        Object nested = binder.getBean(target.clazz, doc);
        setTo(obj, nested);
    }

    @Override
    public void onToSolrInputDocument(Object obj, SolrInputDocument doc, 
DocumentObjectBinder binder) {
        SolrInputDocument nested = binder.toSolrInputDocument(getFrom(obj));

        for (Map.Entry<String, SolrInputField> entry : nested.entrySet()) {
            doc.addField(entry.getKey(), entry.getValue());
        }
    }
}
{code}

Or something entirely new like an annotation for ChildDocuments:

{code:java}
public class ChildDocumentsAnnotationListener extends 
AccessorAnnotationListener<ChildDocuments> {

    public ChildDocumentsAnnotationListener(AnnotatedElement element, 
ChildDocuments annotation) {
        super(element, annotation);

        if (!target.isInList || target.clazz.isPrimitive()) {
            throw new BindingException("@NestedDocuments is applicable only on 
List<Object>.");
        }
    }

    @Override
    public void onGetBean(Object obj, SolrDocument doc, DocumentObjectBinder 
binder) {
        List<Object> nested = new ArrayList<>();
        for (SolrDocument child : doc.getChildDocuments()) {
            nested.add(binder.getBean(target.clazz, child));// this should be 
recursive, but it's only an example
        }

        setTo(obj, nested);
    }

    @Override
    public void onToSolrInputDocument(Object obj, SolrInputDocument doc, 
DocumentObjectBinder binder) {
        SolrInputDocument nested = binder.toSolrInputDocument(getFrom(obj));
        doc.addChildDocuments(nested.getChildDocuments());
    }
}
{code}

In addition, all the logic is encapsulated in the listener, so you can make a 
custom FieldAnnotationListener too, and override the default one

{code:java}
public class CustomFieldAnnotationListener extends FieldAnnotationListener {

    private boolean isTransientPresent;

    public CustomFieldAnnotationListener(AnnotatedElement element, Field 
annotation) {
        super(element, annotation);
        this.isTransentPresent = element.isAnnotationPresent(Transient.class);
    }

    @Override
    public void onGetBean(Object obj, SolrDocument doc, DocumentObjectBinder 
binder) {
        if(!isTransientPresent){
            super.onGetBean(obj, doc, binder);
        }
    }

    @Override
    public void onToSolrInputDocument(Object obj, SolrInputDocument doc, 
DocumentObjectBinder binder) {
        if(!isTransientPresent){
            super.onToSolrInputDocument(obj, doc, binder);
        }
    }
}
{code}

I think this is a good solution for a win-win scenario:

- The code is almost unchanged (only shifted, wisely changed) so there is no 
new code to test and to support for the devs.

- People who only need the plain @Field will be untouched and unaware of the 
change.

- People who need some extra functionalities (like converters) can now easily 
extend the @Field logic, or make new annotations.

- With some luck, a Community-Driven repository with the most useful features 
can emerge.

- [Bonus] The binder is now entirely documented and not so scarier like before 
(we are almost at Halloween, but still... :) )

 The code is production-ready, if you are interested in merging it to the core, 
we can discuss the last touch (mainly what to leave protected and what to set 
private).

---
Update1: Added PATCH


  was:
Hello good people.

It is understandable that the priority of SolrJ is to provide a stable API for 
java and not a rich-feature client, I'm well aware of that. On the other hand 
more features nowadays mean most of the time Spring Solr Data. Although I 
appreciate the enrichment work of that lib, sometimes depending on its 
monolithic dependencies and magic is not a valid option.

So, I was thinking that the official DocumentObjectBinder could benefit from 
some love, and I had implemented a listener pattern for the annotations. 

You can register your annotations and they relate listeners in the binder, and 
it will invoke the corresponding method in the listener on getBean and on 
toSolrInputDocument, therefore granting the chance to do something during the 
ongoing process.

Changes are:

* [MOD] */beans/DocumentObjectBinder*:  The new logic and a new constructor for 
registering the annotations
* [ADD] */impl/AccessorAnnotationListener*: Abstract utility class with the 
former get(), set(), isArray, isList, isContainedInMap etc...
* [ADD] */impl/FieldAnnotationListener*: all the rest of DocField for dealing 
with @Field
* [ADD] */AnnotationListener*: the base listener class
* [MOD] */SolrServer*: added setBinder (this is the only tricky change, I hope 
it's not a problem).

It's all well documented and the code is very easy to read. Tests are all 
green, it should be 100% BC and the performance impact is void (the logic flow 
is exactly the same as now, and I only changed the bare essentials and nothing 
more, anyway).

Some Examples (they are not part of the pull-request):

The long awaited @FieldObject in 4 lines of code:

https://issues.apache.org/jira/browse/SOLR-1945

{code:java}
public class FieldObjectAnnotationListener extends 
AccessorAnnotationListener<FieldObject> {

    public FieldObjectAnnotationListener(AnnotatedElement element, FieldObject 
annotation) {
        super(element, annotation);
    }

    @Override
    public void onGetBean(Object obj, SolrDocument doc, DocumentObjectBinder 
binder) {
        Object nested = binder.getBean(target.clazz, doc);
        setTo(obj, nested);
    }

    @Override
    public void onToSolrInputDocument(Object obj, SolrInputDocument doc, 
DocumentObjectBinder binder) {
        SolrInputDocument nested = binder.toSolrInputDocument(getFrom(obj));

        for (Map.Entry<String, SolrInputField> entry : nested.entrySet()) {
            doc.addField(entry.getKey(), entry.getValue());
        }
    }
}
{code}

Or something entirely new like an annotation for ChildDocuments:

{code:java}
public class ChildDocumentsAnnotationListener extends 
AccessorAnnotationListener<ChildDocuments> {

    public ChildDocumentsAnnotationListener(AnnotatedElement element, 
ChildDocuments annotation) {
        super(element, annotation);

        if (!target.isInList || target.clazz.isPrimitive()) {
            throw new BindingException("@NestedDocuments is applicable only on 
List<Object>.");
        }
    }

    @Override
    public void onGetBean(Object obj, SolrDocument doc, DocumentObjectBinder 
binder) {
        List<Object> nested = new ArrayList<>();
        for (SolrDocument child : doc.getChildDocuments()) {
            nested.add(binder.getBean(target.clazz, child));// this should be 
recursive, but it's only an example
        }

        setTo(obj, nested);
    }

    @Override
    public void onToSolrInputDocument(Object obj, SolrInputDocument doc, 
DocumentObjectBinder binder) {
        SolrInputDocument nested = binder.toSolrInputDocument(getFrom(obj));
        doc.addChildDocuments(nested.getChildDocuments());
    }
}
{code}

In addition, all the logic is encapsulated in the listener, so you can make a 
custom FieldAnnotationListener too, and override the default one

{code:java}
public class CustomFieldAnnotationListener extends FieldAnnotationListener {

    private boolean isTransientPresent;

    public CustomFieldAnnotationListener(AnnotatedElement element, Field 
annotation) {
        super(element, annotation);
        this.isTransentPresent = element.isAnnotationPresent(Transient.class);
    }

    @Override
    public void onGetBean(Object obj, SolrDocument doc, DocumentObjectBinder 
binder) {
        if(!isTransientPresent){
            super.onGetBean(obj, doc, binder);
        }
    }

    @Override
    public void onToSolrInputDocument(Object obj, SolrInputDocument doc, 
DocumentObjectBinder binder) {
        if(!isTransientPresent){
            super.onToSolrInputDocument(obj, doc, binder);
        }
    }
}
{code}

I think this is a good solution for a win-win scenario:

- The code is almost unchanged (only shifted, wisely changed) so there is no 
new code to test and to support for the devs.

- People who only need the plain @Field will be untouched and unaware of the 
change.

- People who need some extra functionalities can now easily extend the @Field 
logic, or make new annotations.

- With some luck, a Community-Driven repository with the most useful features 
can emerge.

- [Bonus] The binder is now entirely documented and not so scarier like before 
(we are almost at Halloween, but still... :) )

 The code is production-ready, if you are interested in merging it to the core, 
we can discuss the last touch (mainly what to leave protected and what to set 
private).

---
Update1: Added PATCH



> Refactored DocumentObjectBinder and added AnnotationListeners
> -------------------------------------------------------------
>
>                 Key: SOLR-6645
>                 URL: https://issues.apache.org/jira/browse/SOLR-6645
>             Project: Solr
>          Issue Type: New Feature
>          Components: clients - java
>            Reporter: Fabio Piro
>              Labels: annotations, binder, listener, solrj
>         Attachments: SOLR-6645.patch
>
>
> Hello good people.
> It is understandable that the priority of SolrJ is to provide a stable API 
> for java and not a rich-feature client, I'm well aware of that. On the other 
> hand more features nowadays mean most of the time Spring Solr Data. Although 
> I appreciate the enrichment work of that lib, sometimes depending on its 
> monolithic dependencies and magic is not a valid option.
> So, I was thinking that the official DocumentObjectBinder could benefit from 
> some love, and I had implemented a listener pattern for the annotations. 
> You can register your annotations and they relate listeners in the binder, 
> and it will invoke the corresponding method in the listener on getBean and on 
> toSolrInputDocument, therefore granting the chance to do something during the 
> ongoing process.
> Changes are:
> * [MOD] */beans/DocumentObjectBinder*:  The new logic and a new constructor 
> for registering the annotations
> * [ADD] */impl/AccessorAnnotationListener*: Abstract utility class with the 
> former get(), set(), isArray, isList, isContainedInMap etc...
> * [ADD] */impl/FieldAnnotationListener*: all the rest of DocField for dealing 
> with @Field
> * [ADD] */AnnotationListener*: the base listener class
> * [MOD] */SolrServer*: added setBinder (this is the only tricky change, I 
> hope it's not a problem).
> It's all well documented and the code is very easy to read. Tests are all 
> green, it should be 100% BC and the performance impact is void (the logic 
> flow is exactly the same as now, and I only changed the bare essentials and 
> nothing more, anyway).
> Some Examples (they are not part of the pull-request):
> The long awaited @FieldObject in 4 lines of code:
> https://issues.apache.org/jira/browse/SOLR-1945
> {code:java}
> public class FieldObjectAnnotationListener extends 
> AccessorAnnotationListener<FieldObject> {
>     public FieldObjectAnnotationListener(AnnotatedElement element, 
> FieldObject annotation) {
>         super(element, annotation);
>     }
>     @Override
>     public void onGetBean(Object obj, SolrDocument doc, DocumentObjectBinder 
> binder) {
>         Object nested = binder.getBean(target.clazz, doc);
>         setTo(obj, nested);
>     }
>     @Override
>     public void onToSolrInputDocument(Object obj, SolrInputDocument doc, 
> DocumentObjectBinder binder) {
>         SolrInputDocument nested = binder.toSolrInputDocument(getFrom(obj));
>         for (Map.Entry<String, SolrInputField> entry : nested.entrySet()) {
>             doc.addField(entry.getKey(), entry.getValue());
>         }
>     }
> }
> {code}
> Or something entirely new like an annotation for ChildDocuments:
> {code:java}
> public class ChildDocumentsAnnotationListener extends 
> AccessorAnnotationListener<ChildDocuments> {
>     public ChildDocumentsAnnotationListener(AnnotatedElement element, 
> ChildDocuments annotation) {
>         super(element, annotation);
>         if (!target.isInList || target.clazz.isPrimitive()) {
>             throw new BindingException("@NestedDocuments is applicable only 
> on List<Object>.");
>         }
>     }
>     @Override
>     public void onGetBean(Object obj, SolrDocument doc, DocumentObjectBinder 
> binder) {
>         List<Object> nested = new ArrayList<>();
>         for (SolrDocument child : doc.getChildDocuments()) {
>             nested.add(binder.getBean(target.clazz, child));// this should be 
> recursive, but it's only an example
>         }
>         setTo(obj, nested);
>     }
>     @Override
>     public void onToSolrInputDocument(Object obj, SolrInputDocument doc, 
> DocumentObjectBinder binder) {
>         SolrInputDocument nested = binder.toSolrInputDocument(getFrom(obj));
>         doc.addChildDocuments(nested.getChildDocuments());
>     }
> }
> {code}
> In addition, all the logic is encapsulated in the listener, so you can make a 
> custom FieldAnnotationListener too, and override the default one
> {code:java}
> public class CustomFieldAnnotationListener extends FieldAnnotationListener {
>     private boolean isTransientPresent;
>     public CustomFieldAnnotationListener(AnnotatedElement element, Field 
> annotation) {
>         super(element, annotation);
>         this.isTransentPresent = element.isAnnotationPresent(Transient.class);
>     }
>     @Override
>     public void onGetBean(Object obj, SolrDocument doc, DocumentObjectBinder 
> binder) {
>         if(!isTransientPresent){
>             super.onGetBean(obj, doc, binder);
>         }
>     }
>     @Override
>     public void onToSolrInputDocument(Object obj, SolrInputDocument doc, 
> DocumentObjectBinder binder) {
>         if(!isTransientPresent){
>             super.onToSolrInputDocument(obj, doc, binder);
>         }
>     }
> }
> {code}
> I think this is a good solution for a win-win scenario:
> - The code is almost unchanged (only shifted, wisely changed) so there is no 
> new code to test and to support for the devs.
> - People who only need the plain @Field will be untouched and unaware of the 
> change.
> - People who need some extra functionalities (like converters) can now easily 
> extend the @Field logic, or make new annotations.
> - With some luck, a Community-Driven repository with the most useful features 
> can emerge.
> - [Bonus] The binder is now entirely documented and not so scarier like 
> before (we are almost at Halloween, but still... :) )
>  The code is production-ready, if you are interested in merging it to the 
> core, we can discuss the last touch (mainly what to leave protected and what 
> to set private).
> ---
> Update1: Added PATCH



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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

Reply via email to