Use apps versions to generate suffix when possible #7244

skjnldsv · 2017-11-22T13:00:49Z

Reduce the hash to 8 chars (readability)
Generate hash based on apps versions if possible and not nextcloud version

This will force css cache update on the user's browser side on app update. And avoid many errors like calendar or contacts had in the past.

codecov · 2017-11-22T13:27:43Z

Codecov Report

Merging #7244 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #7244      +/-   ##
============================================
- Coverage     50.97%   50.96%   -0.01%     
- Complexity    24735    24745      +10     
============================================
  Files          1586     1586              
  Lines         94257    94275      +18     
  Branches       1365     1365              
============================================
+ Hits          48049    48050       +1     
- Misses        46208    46225      +17

Impacted Files	Coverage Δ	Complexity Δ
lib/private/TemplateLayout.php	`0% <0%> (ø)`	`45 <14> (+10)`	⬆️
apps/files_trashbin/lib/Trashbin.php	`72.53% <0%> (+0.24%)`	`136% <0%> (ø)`	⬇️

MorrisJobke · 2017-11-24T13:23:01Z

What I did:

open browser
noted down the hash of a CSS that was generated from SCSS: /server/css/files_sharing/d1b66d1f58bf8c563feed8479657874b-mergedAdditionalStyles.css
changed the version of files_sharing to 1.5.1 from 1.5.0
actual: same hash
expected: different hash

@skjnldsv What did I wrong?

skjnldsv · 2017-11-27T15:24:17Z

@MorrisJobke It's the hash after the .css
You need to disable the debug mode though.

nickvergessen · 2017-11-29T13:10:36Z

lib/private/TemplateLayout.php

 		}
+		if ($app !== false && $app !== '') {
+			$v = \OC_App::getAppVersions();
+			$appName = end(explode('/', $app));


Also the explode looks like it wont work for apps/ next to server/ so where your $web is not straight forward?

nickvergessen · 2017-11-29T13:10:45Z

lib/private/TemplateLayout.php


-	protected function getVersionHashSuffix() {
-		if(\OC::$server->getConfig()->getSystemValue('debug', false)) {
+	protected function getVersionHashSuffix($app=false) {


Missing spaces around =

Fixed

skjnldsv · 2017-12-05T17:12:59Z

All clear! Now we have nice and beautiful version suffixes! :)

<link rel="stylesheet" href="/apps/unsplash/css/login.css?v=f63163cd-18">
<link rel="stylesheet" href="/css/core/2933078af7952208ea8b73dc956a6293-share.css?v=190c0a6d-18">
<link rel="stylesheet" href="/apps/files_versions/css/versions.css?v=ca9f0d77-18">
<link rel="stylesheet" href="/css/core/2933078af7952208ea8b73dc956a6293-jquery.ocdialog.css?v=190c0a6d-18">
<link rel="stylesheet" href="/css/contacts/2933078af7952208ea8b73dc956a6293-style.css?v=47354877-18">
<link rel="stylesheet" href="/apps2/contacts/css/vendor/ui-select/select.min.css?v=47354877-18">

rullzer · 2017-12-06T07:32:18Z

lib/private/TemplateLayout.php

+			}
+			return end($pathParts);
+		}
+


Don't lie in your phpdoc 😉 If you say you returning a string do so :P

MorrisJobke · 2017-12-08T15:47:08Z

lib/private/TemplateLayout.php

+	 * @param string $path
+	 * @return string
+	 */
+	static public function getAppNamefromPath($path) {


Is there a reason why this is a static method? I guess the static can be dropped here.

MorrisJobke · 2017-12-08T15:47:37Z

@skjnldsv Could you address the comments? And then this can get in.

Done

skjnldsv · 2017-12-08T16:43:29Z

@MorrisJobke @rullzer all clear :)

MorrisJobke · 2017-12-11T12:54:27Z

lib/private/TemplateLayout.php

+			}
+			return end($pathParts);
+		}
+		return false


Syntax error: Missing ; 😉 Let me push a fix for this

I was sure I pushed a fix for this! :O

