kreait / firebase-bundle

A Symfony Bundle for the Firebase PHP Admin SDK
https://github.com/kreait/firebase-php
MIT License
135 stars 25 forks source link

Allow credentials to be provided as an array #20

Closed AliMsayleb closed 4 years ago

AliMsayleb commented 4 years ago

kreait_firebase:

projects:
    project_name:
        credentials:
            type: "%env(FIREBASE_CREDENTIALS_TYPE)%"
            project_id: "%env(FIREBASE_PROJECT_ID)%"
            private_key_id: "%env(FIREBASE_PRIVATE_KEY_ID)%"
            client_id: "%env(FIREBASE_CLIENT_ID)%"
            client_email: "%env(FIREBASE_CLIENT_EMAIL)%"
            private_key: "%env(string:FIREBASE_PRIVATE_KEY)%"

In the code of the Service account which is taking the credentials, there is a function to handle credentials being sent from array But in the Configuration file it's not allowing it being sent in this way The other way would be like this

        credentials: '{"type": "%env(FIREBASE_CREDENTIALS_TYPE)%","project_id": "%env(FIREBASE_PROJECT_ID)%","private_key_id": "%env(FIREBASE_PRIVATE_KEY_ID)%", "client_id": "%env(FIREBASE_CLIENT_ID)%", "client_email": "%env(FIREBASE_CLIENT_EMAIL)%","private_key": "%env(string:FIREBASE_PRIVATE_KEY)%"}'

Which is pretty much the same just a bit clearer

(Please note that this is the first time i contribute to an open source library so i really don't know about the process of what should be done)

jeromegamez commented 4 years ago

Hey @AliMsayleb, and thank you for making your first contribution to this library, which makes me glad!

Could you please add the following tests to the ProjectFactoryTest class, right after the it_can_handle_a_credentials_path() test here?

/**
 * @test
 * @group legacy
 * @expectedDeprecation The %s method is deprecated (4.33 Use the component-specific create*() methods instead.).
 */
public function it_can_handle_a_credentials_string()
{
    $this->firebaseFactory
        ->expects($this->once())
        ->method('withServiceAccount');

    $credentials = file_get_contents(__DIR__.'/../../_fixtures/valid_credentials.json');

    $this->factory->create(['credentials' => $credentials]);
}

/**
 * @test
 * @group legacy
 * @expectedDeprecation The %s method is deprecated (4.33 Use the component-specific create*() methods instead.).
 */
public function it_can_handle_a_credentials_array()
{
    $this->firebaseFactory
        ->expects($this->once())
        ->method('withServiceAccount');

    $credentials = json_decode(file_get_contents(__DIR__.'/../../_fixtures/valid_credentials.json'), true);

    $this->factory->create(['credentials' => $credentials]);
}

I already tested those tests locally, but I would like them to be attributed to you as well instead of me adding them after the merge :)

This way we can make sure that the array support you added is fully tested and won't break with future changes.

jeromegamez commented 4 years ago

Thanks! 🌺