2.2.1
#2 Add properties
mentions a "infinispan-module.propertie": we don't use such a file, the properties are meant to be defined in the main Infinispan configuration.

Proposal: merge #2 and #3 in one, call it "Set the configuration properties" (avoid the term Persistence which is misleading)
Text before the example: "Set the configuration properties for the Infinispan Query section:"

(In reply to Martin Gencur from comment #0)
> Here are initial notes, I'll be adding more as I'll through the ramaining
> part of the docs.
> 
> 
> == 1.1. Introduction
> 
> Can we add a note that throughout the text we describe Hibernate Search
> Query DSL (domain specific language) as well as Infinispan Query DSL ?
> This could help users realise that there are actually two ways to create
> queries and be prepared for it.

Added note:

In this document, the Hibernate Search Query DSL and the Infinispan Query DSL are discussed frequently. The two are separate DSLs which provide two different ways to create queries and should not be confused with each other.

> 
> == 1.4 Indexing
> 
> * The paragraph starting with "A lucene index appears at /var/....." shoudl
> be removed. The index might be stored in memory as well as on disk, and in
> the case of storing on disk, the location would be perhaps different.

Removed para.

> 
> == 1.4.2 Using the Index Manager
> 
> * This topic actually does not say anything about Index Manager. I'd suggest
> removing the whole topic as it does not help understand the process.
> Or can you describe what was the purpose of this?

This is ported from the Hibernate Search Guide, so if it is irrelevant, we will exclude it. Should now be gone.

> 
> == 1.5. Batching
> 
> * This chapter should probably be a sub-chapter of 1.4. I.e. 1.4.3 and not
> 1.5. Batching is related to indexing.

Shifted to 1.4.3 as recommended.

> 
> == 1.6 Searching
> 
> * Sentence "To execute a search, create a Lucene query (using either the
> Lucene API 
> ( Section 5.1.1, “Building a Lucene Query Using the Lucene API”) or the
> Infinispan Query 
> query DSL ( Section 5.1.3.1, “Generality” )."    ..should be replaced by:
> "To execute a search, create a Lucene query (using either the Hibernate
> Search query DSL 
> ( Section 5.1.1, “Building a Lucene Query Using Hibernate Search query DSL))
> or the 
> Infinispan Query query DSL ( Section 5.1.3.1, “Generality” ).
> 
> (I'll later suggest the respective change in chapter 5.1.1)
> The important thing is that what is created is a Lucene Query but we create
> it via Hibernate Search query DSL 
> (that's what we're showing in the docs, not via Lucene API. 

OK, the name of the target topic is automatically updated so I've changes the name of the target topic and it should now say the correct name in the link.

> 
> * Title of the code snippet should be changed from "Example 1.1. Using an
> Infinispan Query Session to Create and Execute a Search"
> to "Example 1.1. Using an Infinispan Query (Hibernate Search query DSL) to
> Create and Execute a Search"

Fixed example title.

> * The code snippet is taken from WFK docs and is not correct, it should look
> like this:
> 
> ------------------------------
> // create native Lucene query using the Hibernate query DSL
> // alternatively you can create the Lucene query using the Lucene query
> parser
> // or the Lucene programmatic API.
> import org.hibernate.search.query.dsl.QueryBuilder;
> 
> QueryBuilder qb =
> Search.getSearchManager(cache).buildQueryBuilderForClass(Book.class).get();
> 
> org.apache.lucene.search.Query query = qb
>   .keyword()
>   .onFields("title", "author")
>   .matching("Java rocks!")
>   .createQuery();
> 
> // wrap Lucene query in a org.infinispan.query.CacheQuery
> CacheQuery cacheQuery = Search.getSearchManager(cache).getQuery(query,
> Book.class);
> 
> List list = cacheQuery.list();
> --------------------------------------

Fixed code.

> == 1.7. Analyzer
> 
> * This topic is, IMO, too advanced for this moment. Actually, I think that
> later in the docs we describe Analyzers sufficiently so 
> this topic could be removed altogether. But let me get to those chapters:)

OK removing for now. We can add this back if the later Analyzer stuff is not sufficient.

> 
> == Chapter 2. Set Up and Configure Infinispan Query
> 
> * We have a separate guide for queries which contains instructions for
> admins as well as devs. We shoud, IMO place a short note in both Admin guide
> and Developer guide and
> point to the relevant sections in Query Guide.
> Specifically - Admin guide should contain a note and a link to chapter
> "Chapter 2. Set Up and Configure Infinispan Query" so that users don't get
> lost and get the whole picture.

At the moment, we have just moved all IQ stuff to this dedicated guide. Later, we can improve this content to address specific paths for the devs and admins but at the moment the admin and dev guides do not have any IQ content, so I'm not sure where we could place directs to the IQ guide in context.

> == 2.1.1. Infinispan Query Dependencies
> 
> * We should remove the specific version "4.2.0.Final-redhat-1" from
> hibernate search dependencies. They're both wrong and easy to forget about
> in next versions. We should recommend
> using the version which is available in the library distribution we ship
> with the release.

Replaced version with {VERSION}
 
> * One dependency is missing:
> 
> <dependency>
>     <groupId>org.hibernate</groupId>
>     <artifactId>hibernate-search-engine</artifactId>
>     <version>${version.hibernate.search}</version>
> </dependency>

Added as last dependency.

> 
> * The title for the first dependency should be changed to "To use the JBoss
> Data Grid Infinispan Query via Maven, the following....
> Otherwise it does not make much sense to me.

Fixed.

> 
> == 2.1.2. Infinispan Query Configuration Example
> 
> * This topic is misleading. I suspect this was taken from WFK. It does not
> say where to set the properties and they're not fully correct either. I'd
> suggest
> removing this topic and we can add it later if it turns out that some info
> is missing. We describe the configuration in detail later.

OK, removed.

(In reply to Sanne Grinovero from comment #2)
> 2.2.1
> #2 Add properties
> mentions a "infinispan-module.propertie": we don't use such a file, the
> properties are meant to be defined in the main Infinispan configuration.
 
Removed step.

> Proposal: merge #2 and #3 in one, call it "Set the configuration properties"
> (avoid the term Persistence which is misleading)
> Text before the example: "Set the configuration properties for the
> Infinispan Query section:"

Done as recommended.

Waiting for further feedback in this bug. Sanne, please set this back to ASSIGNED when you have further things to be done.

Another round:

== 2.2.1. Configure the Query Module

* This topic seems redundant to me, we don't describe where to set the properties exactly and we describe the process in more detail later. I'd remove this topic.

== 2.2.3. Configure Indexing Programmatically

* The code snippet looks good but let's change "properties.put("hibernate.search.[other options]", "[...]");" to
"properties.put("[other.options]", "[...]");    ...we want to avoid recommending "hibernate.search" prefix

== 3.1. Annotating Objects for Hibernate Search

Remove the @ProvidedId from this topic. It is no longer a requirement to annotate classes with @ProvidedId

== 4.1.3. @NumericField

* @Fieldproperties should be divided to two words: "@Field properties" 

== 4.3.2. @IndexedEmbedded

* "the index will contain the following fields: "  should not contain "id" , this is a leftover from HS docs

== 4.4.1. Static Index Time Boosting

* @Field@Field  ...should be probably only @Field

== 4.5.2. Named Analyzers

* The note about additional dependencies in lib/optional should be removed. We don't ship HS distribution
* The code snippet should be removed, it's redundant here.

== 4.5.3. Analyzer Definitions

* The secong code snippet should be Analyzer analyzer = Search.getSearchManager(cache).getSearchFactory().getAnalyzer("customanalyzer")

== 4.5.4. @AnalyzerDef for Solr

* The version of Hibernate-search-analyzers should NOT be hardcoded.
* The second code snippet contains redundant "filters = {"

(In reply to Martin Gencur from comment #6)
> Another round:
> 
> == 2.2.1. Configure the Query Module
> 
> * This topic seems redundant to me, we don't describe where to set the
> properties exactly and we describe the process in more detail later. I'd
> remove this topic.

OK, now removed.

> == 2.2.3. Configure Indexing Programmatically
> 
> * The code snippet looks good but let's change
> "properties.put("hibernate.search.[other options]", "[...]");" to
> "properties.put("[other.options]", "[...]");    ...we want to avoid
> recommending "hibernate.search" prefix

OK, changed.

http://docbuilder.usersys.redhat.com/14318/#Configure_Querying_Programmatically
 
> == 3.1. Annotating Objects for Hibernate Search
> 
> Remove the @ProvidedId from this topic. It is no longer a requirement to
> annotate classes with @ProvidedId

OK, removed.

http://docbuilder.usersys.redhat.com/14318/#Indexing_Keys_and_Values
 
> == 4.1.3. @NumericField
> 
> * @Fieldproperties should be divided to two words: "@Field properties" 

Fixed.

http://docbuilder.usersys.redhat.com/14318/#NumericField

> == 4.3.2. @IndexedEmbedded
> 
> * "the index will contain the following fields: "  should not contain "id" ,
> this is a leftover from HS docs

Fixed.

http://docbuilder.usersys.redhat.com/14318/#IndexedEmbedded

> 
> == 4.4.1. Static Index Time Boosting
> 
> * @Field@Field  ...should be probably only @Field

Fixed, no direct link sadly.

> == 4.5.2. Named Analyzers
> 
> * The note about additional dependencies in lib/optional should be removed.
> We don't ship HS distribution
> * The code snippet should be removed, it's redundant here.

Fixed.

http://docbuilder.usersys.redhat.com/14318/#Named_analyzers

> 
> == 4.5.3. Analyzer Definitions
> 
> * The secong code snippet should be Analyzer analyzer =
> Search.getSearchManager(cache).getSearchFactory().
> getAnalyzer("customanalyzer")

Fixed.

http://docbuilder.usersys.redhat.com/14318/#Analyzer_Definitions
 
> == 4.5.4. @AnalyzerDef for Solr
> 
> * The version of Hibernate-search-analyzers should NOT be hardcoded.
Not sure what this means. I've removed the hard coded dependency version if that's what you meant.

> * The second code snippet contains redundant "filters = {"
Fixed.

>> == 4.5.4. @AnalyzerDef for Solr
>> 
>> * The version of Hibernate-search-analyzers should NOT be hardcoded.
>Not sure what this means. I've removed the hard coded dependency version if >that's what you meant.

Yes, that's what I meant but I'd prefer something like this:
<version>${version.hibernate.search}</version>  rather than <version>${version}Final-redhat</version>

And the same for 2.1.1's hibernate search dependencies.

Thanks Misha

Thanks, Martin. Those two topics should have ${version.hibernate.search} now.

== 4.6.1. Built-in Bridges

* Replace Hibernate Query with Hibernate Search

== 5.1. Building Queries

* org.hibernate.Query should probably by replaced with org.infinispan.query.CacheQuery

== 5.1.2. Building a Lucene Query

* typo: "Lucence"

* the first code snippet shoudl be: Search.getSearchManager(cache).buildQueryBuilderForClass(Myth.class.class).get();

* Let's not use term "entity" because it applies only to HS. E.g. "indexed entity type" should be chagned to "indexed type",
"each entity type" -> "each type"

* Please remove the sentence starting with "Customize queries generated ....". It's not entirely true and is misleading.

== 5.1.3. Build a Query with Infinispan Query

* Example 5.1 should be renamed from "Wrapping a Lucene Query in a Hibernate Query" to
"Wrapping a Lucene Query in an Infinispan CacheQuery

* Typo :-) ... "as follows:hi" -> "as follows:"

== 5.1.3.4. Fetching Strategy

* This chapter should be removed. JDG does not support JOINs. This topic comes from HS docs and is applicable only to 
relational databases

== 5.1.3.5. Projection

* Formatting of the example 5.6 should be fixed.

* Example 5.7 should be changed to this:

SearchManager searchManager = Search.getSearchManager(cache);
CacheQuery cacheQuery = searchManager.getQuery(luceneQuery, Book.class);
query.projection( FullTextQuery.SCORE, FullTextQuery.THIS, "mainAuthor.name" );
List results = cacheQuery.list();
Object[] firstResult = (Object[]) results.get(0);
float score = firstResult[0];
Book book = firstResult[1];
String authorName = firstResult[2];

== 5.1.3.6. Customizing Object Initialization Strategies 

This chapter should be removed. It's purely for HS.   

== 5.1.3.8. Raise an Exception on Time Limit

* Example 5.9 should be changed as follows:

------------------
SearchManager searchManager = Search.getSearchManager(cache);
searchManager.setTimeoutExceptionFactory( new MyTimeoutExceptionFactory() );
CacheQuery cacheQuery = searchManager.getQuery(luceneQuery, Book.class);

//define the timeout in seconds
cacheQuery.timeout(2, TimeUnit.SECONDS)

try {
    query.list();
}
catch (MyTimeoutException e) {
    //do something, too slow
}

private static class MyTimeoutExceptionFactory implements TimeoutExceptionFactory {
   @Override
     public RuntimeException createTimeoutException(String message, Query query) {
        return new MyTimeoutException();
   }
}

public static class MyTimeoutException extends RuntimeException {
}
--------------------

* Example 5.10 should be removed then. It's related only to HS.
* The first paragraph of 5.1.3.8 should be changed to reflect the changes in this chapter.

== 5.2. Retrieving the Results

* uniqueResult(), iterate(), scroll() should be removed from the list, these ops are not available in Infinispan Query

== 5.2.1. Performance Considerations

This chapter shoudl be changed so that it does not contain information about methods that are not available in Infinsipan Query (mentioned above)

== 5.2.2. Result Size 

* Example 5.11 should look like this:

CacheQuery cacheQuery = Search.getSearchManager(cache).getQuery(luceneQuery, Book.class);
//return the number of matching books without loading a single one
assert 3245 == query.getResultSize(); 

CacheQuery cacheQueryLimited = Search.getSearchManager(cache).getQuery(luceneQuery, Book.class);
query.setMaxResult(10);
List results = query.list();
assert 10 == results.size()
//return the total number of matching books regardless of pagination
assert 3245 == query.getResultSize();

== 5.2.3. ResultTransformer 

* Remove. This is not available in Infinispan Query.

== 5.2.4. Understanding Results

* "fullTextQuery.explain(int) method" shoudl be changed to cacheQuery.explain(int) method 
* Remote sentence "Retrieve the ID of the document using projections and the FullTextQuery.DOCUMENT_ID constant. The document ID and entity ID are not the same ID and only the former is required for this approach."
  Thsi sentence is probably only applicable to HS.

* I'd suggest removing the "projection" way of getting the Explanation for now. I'm not able to provide a good example quickly and 
the existing one is not correct. It's for HS.

== 5.3.1. Defining and Implementing a Filter

Example 5.14 should be:
cacheQuery = Search.getSearchManager(cache).getQuery(query, Driver.class);
cacheQuery.enableFullTextFilter("bestDriver");
cacheQuery.enableFullTextFilter("security").setParameter( "login", "andre" );
cacheQuery.list(); //returns only best drivers where andre has credentials

== 5.3.2. The @Factory Filter

* Example 5.17 should  be:
cacheQuery = Search.getSearchManager(cache).getQuery(query, Driver.class);
cacheQuery.enableFullTextFilter("security").setParameter( "level", 5 );

* Example 5.18. - The @Key annotation is on the same line as the method, should be on a separate line.


== 5.3.5. Using Filters in a Sharded Environment

* The last code snippet should be:

-------------------
@Indexed
@FullTextFilterDef(name="customer", impl=ShardSensitiveOnlyFilter.class)
public class Customer {
   ...
}

CacheQuery cacheQuery = Search.getSearchManager(cache).getQuery(query, Customer.class);
cacheQuery.enableFulltextFilter("customer").setParameter("CustomerID", 5);
@SuppressWarnings("unchecked")
List results = query.List();
--------------------

== 6.1. The Query DSL

The note about Lucene API already exists a few paragraphs earlier. It's not needed to mention it again, IMO.

Note: Only Chapters 6 and 7 are remaining now. I'll go through your fixes tomorrow and verify them.

Providing feedback to your latest fixes. Chapters 6,7,8 still remain to be verified.

> 
> * Title of the code snippet should be changed from "Example 1.1. Using an
> Infinispan Query Session to Create and Execute a Search"
> to "Example 1.1. Using an Infinispan Query (Hibernate Search query DSL) to
> Create and Execute a Search"

Fixed example title.

Martin: There's still "Infinispan Query Session" but should be just "Infinispan Query" (remove session)

> == 3.1. Annotating Objects for Hibernate Search
> 
> Remove the @ProvidedId from this topic. It is no longer a requirement to
> annotate classes with @ProvidedId

OK, removed.

Martin: It is still there (in the list)

> == 4.6.1. Built-in Bridges
> 
> * Replace Hibernate Query with Hibernate Search

Done.

Martin: there's an Error in the place of this topic


> 
> == 5.1.2. Building a Lucene Query
> 
> * typo: "Lucence"
Fixed

> * the first code snippet shoudl be:
> Search.getSearchManager(cache).buildQueryBuilderForClass(Myth.class.class).
> get();
Fixed

Martin: "Myth.class.class" should be replaced with just "Myth.class" . Sorry, my bad. My code
already contained the issue.

> == 5.1.3.5. Projection
> 
> * Formatting of the example 5.6 should be fixed.
If you mean the first line being broken up, that is automatic, I'm afraid. It can't do one full line of that length so it automatically adds a line break. :(

> * Example 5.7 should be changed to this:
> 
> SearchManager searchManager = Search.getSearchManager(cache);
> CacheQuery cacheQuery = searchManager.getQuery(luceneQuery, Book.class);
> query.projection( FullTextQuery.SCORE, FullTextQuery.THIS, "mainAuthor.name"
> );
> List results = cacheQuery.list();
> Object[] firstResult = (Object[]) results.get(0);
> float score = firstResult[0];
> Book book = firstResult[1];
> String authorName = firstResult[2];

Done

Martin: Now there're two examples at one place (Example 5.6. Using Projection to Retrieve Metadata), the first one should be removed.

> == 5.1.3.8. Raise an Exception on Time Limit

Martin. The example looks good. However, the first sentence of this chapter (now 5.1.3.6) should be changed to:
If a query uses more than the defined amount of time, a custom exception might be defined to be thrown.
To define the limit when using the CacheQuery API, use the following approach:

> * Example 5.18. - The @Key annotation is on the same line as the method,
> should be on a separate line.
Fixed.

Martin: This does not look like it was fixed. Can you re-check?

Notes for section 6:

== 2.1.1. Infinispan Query Dependencies

The list of dependencies is not complete. We list here just a few most important dependencies. However, we should provide a
full list of deps. One of the options is to point users to runtime-classpath.xml file in the library distribution. 
In that file there are lists of dependencies for several features, including "infinispan-query" which is the correct one for this chapter.

== 6.1.1. Enabling DSL-based Queries

* Libraries infinispan-query-dsl.jar and infinispan-query.jar are not enough, there are many more libraries required. We coudl 
again recommend that users included all libraries required for Hibernate Search based queries (see 2.1.1) plus infinispan-query-dsl.jar

* The infinispan-query maven dependency can be removed from the chapter. As I said, the infinispan-query-dsl dependency is needed so 
on top of infinispan-query dependencies, so this particular XML snippet could be changed to:
<dependency>
    <groupId>org.infinispan</groupId>
    <artifactId>infinispan-query-dsl</artifactId>
    <version>${infinispan.version}</version>
</dependency>

== 6.2. Protobuf Marshaller

6.2.1. Storing Protobuf Encoded Entities

* We should provide more information about the Protobuf file format, probably by pointing to https://developers.google.com/protocol-buffers/docs/overview, similar
to Infinispan docs. If we don't want to point to this link, we'll need to add more information about the format.

== 6.2. Protobuf Marshaller

* The following chapters should be moved under "6.3. Performing Remote Queries via the Java Hot Rod Client " because they're all related to HotRod client and remote queries:
6.2. Protobuf Marshaller
6.2.1. Storing Protobuf Encoded Entities
6.2.2. About Protobuf Messages
6.2.3. Using Protobuf with Hot Rod
6.2.4. Registering Per Entity Marshallers
6.2.5. Indexing Protobuf Encoded Entities

== 6.3. Performing Remote Queries via the Java Hot Rod Client - Procedure 6.1. Enabling Remote Querying via Hot Rod

* It's not required to add any libraries to the server, they're already there.
* The client needs more dependencies than just the DSL jar. I'd again recommend the classpath specified in runtime-classpath.txt in the library distribution
under "infinispan-client-hotrod" , this set of libraries includes protostream, protobuf, DSL jar etc.

Feedback for Chapter 7. Monitoring 

* The first sentence should be probably as follows:

Infinispan Query provides access to statistics and operations related to indexing. 
The statistics provide information about classes being indexed and entities stored in the index. 
Lucene query and object loading times can also be determined by specifying the generate_statistics property in the configuration.

== 7.1.2. About JMX

* I'm not sure what this topic should look like. What I know is that IndexControl MBean is not available as well as IndexProgressMonitor.
I'm aware of these MBeans only: StatisticsInfo, MassIndexer

* sections 7.3. IndexControlMBean and 7.4. IndexingProgressMonitorMBean should be removed then. 

Note: Chapter 7 should be completely reworked as, IMO, it does not provide any useful information. Please ask SMEs to provide more info on this chapter (anistor, sanne)

To be precise about the dependencies in sections 2.1.1 and 6.1.1, section 2.1.1 is fine for Maven users, but for non-maven users we could recommend the runtime-classpath.xml where the list of required dependencies sits.
For section 6.1.1 we should change the maven dependency to infinispan-query-dsl and advise to use also all dependencies from section 2.1.1. For non-maven users though, we should again recommend where to get the list of dependencies - in runtime-classpath.xml

Thanks

I realised that in chapter 2.1.1, we can only mention infinispan-query Maven dependency. All the other dependencies (hibernate search) are transitive dependencies of infinispan-query so will be downloaded automatically.

Leaving out Ch 7 feedback as it is tracked in another bug.

(In reply to Martin Gencur from comment #17)
> To be precise about the dependencies in sections 2.1.1 and 6.1.1, section
> 2.1.1 is fine for Maven users, but for non-maven users we could recommend
> the runtime-classpath.xml where the list of required dependencies sits.


> For section 6.1.1 we should change the maven dependency to
> infinispan-query-dsl and advise to use also all dependencies from section
> 2.1.1. For non-maven users though, we should again recommend where to get
> the list of dependencies - in runtime-classpath.xml

I think these two should be OK already based on existing changes. Please correct me if that is not the case.

My mistake with the XML vs. TXT. It's really a txt file. Thanks:)

Two instances of runtime-classpath found, one with jar and one with xml. Both are changes to .txt now.

Checked with Martin and have his approval to set to VERIFIED!

The fix for this bug is now generally released and available here:

https://access.redhat.com/site/documentation/en-US/Red_Hat_JBoss_Data_Grid/6.2/index.html