MorrisJobke

Works with my syntax fix 👍

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Signed-off-by: Morris Jobke <[email protected]>

MorrisJobke · 2017-12-11T13:24:48Z

Rebased to fix the failing integration test, that was fixed on master.

MorrisJobke · 2017-12-11T13:51:17Z

Rebased to fix the failing integration test, that was fixed on master.

I was wrong - it was not yet merged to master -> CI looks good so far -> merging.

MorrisJobke · 2017-12-11T13:54:36Z

Of course I have overlooked one issue in the setup integration tests because of the other one.

{"reqId":"KzL6vs3sDWz9myzeplMg","level":3,"time":"2017-12-11T13:53:47+00:00","remoteAddr":"192.168.99.1","user":"--","app":"core","method":"GET","url":"\/server\/","message":"Exception: {\"Exception\":\"Doctrine\\\\DBAL\\\\DBALException\",\"Message\":\"Failed to connect to the database: An exception occurred while executing 'PRAGMA journal_mode = WAL':\\n\\nSQLSTATE[HY000]: General error: 10 disk I\\\/O error\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/projects\\\/server\\\/3rdparty\\\/doctrine\\\/dbal\\\/lib\\\/Doctrine\\\/DBAL\\\/Connection.php(992): OC\\\\DB\\\\Connection->connect()\\n#1 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/DB\\\/Connection.php(216): Doctrine\\\\DBAL\\\\Connection->executeUpdate('PRAGMA read_unc...', Array, Array)\\n#2 \\\/srv\\\/projects\\\/server\\\/3rdparty\\\/doctrine\\\/dbal\\\/lib\\\/Doctrine\\\/DBAL\\\/Connection.php(623): OC\\\\DB\\\\Connection->executeUpdate('PRAGMA read_unc...')\\n#3 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/DB\\\/Connection.php(151): Doctrine\\\\DBAL\\\\Connection->setTransactionIsolation(2)\\n#4 \\\/srv\\\/projects\\\/server\\\/3rdparty\\\/doctrine\\\/dbal\\\/lib\\\/Doctrine\\\/DBAL\\\/DriverManager.php(172): OC\\\\DB\\\\Connection->__construct(Array, Object(Doctrine\\\\DBAL\\\\Driver\\\\PDOSqlite\\\\Driver), Object(Doctrine\\\\DBAL\\\\Configuration), Object(Doctrine\\\\Common\\\\EventManager))\\n#5 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/DB\\\/ConnectionFactory.php(152): Doctrine\\\\DBAL\\\\DriverManager::getConnection(Array, Object(Doctrine\\\\DBAL\\\\Configuration), Object(Doctrine\\\\Common\\\\EventManager))\\n#6 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/Server.php(605): OC\\\\DB\\\\ConnectionFactory->getConnection('sqlite', Array)\\n#7 \\\/srv\\\/projects\\\/server\\\/3rdparty\\\/pimple\\\/pimple\\\/src\\\/Pimple\\\/Container.php(113): OC\\\\Server->OC\\\\{closure}(*** sensitive parameters replaced ***)\\n#8 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Utility\\\/SimpleContainer.php(116): Pimple\\\\Container->offsetGet('OCP\\\\\\\\IDBConnecti...')\\n#9 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/ServerContainer.php(132): OC\\\\AppFramework\\\\Utility\\\\SimpleContainer->query('OCP\\\\\\\\IDBConnecti...')\\n#10 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Utility\\\/SimpleContainer.php(164): OC\\\\ServerContainer->query('OCP\\\\\\\\IDBConnecti...')\\n#11 \\\/srv\\\/projects\\\/server\\\/3rdparty\\\/pimple\\\/pimple\\\/src\\\/Pimple\\\/Container.php(109): OC\\\\AppFramework\\\\Utility\\\\SimpleContainer->OC\\\\AppFramework\\\\Utility\\\\{closure}(*** sensitive parameters replaced ***)\\n#12 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Utility\\\/SimpleContainer.php(116): Pimple\\\\Container->offsetGet('DatabaseConnect...')\\n#13 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/ServerContainer.php(132): OC\\\\AppFramework\\\\Utility\\\\SimpleContainer->query('DatabaseConnect...')\\n#14 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/Server.php(1414): OC\\\\ServerContainer->query('DatabaseConnect...')\\n#15 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/Server.php(421): OC\\\\Server->getDatabaseConnection()\\n#16 \\\/srv\\\/projects\\\/server\\\/3rdparty\\\/pimple\\\/pimple\\\/src\\\/Pimple\\\/Container.php(113): OC\\\\Server->OC\\\\{closure}(*** sensitive parameters replaced ***)\\n#17 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Utility\\\/SimpleContainer.php(116): Pimple\\\\Container->offsetGet('OC\\\\\\\\AppConfig')\\n#18 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/ServerContainer.php(132): OC\\\\AppFramework\\\\Utility\\\\SimpleContainer->query('OC\\\\\\\\AppConfig')\\n#19 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Utility\\\/SimpleContainer.php(164): OC\\\\ServerContainer->query('OC\\\\\\\\AppConfig')\\n#20 \\\/srv\\\/projects\\\/server\\\/3rdparty\\\/pimple\\\/pimple\\\/src\\\/Pimple\\\/Container.php(109): OC\\\\AppFramework\\\\Utility\\\\SimpleContainer->OC\\\\AppFramework\\\\Utility\\\\{closure}(*** sensitive parameters replaced ***)\\n#21 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Utility\\\/SimpleContainer.php(116): Pimple\\\\Container->offsetGet('AppConfig')\\n#22 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/ServerContainer.php(132): OC\\\\AppFramework\\\\Utility\\\\SimpleContainer->query('AppConfig')\\n#23 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/Server.php(1336): OC\\\\ServerContainer->query('AppConfig')\\n#24 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/legacy\\\/app.php(961): OC\\\\Server->getAppConfig()\\n#25 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/TemplateLayout.php(206): OC_App::getAppVersions()\\n#26 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/TemplateLayout.php(158): OC\\\\TemplateLayout->getVersionHashSuffix()\\n#27 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/legacy\\\/template.php(207): OC\\\\TemplateLayout->__construct('error', '')\\n#28 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/Template\\\/Base.php(132): OC_Template->fetchPage()\\n#29 \\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/legacy\\\/template.php(351): OC\\\\Template\\\\Base->printPage()\\n#30 \\\/srv\\\/projects\\\/server\\\/index.php(71): OC_Template::printExceptionErrorPage(Object(Doctrine\\\\DBAL\\\\DBALException))\\n#31 {main}\",\"File\":\"\\\/srv\\\/projects\\\/server\\\/lib\\\/private\\\/DB\\\/Connection.php\",\"Line\":64}","userAgent":"Mozilla\/5.0 (Macintosh; Intel Mac OS X 10_13_1) AppleWebKit\/604.3.5 (KHTML, like Gecko) Version\/11.0.1 Safari\/604.3.5","version":""}

