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

added code for getting current oCis version #229

Merged
merged 6 commits into from
Jul 5, 2024
Merged

Conversation

S-Panta
Copy link
Contributor

@S-Panta S-Panta commented Jul 4, 2024

New API endpoint could be added in a different version of ocis. oCIS will throw 404 status code , that leads to confusion, whether api request has problem or api endpoint has not implemented.
This PR adds capability api endpoint to get current version of oCIS so that it would be easy to dissect the methods to use in different version of oCIS

@S-Panta S-Panta force-pushed the get-current-ocis-version branch 2 times, most recently from 2e0ec48 to 7f123a5 Compare July 4, 2024 06:37
@S-Panta S-Panta marked this pull request as ready for review July 4, 2024 06:39
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have an integration test for this?
Then we will know that it works, at least for the happy path.

src/Ocis.php Show resolved Hide resolved
src/Ocis.php Outdated Show resolved Hide resolved
src/Ocis.php Outdated Show resolved Hide resolved
@individual-it
Copy link
Member

Why does the function need to be public?
What about making it private and only using it internally when an OCIS object is initiated and setting a version in a private variable. Then when we contact a endpoint that we know is not available in that version, we can throw an exception

@phil-davis
Copy link
Contributor

Why does the function need to be public?

IMO it does not need to be public, consumers of the SDK do not need to be able to get the ocis version, anything related to differences of the ocis version should be handled internally.

The function should be used to fetch and remember the ocis version.

@S-Panta you could also add code in this PR to do "throw an exception about API endpoint not implemented on ocis version less than 6.0.0 " for the existing code that was first introduced in PR #225 - that way the new getOcisVersion will be used.

src/Ocis.php Outdated Show resolved Hide resolved
src/Ocis.php Outdated Show resolved Hide resolved
src/Ocis.php Show resolved Hide resolved
tests/integration/Owncloud/OcisPhpSdk/DriveTest.php Outdated Show resolved Hide resolved
src/Ocis.php Outdated Show resolved Hide resolved
src/Ocis.php Outdated Show resolved Hide resolved
@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/ocis-php-sdk/1578/5/3
code-style problems. Please check code-style locally before pushing to GitHub.

src/Drive.php Outdated Show resolved Hide resolved
src/Ocis.php Outdated Show resolved Hide resolved
src/Ocis.php Outdated Show resolved Hide resolved
tests/integration/Owncloud/OcisPhpSdk/DriveTest.php Outdated Show resolved Hide resolved
@amrita-shrestha
Copy link
Contributor

amrita-shrestha commented Jul 5, 2024

maybe this wil pass unit tests

private function getGuzzleMock(?string $responseContent = null): MockObject
    {
        if ($responseContent === null) {
            $responseContent = '
                {
                    "data": {
                        "capabilities": {
                            "core": {
                                "status": {
                                    "productversion": "6.0.0+aa6041abb6"
                               }
                            }
                        }
                    }
                }';
        }

        $streamMock = $this->createMock(StreamInterface::class);
        $streamMock->method('getContents')->willReturn($responseContent);
        $responseMock = $this->createMock(ResponseInterface::class);
        $responseMock->method('getBody')->willReturn($streamMock);
        $guzzleMock = $this->createMock(Client::class);
        $guzzleMock->method('get')
            ->willReturn($responseMock);
        return $guzzleMock;
    }

    public function testSetAccessTokenPropagatesToDrives(): void
    {
        $driveMock = [];
        $driveMock[] = $this->createMock(Drive::class);
        $driveMock[] = $this->createMock(Drive::class);
        $driveCollectionMock = $this->createMock(CollectionOfDrives::class);
        $driveCollectionMock->method('getValue')
            ->willReturn($driveMock);
        $drivesGetDrivesApi = $this->createMock(DrivesGetDrivesApi::class);
        assert($drivesGetDrivesApi instanceof DrivesGetDrivesApi);
        $drivesGetDrivesApi->method('listAllDrives')
            ->willReturn($driveCollectionMock);
        $ocis = new Ocis(
            'https://localhost:9200',
            'tokenWhenCreated',
            [ 'guzzle' => $this->getGuzzleMock() ,
                'drivesGetDrivesApi' => $drivesGetDrivesApi]
        );
        $drives = $ocis->getAllDrives();
        foreach ($drives as $drive) {
            $this->assertSame('tokenWhenCreated', $drive->getAccessToken());
        }
        $ocis->setAccessToken('changedToken');
        foreach ($drives as $drive) {
            $this->assertSame('changedToken', $drive->getAccessToken());
        }
    }

Copy link

sonarcloud bot commented Jul 5, 2024

@S-Panta S-Panta merged commit 01e96aa into main Jul 5, 2024
2 checks passed
@S-Panta S-Panta deleted the get-current-ocis-version branch July 5, 2024 10:46
@phil-davis
Copy link
Contributor

Good - we can reuse a lot of this code when implementing the other root endpoints, where we need to know the ocis version etc.

ownclouders pushed a commit that referenced this pull request Jul 5, 2024
* added code for getting current oCis version

* added code to handle exception if no endpoint found

* addressing reviews

* updated code and drone env for master and stable

* adding guzzle mock for getting ocis version

* adding test for stable ocis for sharing drive via root
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.

4 participants