bitemyapp / bloodhound

Haskell Elasticsearch client and query DSL
bitemyapp.com
BSD 3-Clause "New" or "Revised" License
424 stars 118 forks source link

DocId needs url-encoding for certain operations #247

Open robinp opened 6 years ago

robinp commented 6 years ago

Creating a document with DocId containing slashes works, but fetching it with getDocument / documentExists doesn't find it. The difference is that creation uses JSON in POST, while the get/exist uses GET and url parameters.

So for the GET case, the DocId should be encoded.

I found https://github.com/bitemyapp/bloodhound/issues/38 which seemed to be similar. I tried running the tests locally, but many of them failed anyway. Is there a special requirement for setting up a test env? (I have ES 6.5 running locally).

bitemyapp commented 6 years ago

Is there a special requirement for setting up a test env? (I have ES 6.5 running locally).

Yes, running ES 5.x >:P

https://github.com/bitemyapp/bloodhound/tree/master/src/Database we haven't ported to 6.x yet. Tracking Elastic's churn is labor intensive and we don't have a means of abstracting over the commonalities and differences that we're happy with yet.

Happily take a patch that fixes this url-encoding issue.

robinp commented 6 years ago

Good to know. Thanks, will have a look. By the way, BloodHound works with 6.3, at least for the functionality I was concerned so far.

bitemyapp commented 6 years ago

By the way, BloodHound works with 6.3, at least for the functionality I was concerned so far.

I'm not surprised, but I define compatibility as encompassing the entire test suite.

Good hunting!

robinp commented 5 years ago

Thanks. What would be the way to add support for 6.x? Copy the V5 code into a V6 module, and work incrementally?

bitemyapp commented 5 years ago

@robinp that's probably the most straight-forward way to go about it. @MichaelXavier sound copacetic to you?

MichaelXavier commented 5 years ago

That'd be how I'd do it!

JoseD92 commented 5 years ago

I tried to reproduce this issue but could not find a combination that allows me to insert/index a document but not read it, both indexDocument and getDocument use the DocId on url parameters, so it is always going to find whatever we can insert with indexDocument. @robinp do can you provide some example that shows how to reproduce the error.

As a side note, I create a twitter index as the examples seen in Database.V5.Bloodhound.Client using ES version 5.6.16 and bloodhound master branch on commit fc310b23c3c1698ff7a5ca3c54236ef84846dbf3, then insert and retrieve various documents with different names, I tried let myDoc = DocId s in (runBH' $ indexDocument testIndex testMapping defaultIndexDocumentSettings exampleTweet myDoc >> getDocument testIndex testMapping myDoc) with the values for s: