Skip to content

Bootstrap: add initial state registration #21821

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.

Sign up for GitHub

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

Jump to bottom
Merged
rullzer merged 2 commits into from
Oct 4, 2020
Merged

Bootstrap: add initial state registration #21821

rullzer merged 2 commits into from
Oct 4, 2020

Conversation

@rullzer
Copy link
Member

@rullzer rullzer commented Jul 13, 2020

Covers basically

  • Allow registering 'global' initial states in the registration step
  • Deprecate the old way
  • Move accessibility over as a PoC

I'm not 100% happy with the location of the base class. But I could not come up with a better name just now.

@rullzer rullzer added enhancement 3. to review Waiting for reviews labels Jul 13, 2020
@rullzer rullzer added this to the Nextcloud 20 milestone Jul 13, 2020
MorrisJobke
MorrisJobke approved these changes Jul 13, 2020
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Code looks good 👍

ChristophWurst
ChristophWurst requested changes Jul 14, 2020
View reviewed changes
@rullzer rullzer force-pushed the enh/bootstrap/initial_state branch from 4ea97c1 to 6d79509 Compare July 14, 2020 07:09
@rullzer rullzer requested a review from ChristophWurst July 14, 2020 08:08
ChristophWurst
ChristophWurst approved these changes Jul 14, 2020
View reviewed changes
lib/private/AppFramework/Bootstrap/Coordinator.php Outdated
$context->delegateContainerRegistrations($apps);
$context->delegateMiddlewareRegistrations($apps);
$context->delegateSearchProviderRegistration($apps, $this->searchComposer);
$context->delegateInitialStateProviderRegistration($this->initialStateService);
Copy link
Member

@ChristophWurst ChristophWurst Jul 14, 2020

Choose a reason for hiding this comment

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

#21828 might help make this even lazier

Copy link
Member

@MorrisJobke MorrisJobke Jul 14, 2020

Choose a reason for hiding this comment

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

#21828 might help make this even lazier

It's merged - ready to rebase this here and incorporate the changes :)

@ChristophWurst ChristophWurst added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 15, 2020
MorrisJobke
MorrisJobke reviewed Jul 23, 2020
View reviewed changes
lib/public/IInitialStateService.php Outdated
* @param string $key
* @param Closure $closure returns a primitive or an object that implements JsonSerializable
*
* @deprecated 20.0.0 Use OCP\AppFramework\Services\InitialStateProvider
Copy link
Member

@MorrisJobke MorrisJobke Jul 23, 2020

Choose a reason for hiding this comment

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

Do we want to deprecate the whole interface and only use InitialStateProvider?

Copy link
Member

@juliusknorr juliusknorr Jul 24, 2020

Choose a reason for hiding this comment

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

Providing state inline is still useful when it is not about global state but for example providing data in a controller method, so I'd prefer to keep the simple lazy approach.

Copy link
Member Author

@rullzer rullzer Jul 24, 2020

Choose a reason for hiding this comment

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

But for that we'd have OCP\AppFramework\Services\InitialStateProvider right?
Or do you have a different use case?

Copy link
Member

@juliusknorr juliusknorr Jul 24, 2020

Choose a reason for hiding this comment

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

Well if i need to provide some state in just one controller method (e.g. for my apps page route, i don't see why I should register a provider globally in the app bootstrap. Instead just registering it with provideLazyInitialState in the controller itself seems far more organized imo. Or am I missing something here?

Copy link
Member Author

@rullzer rullzer Jul 24, 2020

Choose a reason for hiding this comment

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

Ah sorry wrong interface OCP\AppFramework\Services\IInitialState
You can have that injected. AppID already set etc ;)

Copy link
Member

@juliusknorr juliusknorr Jul 24, 2020

Choose a reason for hiding this comment

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

Ah never mind then. 👍

Copy link
Member

@MorrisJobke MorrisJobke Jul 24, 2020

Choose a reason for hiding this comment

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

Okay, that means then that we can deprecate the whole interface?

@MorrisJobke MorrisJobke force-pushed the enh/bootstrap/initial_state branch from 6d79509 to 65b1c4d Compare August 11, 2020 09:20
@MorrisJobke
Copy link
Member

MorrisJobke commented Aug 11, 2020

Rebased to resolve conflicts.

@MorrisJobke MorrisJobke mentioned this pull request Aug 11, 2020
Merged
57 tasks
@rullzer rullzer modified the milestones: Nextcloud 20, Nextcloud 21 Aug 19, 2020
@rullzer rullzer force-pushed the enh/bootstrap/initial_state branch from 65b1c4d to 6f23181 Compare October 3, 2020 13:49
@rullzer rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 3, 2020
@rullzer
Copy link
Member Author

rullzer commented Oct 3, 2020

Found some time to tackle this again... lets see

@faily-bot
Copy link

faily-bot bot commented Oct 3, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 33613: failure

