Add support for json params object

[takanuva15]

Add support for json params object

closes #26

[takanuva15]

@stevenferrer looks like you have to approve something to be able to run the GitHub workflows

[stevenferrer]

Hi @takanuva15, thanks for submitting a PR!

From the looks if it, the params looks very similar to the facet field. I think it would be a lot cleaner if we implement it the way facets are implemented. I'm thinking we can have an interface like Paramer (just to comply with how Go names interfaces) and have a struct like SpellCheckParam implement it. Then Params method could take a list of Paramer as argument. Thoughts?

Paramer interface and SpellCheckParam.

type Paramer interface {
    BuildParam() M
}

type SpellCheckParam struct {
    q string
    // other fields...
}

func (p *SpellCheckParam) BuildParam() M {
    m := M{}
    if p.q != "" {
        m["spellcheck.q"] = f.q
    }

    // other fields...

    return m
}

func (p *SpellCheckParam) Q(q string) *SpellCheckParam {
    p.q = q
   return p
}

// other fields...

Params method.

func (q *Query) Params(params ...Paramer) *Query {
	q.params = params
	return q
}

[takanuva15]

Hi, that definitely makes sense for structured stuff like facets, but the issue with "params" is that it can be literally any query parameter you want - solr just treats it as if you were appending it directly to the url using the standard http-param format. In the example solr gives:

{
  params: {
    q: "memory",
    rows: 1
  }
}'

They were able to configure q and rows directly in params, which we would only be able to replicate in this library by either defining Paramers for these, or simply allowing params to accept a raw map that gets serialized during query-building. In this sense, params is closer to the existing Queries function on the solr.Query object. (I'm ignoring the query.Rows(...) function for the moment since the goal with params here is to enable the same flexibility that Solr allows)

For my situation, I'm dealing with a large enterprise codebase with a lot of complex queries. As one example, I need to dereference arbitrary parameters: q={!boost b=$xyzCompanyCustomDateboostFn v=$speciallyGeneratedQueryStr defType=dismax}. The equivalent JSON API request is this:

{
    "params": {
        "speciallyGeneratedQueryStr": "specialQueryTerms",
        "xyzCompanyCustomDateboostFn": "recip(ms(NOW/HOUR,myDateField),...)"
    },
    "query": "{!boost b=$xyzCompanyCustomDateboostFn v=$speciallyGeneratedQueryStr defType=dismax}",
}

I can't just inline everything due to the existing logging/debugging/maintenance infrastructure that our team has around these requests, so I need the params block as above. With the new Params function in this PR, I was easily able to configure the library to make such a request working:

queryStr := "{!boost b=$xyzCompanyCustomDateboostFn v=$speciallyGeneratedQueryStr defType=dismax}"
query := solr.NewQuery(queryStr).
	Params(solr.M{
		"speciallyGeneratedQueryStr":  "specialQueryTerms",
		"xyzCompanyCustomDateboostFn": "recip(ms(NOW/HOUR,myDateField),...)",
	})
res, err := s.solrClient.Query(context.Background(), "myColl", query)

To make it work with Paramer, I think we would have to define some sort of ArbitraryParams struct to accept a KV(key string, value any) pair to get stored and serialized during BuildParam(). And at that point, we would just be making a wrapper around solr.M{}.

(To be fair, the params object I added in the unit test is oversimplified, so I'll go back in and update it with the above use case)

[stevenferrer]

Thanks for sharing your use case! With that, I'd agree that it's simpler to keep this field a map. I'll suggest to add a separate test-case to demonstrate a realistic use of this field, thanks 👍

[takanuva15]

Hi, PR is now updated with another test case using parameter dereferencing syntax

[stevenferrer]

lgtm 👍

[takanuva15]

Thanks! It looks like there's no new tag for this on the GitHub repo yet - can we add one for importing into go.mod?

[stevenferrer]

Thanks! It looks like there's no new tag for this on the GitHub repo yet - can we add one for importing into go.mod?

If you're still planning to send PR for #27 and #29, we can create a new tag to include these three new changes.

[takanuva15]

I don't have bandwidth for #27 at this point, but I'll throw up a quick PR for #29