Commit Graph

15 Commits

Author SHA1 Message Date
Hannah Wolfe
e060a4f811 🎨 🐛 Improve theme lib, middleware & error handling (#8145)
no issue

🎨 simplify loader - use loadOneTheme for init
- use loadOneTheme for init
- move updateThemeList to the one place that it is used
- this just reduces the surface area of the loader

🎨 Move init up to index temporarily
- need to figure out what stuff goes in here as well as loading themes
- will move it again later once I've got it figured out

🎨 Reorder & cleanup theme middleware
- move the order in blog/app.js so that theme middleware isn't called for shared assets
- add comments & cleanup in the middleware itself, for clarity

🎨 Simplify the logic in themes middleware
- Separate out config dependent on settings changing and config dependent on request
- Move blogApp.set('views') - no reason why this isn't in the theme activation method as
  it's actually simpler if it is there, we already know the active theme exists & can remove the if-guard

🎨 Improve error handling for missing theme
- ensure we display a warning
- don't have complex logic for handling errors
- move loading of an empty hbs object into the error-handler as this will support more cases

🐛 Fix assetHash clearing bug on theme switch
- asset hash wasn't correctly being set on theme switch

🎨 Remove themes.read & test loader instead
- Previously, we've simplified loader & improved error handling
- We are now able to completely remove theme.read as it's nothing more than a wrapper for package.read
- This also means we can change our tests from testing the theme reader to loader
2017-03-13 17:30:35 +01:00
Hannah Wolfe
e3c82c1643 🎨 Use settingsCache in theme handler (#8091)
no issue

- we already have the settingsCache here, makes no sense to call the API
2017-03-02 23:05:35 +01:00
Hannah Wolfe
f8b498d6e7 🔥 No more availableThemes (#8085)
no issue

🎨 Switch themes API to use config.availableThemes
- this gets rid of the only places where settings.availableThemes are used

🔥 Get rid of settings.availableThemes
- this is no longer used anywhere
- also get rid of every related call to updateSettingsCache

🔥 Replace config.availableThemes with theme cache
- Creates a tailor-made in-memory cache for themes inside the theme module
- Add methods for getting & setting items on the cache
- Move all references to config.availableThemes to use the new cache
- This can be abstracted later to support other kinds of caches?

🎨 Start improving theme lib's API
Still TODO: simplifying/clarifying:
- what is the structure of the internal list
- what is the difference between a package list, and a theme list?
- what is the difference between reading a theme and loading it?
- how do we update the theme list (add/remove)
- how do we refresh the theme list? (hot reload?!)
- how do we get from an internal list, to one that is sent as part of the API?
- how are we going to handle theme storage: read/write, such that the path is configurable

🎨 Use themeList consistently
🎨 Update list after storage
2017-03-02 17:53:48 +01:00
Katharina Irrgang
a68592a6b9 🔥 remove forceAdminSSL and urlSSL, add admin url (#7937)
* 🔥  kill apiUrl helper, use urlFor helper instead

More consistency of creating urls.
Creates an easier ability to add config changes.

Attention: urlFor function is getting a little nesty, BUT that is for now wanted to make easier and centralised changes to the configs.
The url util need's refactoring anyway.

* 🔥  urlSSL

Remove all urlSSL usages.
Add TODO's for the next commit to re-add logic for deleted logic.

e.g.

- cors helper generated an array of url's to allow requests from the defined config url's -> will be replaced by the admin url if available
- theme handler prefered the urlSSL in case it was defined -> will be replaced by using the urlFor helper to get the blog url (based on the request secure flag)

The changes in this commit doesn't have to be right, but it helped going step by step.
The next commit is the more interesting one.

* 🔥    remove forceAdminSSL, add new admin url and adapt logic

I wanted to remove the forceAdminSSL as separate commit, but was hard to realise.
That's why both changes are in one commit:

1. remove forceAdminSSL
2. add admin.url option

- fix TODO's from last commits
- rewrite the ssl middleware!
- create some private helper functions in the url helper to realise the changes
- rename some wordings and functions e.g. base === blog (we have so much different wordings)
- i would like to do more, but this would end in a non readable PR
- this commit contains the most important changes to offer admin.url option

* 🤖  adapt tests

IMPORTANT
- all changes in the routing tests were needed, because each routing test did not start the ghost server
- they just required the ghost application, which resulted in a random server port
- having a random server port results in a redirect, caused by the ssl/redirect middleware

* 😎  rename check-ssl middleware

* 🎨  fix theme-handler because of master rebase
2017-02-03 18:13:22 +00:00
Hannah Wolfe
59e2694acf Misc Middleware cleanup (#7526)
* 💄 Combine slashes & uncapitalise middleware

- these bits of middleware belong together
- ideally they should be optimised

* 🎨 Move ghostLocals out of themeHandler

GhostLocals sets several important values which are needed for every part of the application,
admin, api and theme. Therefore, it doesn't make sense for it to be bundled in the themeHandler.

* 🐛 Fix the uncapitalise middleware

- Updated to make correct use of req.baseUrl, req.path, req.url & req.originalUrl
- Updated the tests to actually cover our weird cases

* 🎨 Move ghostVersion logic out of config

* 💄 Group static / asset-related middleware together

* 🔥 Remove /shared/ asset handling

- The 5 files which are located in `/shared/` are all handled by individual calls to `serveSharedFile`
- Therefore this code is redundant
2016-10-10 21:14:32 +02:00
Katharina Irrgang
d81bc91bd2 Error creation (#7477)
refs #7116, refs #2001

- Changes the way Ghost errors are implemented to benefit from proper inheritance
- Moves all error definitions into a single file
- Changes the error constructor to take an options object, rather than needing the arguments to be passed in the correct order.
- Provides a wrapper so that any errors that haven't already been converted to GhostErrors get converted before they are displayed.

Summary of changes:

* 🐛  set NODE_ENV in config handler
*   add GhostError implementation (core/server/errors.js)
  - register all errors in one file
  - inheritance from GhostError
  - option pattern
* 🔥  remove all error files
*   wrap all errors into GhostError in case of HTTP
* 🎨  adaptions
  - option pattern for errors
  - use GhostError when needed
* 🎨  revert debug deletion and add TODO for error id's
2016-10-06 13:27:35 +01:00
Katharina Irrgang
1882278b5b 🎨 configurable logging with bunyan (#7431)
- 🛠  add bunyan and prettyjson, remove morgan

-   add logging module
  - GhostLogger class that handles setup of bunyan
  - PrettyStream for stdout

-   config for logging
  - @TODO: testing level fatal?

-   log each request via GhostLogger (express middleware)
  - @TODO: add errors to output

- 🔥  remove errors.updateActiveTheme
  - we can read the value from config

- 🔥  remove 15 helper functions in core/server/errors/index.js
  - all these functions get replaced by modules:
    1. logging
    2. error middleware handling for html/json
    3. error creation (which will be part of PR #7477)

-   add express error handler for html/json
  - one true error handler for express responses
  - contains still some TODO's, but they are not high priority for first implementation/integration
  - this middleware only takes responsibility of either rendering html responses or return json error responses

- 🎨  use new express error handler in middleware/index
  - 404 and 500 handling

- 🎨  return error instead of error message in permissions/index.js
  - the rule for error handling should be: if you call a unit, this unit should return a custom Ghost error

- 🎨  wrap serve static module
  - rule: if you call a module/unit, you should always wrap this error
  - it's always the same rule
  - so the caller never has to worry about what comes back
  - it's always a clear error instance
  - in this case: we return our notfounderror if serve static does not find the resource
  - this avoid having checks everywhere

- 🎨  replace usages of errors/index.js functions and adapt tests
  - use logging.error, logging.warn
  - make tests green
  - remove some usages of logging and throwing api errors -> because when a request is involved, logging happens automatically

- 🐛  return errorDetails to Ghost-Admin
  - errorDetails is used for Theme error handling

- 🎨  use 500er error for theme is missing error in theme-handler

- 🎨  extend file rotation to 1w
2016-10-04 16:33:43 +01:00
Kevin Ansfield
302702c740 Revert "fix: ensure we initialise activeTheme on bootstrap (#6950)"
This reverts commit 8f2afeed03.
2016-07-21 12:26:16 +01:00
Katharina Irrgang
8f2afeed03 fix: ensure we initialise activeTheme on bootstrap (#6950)
closes #6948
- the hbs engine was never initialised  when server starts
- when you request a page which does not exist, express jumps directly into the error handlers
- delete some dynamic hbs engine setters in theme handler
2016-07-21 12:09:11 +01:00
kirrg001
250d571fe6 improvement: jshint/jscs
no issue
- add mocha option to jshintrc, no need to define globals in files anymore
- call client grunt commands in case of jshint/jscs
2016-06-17 22:59:47 +02:00
Cameron Viner
8fd272476b Removed uneeded jshint comments in order to cleanup the tests
closes #6505
-Removed all of the /*jshint expr:true*/ comments from the tests
-Removed all of the should.equal(true, true) statements from the tests
-Removed should from the greenkeeper ignores
2016-02-17 12:52:43 +00:00
Cameron Viner
faba83d5dc deps: should@8.2.1
closes #6448
-upgraded should.js to the latest version (8.2.1)
-Changed the tests so that they comply with the breaking changes introduced in the new version of should.js
-Installs the package should-http so should.be.json() can be used
-Installs the package should-sinon so that should.be.calledOnce() can be used
2016-02-09 13:39:10 +00:00
Hannah Wolfe
bc97de5fe9 Unify usage of config in unit tests
no issue

- provide a single point for accessing config in unit tests
- create a single way to set and restore config
- ensure that restore deletes top level optional keys that are now undefined
- use this._config in check deprecations, otherwise the config gets cached
- solves issues with interdependent tests
2015-12-15 10:48:24 +00:00
Hannah Wolfe
4e29d9e987 Simplify theme middleware + improve tests
refs #5286, #4172, #5888

- no need to pass blogApp around in middleware
- improve test coverage to 100%
2015-10-11 22:26:43 +01:00
Fabian Becker
c1a2601514 Middleware Refactor
- Refactor SSL middleware into separate module.
- Refactor redirectToSetup to separate module + tests
- Refactor serveStaticFile + tests
- Refactor authentication middleware + tests
- Refactor private blogging middleware

refs #5286
2015-08-04 14:53:58 +02:00