checkers

Show full log 
The autoloaders are not up to date
Please run: bash build/autoloaderchecker.sh
And commit the result

nodb

Show full log 
There were 9 errors:

1) Test\InitialStateServiceTest::testStaticData with data set #0 ('string')
ArgumentCountError: Too few arguments to function OC\InitialStateService::__construct(), 1 passed in /drone/src/tests/lib/InitialStateServiceTest.php on line 43 and exactly 3 expected

/drone/src/lib/private/InitialStateService.php:55
/drone/src/tests/lib/InitialStateServiceTest.php:43

2) Test\InitialStateServiceTest::testStaticData with data set #1 (23)
ArgumentCountError: Too few arguments to function OC\InitialStateService::__construct(), 1 passed in /drone/src/tests/lib/InitialStateServiceTest.php on line 43 and exactly 3 expected

/drone/src/lib/private/InitialStateService.php:55
/drone/src/tests/lib/InitialStateServiceTest.php:43

3) Test\InitialStateServiceTest::testStaticData with data set #2 (2.3)
ArgumentCountError: Too few arguments to function OC\InitialStateService::__construct(), 1 passed in /drone/src/tests/lib/InitialStateServiceTest.php on line 43 and exactly 3 expected

/drone/src/lib/private/InitialStateService.php:55
/drone/src/tests/lib/InitialStateServiceTest.php:43

4) Test\InitialStateServiceTest::testStaticData with data set #3 (class@anonymous�/drone/src/tests/lib/InitialStateServiceTest.php0x7f36d11cc5b0 Object ())
ArgumentCountError: Too few arguments to function OC\InitialStateService::__construct(), 1 passed in /drone/src/tests/lib/InitialStateServiceTest.php on line 43 and exactly 3 expected

/drone/src/lib/private/InitialStateService.php:55
/drone/src/tests/lib/InitialStateServiceTest.php:43

5) Test\InitialStateServiceTest::testStaticButInvalidData
ArgumentCountError: Too few arguments to function OC\InitialStateService::__construct(), 1 passed in /drone/src/tests/lib/InitialStateServiceTest.php on line 43 and exactly 3 expected

/drone/src/lib/private/InitialStateService.php:55
/drone/src/tests/lib/InitialStateServiceTest.php:43

6) Test\InitialStateServiceTest::testLazyData with data set #0 ('string')
ArgumentCountError: Too few arguments to function OC\InitialStateService::__construct(), 1 passed in /drone/src/tests/lib/InitialStateServiceTest.php on line 43 and exactly 3 expected

/drone/src/lib/private/InitialStateService.php:55
/drone/src/tests/lib/InitialStateServiceTest.php:43

7) Test\InitialStateServiceTest::testLazyData with data set #1 (23)
ArgumentCountError: Too few arguments to function OC\InitialStateService::__construct(), 1 passed in /drone/src/tests/lib/InitialStateServiceTest.php on line 43 and exactly 3 expected

/drone/src/lib/private/InitialStateService.php:55
/drone/src/tests/lib/InitialStateServiceTest.php:43

8) Test\InitialStateServiceTest::testLazyData with data set #2 (2.3)
ArgumentCountError: Too few arguments to function OC\InitialStateService::__construct(), 1 passed in /drone/src/tests/lib/InitialStateServiceTest.php on line 43 and exactly 3 expected

/drone/src/lib/private/InitialStateService.php:55
/drone/src/tests/lib/InitialStateServiceTest.php:43

9) Test\InitialStateServiceTest::testLazyData with data set #3 (class@anonymous�/drone/src/tests/lib/InitialStateServiceTest.php0x7f36d11cc5b0 Object ())
ArgumentCountError: Too few arguments to function OC\InitialStateService::__construct(), 1 passed in /drone/src/tests/lib/InitialStateServiceTest.php on line 43 and exactly 3 expected

/drone/src/lib/private/InitialStateService.php:55
/drone/src/tests/lib/InitialStateServiceTest.php:43

--

There were 26 warnings:

1) Test\AppFramework\Controller\AuthPublicShareControllerTest::testAuthenticateAuthenticated
Trying to configure method "isAuthenticated" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) OCA\DAV\Tests\unit\CalDAV\CalendarTest::testConfidentialClassification with data set #0 (3, false)
No method rule is set

3) OCA\DAV\Tests\Command\MoveCalendarTest::testMove
Using assertContains() with string haystacks is deprecated and will not be supported in PHPUnit 9. Refactor your test to use assertStringContainsString() or assertStringContainsStringIgnoringCase() instead.

4) OCA\DAV\Tests\unit\BackgroundJob\UpdateCalendarResourcesRoomsBackgroundJobTest::testRun
Passing an array of interface names to createMock() for creating a test double that implements multiple interfaces is deprecated and will no longer be supported in PHPUnit 9.
Passing an array of interface names to getMockBuilder() for creating a test double that implements multiple interfaces is deprecated and will no longer be supported in PHPUnit 9.