Let me fix this.

MorrisJobke · 2017-12-11T14:08:47Z

Let me fix this.

#7449 <- that then also should be backported together with this PR.

rullzer · 2017-12-11T20:23:51Z

@skjnldsv please prepare the backport

MorrisJobke · 2018-01-09T14:49:44Z

@skjnldsv I guess this backport didn't made it yet, right?

skjnldsv · 2018-01-09T15:09:29Z

Oups, my bad!

MorrisJobke · 2018-01-09T15:11:10Z

I would like to wait for 12.0.6 for this, is this okay?

skjnldsv · 2018-01-09T15:13:09Z

Okay for me

skjnldsv added 3. to review Waiting for reviews enhancement medium technical debt labels Nov 22, 2017

skjnldsv added this to the Nextcloud 13 milestone Nov 22, 2017

skjnldsv requested review from MorrisJobke and rullzer November 22, 2017 13:00

skjnldsv self-assigned this Nov 22, 2017

skjnldsv added the backport-request label Nov 22, 2017

skjnldsv requested a review from nickvergessen November 23, 2017 11:32

nickvergessen previously requested changes Nov 29, 2017

View reviewed changes

rullzer previously requested changes Dec 6, 2017

View reviewed changes

MorrisJobke reviewed Dec 8, 2017

View reviewed changes

skjnldsv force-pushed the css-file-suffix-with-apps-versions branch from 4acc3f0 to 5958034 Compare December 8, 2017 16:43

skjnldsv requested review from MorrisJobke, nickvergessen and rullzer December 8, 2017 16:43

MorrisJobke mentioned this pull request Dec 8, 2017

13.0.0 beta 2 #7425

Merged

28 tasks

rullzer approved these changes Dec 11, 2017

View reviewed changes

MorrisJobke requested changes Dec 11, 2017

View reviewed changes

MorrisJobke approved these changes Dec 11, 2017

View reviewed changes

MorrisJobke added the 4. to release Ready to be released and/or waiting for tests to finish label Dec 11, 2017

MorrisJobke removed the 3. to review Waiting for reviews label Dec 11, 2017

skjnldsv and others added 4 commits December 11, 2017 14:24

Use apps versions to generate suffix when possible

820e7b5

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Fixed md5 generation and added fallback for scss requests

f018bfc

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Fixed phpdoc and function type

1ac3126

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Fix syntax error

52e7d05

Signed-off-by: Morris Jobke <[email protected]>

MorrisJobke force-pushed the css-file-suffix-with-apps-versions branch from 759bff6 to 52e7d05 Compare December 11, 2017 13:24

MorrisJobke merged commit f4d7afc into master Dec 11, 2017

MorrisJobke deleted the css-file-suffix-with-apps-versions branch December 11, 2017 13:51

MorrisJobke mentioned this pull request Dec 11, 2017

Fix 500 on setup page #7449

Merged

skjnldsv mentioned this pull request Mar 1, 2018

[stable12] Use apps versions to generate suffix when possible #8611

Merged

rullzer removed the backport-request label Mar 5, 2018

rullzer mentioned this pull request Mar 8, 2018

Add Cache-control: immutable #7904

Merged

1 task

Uh oh!

Use apps versions to generate suffix when possible #7244

Use apps versions to generate suffix when possible #7244

Uh oh!

Conversation

skjnldsv commented Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MorrisJobke commented Nov 24, 2017

Uh oh!

skjnldsv commented Nov 27, 2017

Uh oh!

nickvergessen Nov 29, 2017

Choose a reason for hiding this comment

Uh oh!

nickvergessen Nov 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickvergessen Nov 29, 2017

Choose a reason for hiding this comment

Uh oh!

skjnldsv commented Dec 5, 2017

Uh oh!

rullzer Dec 6, 2017

Choose a reason for hiding this comment

Uh oh!

MorrisJobke Dec 8, 2017

Choose a reason for hiding this comment

Uh oh!

MorrisJobke commented Dec 8, 2017

Uh oh!

skjnldsv commented Dec 8, 2017

Uh oh!

MorrisJobke Dec 11, 2017

Choose a reason for hiding this comment

Uh oh!

skjnldsv Dec 11, 2017

Choose a reason for hiding this comment

Uh oh!

MorrisJobke left a comment

Choose a reason for hiding this comment

Uh oh!

MorrisJobke commented Dec 11, 2017

Uh oh!

MorrisJobke commented Dec 11, 2017

Uh oh!

MorrisJobke commented Dec 11, 2017

Uh oh!

MorrisJobke commented Dec 11, 2017

Uh oh!

rullzer commented Dec 11, 2017

Uh oh!

MorrisJobke commented Jan 9, 2018

Uh oh!

skjnldsv commented Jan 9, 2018

Uh oh!

MorrisJobke commented Jan 9, 2018

Uh oh!

skjnldsv commented Jan 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

skjnldsv commented Nov 22, 2017 •

edited

Loading

codecov bot commented Nov 22, 2017 •

edited

Loading

nickvergessen Nov 29, 2017 •

edited

Loading