diff --git a/docs/guides/caching.md b/docs/guides/caching.md new file mode 100644 index 00000000..bc07acc1 --- /dev/null +++ b/docs/guides/caching.md @@ -0,0 +1,169 @@ +# Server-Side Caching with Hapi.js + +## Redis + +When an external cache is needed, Redis is the recommended choice. For local development, a [Docker image](https://hub.docker.com/_/redis) can be used. + +## Hapi.js server-side caching + +Hapi server-side caching uses the [catbox](https://hapi.dev/module/catbox/) interface to abstract the underlying caching technology (e.g. memory, Redis). + +There are three main concepts: + +* The cache **strategy** (or provider): the underlying caching technology. [`@hapi/catbox-redis`](https://github.com/hapijs/catbox-redis) is the Redis adapter for catbox. +* The cache **client**: a low-level cache abstraction, initialised using a strategy. Hapi initialises an in-memory cache client by default, and you can create additional cache clients using the same or different strategies. +* The cache **policy**: a higher-level cache abstraction that sets a policy on storage within the cache (e.g. expiry times). The cache policy provides additional segmentation within the cache client. Typically the cache policy is how you interact with cache values via the `get` and `set` methods. + +### Configuring the default cache client + +Hapi initialises an in-memory cache client by default. You can make the default cache client use a different strategy. For example, the following uses Redis or memory depending on configuration: + +```javascript +import Hapi from '@hapi/hapi' +import { Engine as CatboxRedis } from '@hapi/catbox-redis' +import { Engine as CatboxMemory } from '@hapi/catbox-memory' +import config from './config.js' + +const CacheEngine = config.useRedis ? CatboxRedis : CatboxMemory +const cacheOptions = config.useRedis + ? { + host: config.cache.host, + port: config.cache.port, + password: config.cache.password, + partition: config.cache.partition, + tls: config.isProd ? {} : undefined + } + : {} + +const server = Hapi.server({ + port: config.port, + cache: [{ + provider: { + constructor: CacheEngine, + options: cacheOptions + } + }] +}) +``` + +### Configuring additional cache clients + +Additional cache clients can be created when initialising the Hapi server by adding new definitions to the `cache` array. Additional caches must be given a `name`. For example, the following creates a new Redis cache client called `session`: + +```javascript +import Hapi from '@hapi/hapi' +import { Engine as CatboxRedis } from '@hapi/catbox-redis' +import config from './config.js' + +const server = Hapi.server({ + port: config.port, + cache: [{ + name: 'session', + provider: { + constructor: CatboxRedis, + options: { + host: config.cache.host, + port: config.cache.port, + password: config.cache.password, + partition: config.cache.partition, + tls: config.isProd ? {} : undefined + } + } + }] +}) +``` + +This example creates two cache clients: the default in-memory cache client and a new cache client called `session` that uses Redis. + +Hapi will always use the default in-memory cache client unless you specify the `name` when using it (either directly or via the cache policy). + +### Creating and using a cache policy + +The cache policy is typically how you interact with the cache. See the [catbox policy documentation](https://hapi.dev/module/catbox/api/) for details on how to set and get data. + +To create a cache policy using a segment within the default cache client: + +```javascript +const myCache = server.cache({ + expiresIn: config.cache.ttl, + segment: 'mySegment' +}) +``` + +To create a cache policy using a segment within a named cache client: + +```javascript +const myCache = server.cache({ + cache: 'session', + expiresIn: config.cache.ttl, + segment: 'mySegment' +}) +``` + +## Integration with yar session cookies + +[`@hapi/yar`](https://hapi.dev/module/yar/) is a plugin that adds unauthenticated session support (state across multiple browser requests) to Hapi. By default it tries to fit session data into a session cookie, but will use server-side storage via the Hapi cache interface if the session data exceeds the maximum cookie size. + +Combining yar with Redis caching allows multiple replicas of a web server to share server-side user session data. + +Example configuration using the default cache client: + +```javascript +import Yar from '@hapi/yar' + +await server.register({ + plugin: Yar, + options: { + maxCookieSize: 0, + cache: { + expiresIn: config.cache.ttl + }, + cookieOptions: { + password: config.cookie.password, + isSecure: config.isProd + } + } +}) +``` + +Setting `maxCookieSize` to `0` forces all session data to be stored server-side. + +Example configuration using a named cache client: + +```javascript +import Yar from '@hapi/yar' + +await server.register({ + plugin: Yar, + options: { + maxCookieSize: 0, + cache: { + cache: 'session', + expiresIn: config.cache.ttl + }, + cookieOptions: { + password: config.cookie.password, + isSecure: config.isProd + } + } +}) +``` + +## catbox-redis connection options + +`@hapi/catbox-redis` v7 uses [ioredis](https://github.com/redis/ioredis) under the hood. The following options are supported: + +| Option | Description | +| --- | --- | +| `host` | Redis server hostname (default: `127.0.0.1`) | +| `port` | Redis server port (default: `6379`) | +| `password` | Authentication password | +| `db` | Database number | +| `partition` | Key prefix for cache segmentation | +| `tls` | TLS configuration object (pass `{}` for default TLS settings) | +| `url` | Redis connection URL (alternative to `host`/`port`) | +| `client` | Pre-configured ioredis instance (must expose a `status` property set to `'ready'`) | +| `sentinels` | Array of `{ host, port }` sentinel addresses | +| `sentinelName` | Sentinel master name (required with `sentinels`) | + +Only one connection method should be used: `host`/`port`, `url`, or `client`. diff --git a/docs/guides/sonarqube_ide.md b/docs/guides/sonarqube_ide.md new file mode 100644 index 00000000..26c59a60 --- /dev/null +++ b/docs/guides/sonarqube_ide.md @@ -0,0 +1,42 @@ +# SonarQube for IDE + +[SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/) is an IDE extension that identifies code quality issues as you code, providing immediate feedback before committing. + +Running analysis locally, rather than waiting for a CI pipeline or a reviewer to flag issues, means you can: + +- fix bugs, code smells and security vulnerabilities as you introduce them, when the context is still fresh in your mind +- avoid pull requests being blocked or sent back for issues that a quick local check would have caught +- spend review time on design and logic, rather than on issues a linter could have found +- learn the reasoning behind each rule from inline explanations, rather than just being told something is wrong + +All Defra projects are required to set up their repositories within the [SonarQube Cloud Defra organisation](https://sonarcloud.io/organizations/defra/projects). Using the IDE extension in connected mode ensures you get the same rules and quality gates locally as in CI, so there are no surprises when your code reaches the pipeline. + +## Dependencies + +- Java Runtime Environment v21+ + +With Ubuntu (including WSL), install the open source JRE: + +```bash +sudo apt-get install openjdk-21-jre +``` + +## VS Code setup + +1. Install the [SonarQube for IDE](https://marketplace.visualstudio.com/items?itemName=SonarSource.sonarlint-vscode) extension. + +2. Set the JRE location in VS Code settings: + +```json +{ + "sonarlint.ls.javaHome": "/usr/lib/jvm/java-21-openjdk-amd64" +} +``` + +This gives you Sonar code analysis using default quality gates for supported languages. + +## Connected mode + +Connected mode binds the extension to your actual project in SonarQube Cloud. This ensures the IDE uses the same rules, quality gates, and exclusions as the SonarQube project configured for your repository. + +Follow the [connected mode setup documentation](https://docs.sonarsource.com/sonarqube-for-ide/vs-code/team-features/connected-mode-setup/#connection-setup) to connect your SonarQube for IDE extension to the SonarQube Cloud project. diff --git a/docs/processes/pull_requests.md b/docs/processes/pull_requests.md index ae340a42..919ab2a7 100644 --- a/docs/processes/pull_requests.md +++ b/docs/processes/pull_requests.md @@ -80,12 +80,14 @@ It's on you to keep your branch up to date with with your main development branc git rebase origin/main ``` -## Get it looked at +## Get it reviewed When you're finished and have pushed your last commit request someone to review it. If there are multiple members on your team and all could review, feel free to request them all. The key thing is at least one other person should review the PR before it is merged. You and the reviewer will then work to confirm the changes are OK. Once the reviewer is happy they need to **approve it**. +When code reviews are conducted to a high standard, they provide a valuable learning opportunity for the author, reviewer and any observers of the review process. + ## Completing the PR Once approved to complete the PR you'll need to `squash` your commits down to one. @@ -97,3 +99,115 @@ The simplest way to do this is in the GitHub UI. Within the PR the merge button When you click it GitHub will present a box which contains the combined text from all the commit messages in the PR. Use this opportunity to reword the content to a single commit message (the smaller the change the simpler this is to do!) When done ensure you delete the branch. Again GitHub will present this option in the UI immediately after merging so make use of it then. + +## Tips for reviewing PRs + +### Tone of code review comments + +The tone of communications is extremely important in fostering an inclusive, collaborative atmosphere within teams. + +Remember that your colleagues put a lot of effort into their work and may feel offended by harsh criticism, particularly if you make assumptions or imply a lack of thought. Approach code reviews with kindness and empathy, assuming the best intentions and applauding good solutions as well as making suggestions for improvement. + +Comments should be used to give praise for good solutions and to point out potential improvements. They should not be used to criticise your colleagues or make strongly opinionated statements. Always be mindful of your tone, considering how others might perceive your comments, and be explicit about when a comment is non-blocking or unimportant. + +#### Be constructive + +Don't use harsh language or criticise without making constructive suggestions. +Do suggest alternative approaches and explain your reasoning. + +#### Be specific + +Don't make vague statements about the changes as a whole. +Do point out specific issues and offer specific ideas for how the changes can be improved. + +#### Avoid strong opinions + +Don't make strong, opinionated statements or dictate specific changes. +Do ask open-ended questions and keep an open mind about alternative solutions and reasoning that you may not have thought of. + +### Scope of a code review + +Code reviews should focus on what is being changed and whether the change is appropriate. The scope will be stated in the acceptance criteria of the ticket. + +Expanding the scope of a pull request at the review stage is not acceptable. It is generally more valuable to swiftly conclude a piece of work that the team has prioritised than to opportunistically seek additional changes, such as refactoring related code. That said, it can be useful to comment on refactoring opportunities without blocking the pull request. + +### What to look for + +#### Are the changes focused? + +Are the changes focused on a specific issue, referenced in the pull request description? If the changes go beyond the intended scope, should they be broken up to make the code review more manageable? + +#### Maintainability + +Is all new code extensible and easy for other developers to understand? Does it follow common design patterns? Look for unnecessary complexity and remember that this is subjective so take care not to be overly critical or opinionated. + +#### Duplication + +Is there duplication within new code or between new and existing code? Could an existing abstraction be reused or should a new abstraction be created? + +Identifying useful abstractions at the review stage may indicate that there wasn't enough collaboration before coding began. See this as a trigger to review the team's ways of working, rather than blocking a pull request unnecessarily. + +#### Reusability + +Have any new abstractions been introduced? Are they sufficiently reusable? If other parts of the system could be updated to use the new abstractions, consider suggesting this in a non-blocking comment. + +#### Impact on other parts of the system + +Will the changes have knock-on effects or otherwise necessitate changes to other parts of the system? + +#### Unit test coverage + +Is all new code covered by detailed unit tests? Have any edge cases been missed? Don't rely on metrics such as code coverage. Inspect the code thoroughly and ensure that the tests contain appropriate assertions to confirm that all the intended functionality works as expected. + +#### Integration tests + +Have integration tests been added to cover all changes to functionality? + +### Concluding a review + +When concluding a review, there are three options: + +1. Comment +2. Approve the PR +3. Block the PR by requesting changes + +#### When to comment + +You should conclude with a comment when your review asks questions which need answering before you can determine whether the pull request is acceptable. If you haven't proposed a solution to a specific problem in the pull request, it's generally better to leave a neutral review than to block the PR with a request for change. + +#### When to approve + +You should approve a pull request when you have confirmed that: + +1. it meets the objectives set out in its description +2. it doesn't introduce new defects or code that is hard to maintain +3. all new and modified code has thorough unit test coverage +4. integration tests have been added where appropriate +5. there are no unresolved questions or comments against the pull request + +Occasionally, it may be prudent to accept a pull request which does not meet all of the above requirements, such as to resolve an urgent issue with the live product. Such cases must always be agreed between the product owner, author(s) and reviewer(s). + +If comments or questions on a pull request have been addressed elsewhere (e.g. face-to-face or on Slack), ensure that the outcome is recorded in comment replies so that it is visible to anyone looking back at the pull request in future. + +Use the Resolve button to make it clear which of your concerns have been addressed and which still need attention. A pull request should only be approved after all reviewers have explicitly indicated that each of their comments have been resolved. + +Sometimes, a reviewer may become unavailable after commenting on a pull request. When that happens, a second reviewer may accept the pull request without resolving the first reviewer's comments, as long as the author and second reviewer agree that they believe all legitimate concerns have been addressed. + +#### When to request changes + +You should request changes to a pull request if any of the following are true: + +1. it creates a defect in the product +2. it exacerbates an existing defect +3. it doesn't meet the objectives set out in its description +4. it would make the product more difficult to maintain +5. new or modified code lacks thorough unit tests +6. required integration tests have not been included + +#### When **not** to request changes + +> Perfection is the enemy of good + +A pull request does not need to be perfect to be good enough. Often, there are many solutions to a problem and one which is not the best is still good enough to meet current needs. + +Make suggestions for improvement as comments without explicitly approving or rejecting the pull request. This way, your suggestions can open dialogue with the author about how important your suggestions are compared to other work you could each move on to. diff --git a/mkdocs.yml b/mkdocs.yml index bb200d10..c735b943 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -56,11 +56,12 @@ nav: - Resolving GitHub security alerts: processes/github_security_alerts.md - Guides: - General guidance: guides/README.md + - Server-Side Caching with Hapi.js: guides/caching.md - Choosing between a mono repo and a multi repo: guides/mono_or_multi_repo.md - Choosing packages: guides/choosing_packages.md - - Developer workflows: guides/developer_workflows.md - Cookie banner: guides/cookies-banner.md - Defra Identity: guides/defra-id.md + - Developer workflows: guides/developer_workflows.md - Docker guidance: guides/docker_guidance.md - Entra: guides/entra.md - GitHub Advanced Security: guides/github_advanced_security.md @@ -74,6 +75,7 @@ nav: - PL/SQL auto-format with TOAD: guides/plsql_auto_format_toad.md - SQL Prompt: guides/sql_prompt_tool.md - Shared libraries: guides/shared_libraries.md + - SonarQube for IDE: guides/sonarqube_ide.md - Style guide for standards: guides/style_guide_for_standards.md - Training: guides/training.md - Using AWS session manager: guides/aws_session_manager.md