5) ExpirationTest::testParseRetentionObligation with data set #0 ('disabled', null, null, null)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

6) ExpirationTest::testParseRetentionObligation with data set #1 ('auto', 30, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

7) ExpirationTest::testParseRetentionObligation with data set #2 ('auto,auto', 30, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

8) ExpirationTest::testParseRetentionObligation with data set #3 ('auto, auto', 30, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

9) ExpirationTest::testParseRetentionObligation with data set #4 ('auto, 3', -1, 3, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

10) ExpirationTest::testParseRetentionObligation with data set #5 ('5, auto', 5, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

11) ExpirationTest::testParseRetentionObligation with data set #6 ('3, 5', 3, 5, false)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

12) ExpirationTest::testParseRetentionObligation with data set #7 ('10, 3', 10, 10, false)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

13) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #0 ('disabled', null, null, null)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

14) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #1 ('auto', -1, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

15) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #2 ('auto,auto', -1, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

16) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #3 ('auto, auto', -1, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

17) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #4 ('auto, 3', -1, 3, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

18) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #5 ('5, auto', 5, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

19) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #6 ('3, 5', 3, 5, false)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

20) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #7 ('10, 3', 10, 10, false)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

21) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #8 ('g,a,r,b,a,g,e', -1, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

22) OCA\Files_Versions\Tests\ExpirationTest::testParseRetentionObligation with data set #9 ('-3,8', -1, -1, true)
assertAttributeEquals() is deprecated and will be removed in PHPUnit 9.
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

23) OCA\Settings\Tests\Mailer\NewUserMailHelperTest::testGenerateTemplateWithPasswordResetToken
Method getTime may not return value of type string, its return declaration is ": int"

24) OCA\TwoFactorBackupCodes\Tests\Unit\Listener\ActivityPublisherTest::testHandleCodesGeneratedEvent
Method publish may not return value of type Mock_IEvent_df2592a7, its return declaration is ": void"

25) OCA\UpdateNotification\Tests\Notification\BackgroundJobTest::testCreateNotifications with data set #1 ('app2', '1.0.1', '1.0.0', '1.0.0', true, array('user1'), array(array('user1')))
Method notify may not return value of type Mock_INotification_79a72336, its return declaration is ": void"

26) OCA\UpdateNotification\Tests\Notification\BackgroundJobTest::testCreateNotifications with data set #2 ('app3', '1.0.1', false, false, true, array('user2', 'user3'), array(array('user2'), array('user3')))
Method notify may not return value of type Mock_INotification_79a72336, its return declaration is ": void"

mariadb10.4-php7.4

acceptance-login

  • tests/acceptance/features/login.feature:38
Show full log 
  Scenario: log in with invalid user once fixed by admin              # /drone/src/tests/acceptance/features/login.feature:38
    Given I act as John                                               # ActorContext::iActAs()
    And I can not log in with user unknownUser and password 123456acb # LoginPageContext::iCanNotLogInWithUserAndPassword()
    When I act as Jane                                                # ActorContext::iActAs()
    And I am logged in as the admin                                   # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                      # SettingsMenuContext::iOpenTheUserSettings()
    And I click the New user button                                   # UsersSettingsContext::iClickTheNewUserButton()
    And I see that the new user form is shown                         # UsersSettingsContext::iSeeThatTheNewUserFormIsShown()
    And I create user unknownUser with password 123456acb             # UsersSettingsContext::iCreateUserWithPassword()
    And I see that the list of users contains the user unknownUser    # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
    And I act as John                                                 # ActorContext::iActAs()
    And I log in with user unknownUser and password 123456acb         # LoginPageContext::iLogInWithUserAndPassword()
    Then I see that the current page is the Files app                 # FilesAppContext::iSeeThatTheCurrentPageIsTheFilesApp()
      Failed asserting that 'http://acceptance-login/index.php/login?user=unknownUser' starts with "http://acceptance-login/index.php/apps/files/".

rullzer added 2 commits October 4, 2020 09:35
@rullzer
Move initial state provider to boostrap
f865a3a 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Move accessibility over to the new way
b5fd75f 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer force-pushed the enh/bootstrap/initial_state branch from 28dc290 to b5fd75f Compare October 4, 2020 07:35
@rullzer rullzer merged commit b976cd5 into master Oct 4, 2020
@rullzer rullzer deleted the enh/bootstrap/initial_state branch October 4, 2020 09:09
@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@kesselb kesselb Awaiting requested review from kesselb

Assignees

No one assigned

Labels

3. to review Waiting for reviews enhancement pending documentation This pull request needs an associated documentation update

Projects

None yet

Milestone

Nextcloud 21

Development

Successfully merging this pull request may close these issues.

5 participants

@rullzer @MorrisJobke @ChristophWurst @juliusknorr