Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for private search [OSF-6898] #6184

Closed
wants to merge 65 commits into from
Closed

Add tests for private search [OSF-6898] #6184

wants to merge 65 commits into from

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Aug 22, 2016

#6197

Purpose

This adds a suite of failing tests for the behavior we'd like to see from search over private data.

Changes

tbd

Side effects

tbd

Ticket

https://openscience.atlassian.net/browse/OSF-6898

Todo

@chadwhitacre chadwhitacre changed the title Start a test file for private elastic search [WIP] Add tests for private search Aug 22, 2016
@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Aug 22, 2016

Figured I'd try running the existing test_elastic.py tests. They're failing for me, which seems like it could come back to haunt me later.

osf.io| $ py.test tests/test_elastic.py
...
==== 21 failed, 31 passed, 3 skipped in 58.81 seconds ====

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Aug 22, 2016

I've installed elasticsearch-head (h/t) to help debug issues trying to write an initial test for search.

@chadwhitacre
Copy link
Contributor Author

With 7ab64b2 I am seeing a "Foo Bar" project node in Mongo, but I'm not seeing a corresponding index entry in Elastic. Hmm ...

@chadwhitacre
Copy link
Contributor Author

Alright, so how does indexing work?

@chadwhitacre
Copy link
Contributor Author

I do see one entry in the index during the test, so indexing is possible somehow.

screen shot 2016-08-22 at 11 34 33 am

@chadwhitacre
Copy link
Contributor Author

Grepping suggests that the UserFactory is a class the instantiation of which is somehow responsible for the index entry that is showing up. However, an __init__ for this class is never called! Hmm ...

diff --git a/tests/factories.py b/tests/factories.py
index 303b740..9d2d492 100644
--- a/tests/factories.py
+++ b/tests/factories.py
@@ -100,6 +100,11 @@ class ModularOdmFactory(base.Factory):


 class UserFactory(ModularOdmFactory):
+
+    def __init__(self, *a, **kw):
+        import pdb; pdb.set_trace()
+        super(UserFactory, self).__init__(*a, **kw)
+
     class Meta:
         model = User
         abstract = False

@chadwhitacre
Copy link
Contributor Author

Serendipitously, grepping also leads me to TestSearchViews! Let's see, here ...

@chadwhitacre
Copy link
Contributor Author

Alright, so it seems that somehow factoryboy does not need to actually instantiate the AuthUserFactory in order to use it as a SubFactory.

@chadwhitacre
Copy link
Contributor Author

Maybe there are no indices for projects?

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Aug 22, 2016

Looks like TestSearchViews mostly tests user search, barely touching project search. It doesn't exercise the /api/v1/search/ view at all.

@chadwhitacre
Copy link
Contributor Author

Search definitely includes projects:

screen shot 2016-08-22 at 2 07 48 pm

@chadwhitacre
Copy link
Contributor Author

The project created in the test in 7ab64b2 is available locally by hitting its URL directly, but it's not showing up in search.

screen shot 2016-08-22 at 2 12 30 pm
screen shot 2016-08-22 at 2 12 55 pm

@chadwhitacre
Copy link
Contributor Author

I wasn't running celery or the apiserver! I'm running inv req -bd to catch up and then I'll try again with those components in place. 😁

@chadwhitacre
Copy link
Contributor Author

I'm seeing a search result through the web. I'm narrowing down the difference between there and the test case.

@chadwhitacre
Copy link
Contributor Author

They're failing for me, which seems like it could come back to haunt me later.

Shaw 'nuff. I figured I'd use test_make_public as a starting point, but that test fails locally for me. Hmm ...

@chadwhitacre
Copy link
Contributor Author

Similar behavior: I'm seeing both user and project (node) in Mongo, but only the user in Elastic.

@chadwhitacre
Copy link
Contributor Author

In my website_v1 index I do see a project as well as a user.

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Aug 23, 2016

I find an update_search. Is that called TTW but not during test? Do I have a celery misconfiguration?

@chadwhitacre
Copy link
Contributor Author

I do end up inside update_search during the test. What's going wrong?

@chadwhitacre
Copy link
Contributor Author

Okay! Something wrong with Celery. If I run with USE_CELERY=False then test_elastic.py passes!

@chadwhitacre
Copy link
Contributor Author

Onward!

@chadwhitacre
Copy link
Contributor Author

Kicked out a new PR (#6197) for some refactoring to make room for the new tests.

@chadwhitacre
Copy link
Contributor Author

Second PR kicked out for refactoring: #6199.

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Aug 23, 2016

Rebased onto #6197.

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Aug 23, 2016

Okay! Wrapping up for the day. With the ground cleared, the next thing I want to wrap my head around is what is available to be searched (projects and users ... anything else?), and what the privacy options for each object are. I want to compile a systematic view of all of these options so that we can test them all. I expect to be back on Friday (three days hence). À bientôt!

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Aug 26, 2016

OSF-6898 says that the endpoint in view here is /api/v1/search/, which shares the search_search view (P.S. what's with the Little Caesar's variables?) with /api/v1/search/<type>/—though I find separate views when type is node or projects. What are the types? Are the separate node and projects views also in view here?

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Aug 26, 2016

There's also /api/v1/user/search/ (route, view), as well as /api/v1/share/* (routes, views) ... aaaaaand a share atom search view not under /api/v1/. OSF-6898 only explicitly mentions "the existing search endpoint /api/v1/search/," but the point being made, that existing endpoints will start returning private data (rather than adding new endpoints), could equally apply to all of the search endpoints.

In terms of scoping, it's easy enough to draw a line at SHARE search (does SHARE even have a concept of private data?). But a narrow interpretation of "Assume the existing search endpoint /api/v1/search/" is difficult to maintain: Does this endpoint include the <type>/ endpoint with which it shares the same route and view? It seems natural to answer, "Yes." But then what about the {node,projects}/ endpoints that are conceptually parallel to <type>/, though they have different routes and views?

@chadwhitacre
Copy link
Contributor Author

<type> in the URL becomes doc_type in the calls to website.search.search.search (🍕🍕🍕 ) and thence to Elastic.

@chadwhitacre
Copy link
Contributor Author

Rebased, was 502779f.

@brianjgeiger brianjgeiger changed the title Add tests for private search Add tests for private search [OSF-6898] Dec 14, 2016
@chadwhitacre
Copy link
Contributor Author

For the record, my contract with COS is up and we didn't renew for 2017, so this is ready for someone else to pick up! :-)

@sloria
Copy link
Contributor

sloria commented Aug 3, 2017

Closing for now, as this has become stale. Will definitely refer back to this when we pick this feature back up. Thanks for your hard work, @whit537 !

@sloria sloria closed this Aug 3, 2017
@chadwhitacre chadwhitacre deleted the tests-for-private-elastic branch August 4, 2017 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants