interfasys / mediametadata

A cloud application which provides CRUD access to the metadata stored in images
GNU Affero General Public License v3.0
5 stars 1 forks source link

Retrieve Metadata from DB #38

Closed imjalpreet closed 8 years ago

imjalpreet commented 8 years ago

@oparoz I have written the basic code for retrieval of metadata from DB. I have checked it and it worked.

Can you give suggestions?

oparoz commented 8 years ago

I'll run this in a bit, but in the mean time, start writing the test for it.

imjalpreet commented 8 years ago

@oparoz But actually, I have already used findEntity. I can't see findOneEntity in the class mapper and I think findEntity is what you are referring to findOneEntity. As there is one more which is findEntities. Am I correct or did I miss something?

oparoz commented 8 years ago

You are absolutely correct. I was looking here too quickly: https://github.com/owncloud/core/blob/stable9.1/tests/lib/AppFramework/Db/MapperTest.php#L48

imjalpreet commented 8 years ago

@oparoz Okay, So should I proceed to tests now?

oparoz commented 8 years ago

Yes please, so that we can quickly move to the API implementation

imjalpreet commented 8 years ago

@oparoz I needed some help. Can you tell how can I mock the mapper method find or findEntity? So that it returns what I want it to return.

oparoz commented 8 years ago

Like this https://github.com/owncloud/core/blob/stable9.1/tests/lib/AppFramework/Db/MapperTest.php#L83-L91 and this https://github.com/owncloud/core/blob/stable9.1/tests/lib/AppFramework/Db/MapperTest.php#L83-L91

No?

imjalpreet commented 8 years ago

@oparoz I meant like I have to test the retrieve method in RetrieveMetadata class, so for that I need to mock findEntity so that it returns what I want, so how I needed some help in doing that as I wasn't able to do it.

oparoz commented 8 years ago

OK, see if this helps: https://github.com/nextcloud/news/blob/7a3a22bd2757e129c8964d9ddcde15ec53b736bb/tests/Unit/Db/MapperTestUtility.php#L84-L189

oparoz commented 8 years ago

That class mentioned above is the helper and then you can use it like in this test class: https://github.com/nextcloud/news/blob/7a3a22bd2757e129c8964d9ddcde15ec53b736bb/tests/Unit/Db/ItemMapperTest.php

I'm thinking this method should be interesting: https://github.com/nextcloud/news/blob/7a3a22bd2757e129c8964d9ddcde15ec53b736bb/tests/Unit/Db/ItemMapperTest.php#L106-L115

imjalpreet commented 8 years ago

Okay, I will have a look at all of them and see if it works.

imjalpreet commented 8 years ago

@oparoz I have written a basic test for the retrieve method but it isn't passing as of now. Can you check what mistake am I making? DB output is not getting mocked properly.

The error is:

 [OCP\AppFramework\Db\DoesNotExistException] Did expect one result but found none when executing: query "SELECT * FROM *PREFIX*mediametadata WHERE image_id = ?"; parameters Array
(
    [0] => 260495
)
; limit ""; offset ""  
oparoz commented 8 years ago

OK, so I've made lots of comments inline :) Once this passes, we can merge it and then work on the API

imjalpreet commented 8 years ago

@oparoz Okay, I will do all the changes. But did you have a look at the error that I mentioned above, do you have anything in mind what I have done wrong?

oparoz commented 8 years ago

If you fix all the issues I reported ,the test will pass. What you did wrong is to not use the mocked DB for your test, you used a real connection which was never populated properly.

imjalpreet commented 8 years ago

@oparoz Okay, Thanks. I will do all the changes and check.

imjalpreet commented 8 years ago

@oparoz I have made the changes and test passed. Now we can move to API.

oparoz commented 8 years ago

Could you please quickly fix the warning, otherwise we're always going to get it in future PRs.

imjalpreet commented 8 years ago

@oparoz Done.

oparoz commented 8 years ago

Code looks good, thanks!