Rewrite of Image and Files (#7070)

* First pass at s3AssetsAPI, now to wire up * Implemented asset mode switch in createImagesContext * Implemented asset mode switch in createFilesContext * Renamed function * Fixed bugs caused by trying to send a stream to S3, now sending buffer instead * Update packages/keystone/src/lib/context/createImagesContext.ts Co-authored-by: Daniel Cousens <413395+dcousens@users.noreply.github.com> * Tidy up * Stream file upload * Fix urls * Init s3 at startup rather than per request * Move things * Remove a duplicated thing * Tests * Update schemas * Update CI config * Change bucket name * Make test bucket publicly readable * Fix CI config * Prettier * Apply suggestions from code review Co-authored-by: Daniel Cousens <413395+dcousens@users.noreply.github.com> * Update other postgres user and password * Get image size if it's not stored in the metadata * Add test for uploading an image directly to s3 and adding it to a field via ref * Docs and changeset * comparable functionality * small examples schema update * Add removeOnDelete option * update yarn.lock * remove fast-glob duplicate from devDependencies * bump dotenv * small updates * adding/updating tests * use return, not break, fix broken S3 file deletion * work in progress changes * Fix things * WIP path prefix * Fix a thing * continue, not break * Fix preserve * Test fixes 1 * tests updates 2 * test fixes 3 * remove problematic import * yarn updates * typescript fixes * fix ordering * Revert error message change * store test files in temp dir * Run deletions at source before operation * Fix local file serving and basic example * base documentation updates * Fix s3 proxy things * Delete CHANGELOG.md * Use my_ to disambiguate naming formality * Config updates * Use my_ for examples * Small docs fix * update changelog to be more reflective of actual changes * rename addServerRoute to serverRoute * drop superfluous .assets object * rephrase unknown storage key error * updates to storage config * Update S3 example * S3 Assets example update * remove unused ref code * Add JSDoc comments to StorageConfig types * change word * Fix URLs * Update packages/core/package.json * update images ui * add target _blank to image anchor * UI updates * internal rename for consistency * UI updates * add transformName for both files and images * docs update * rename generatedUrl to generateUrl * Shorten config example * update generateUrl * small fixes * change files on S3 to octet streams * remove redundant null check * Update default generateName for files * remove redundant error checking * Apply suggestions from code review * move endpoint string manipulation out of request * yarn format * Update local s3 serving * Remove s3 proxy * changes inline documentation copy * Await deletions * fix images to store the data * change warning copy * update yarn.lock * yarn format * does this work? * Delete utils.ts * Update .changeset/unlucky-ducks-raise.md Co-authored-by: Josh Calder <josh@opensaas.com.au> * Update .changeset/unlucky-ducks-raise.md Co-authored-by: Daniel Cousens <413395+dcousens@users.noreply.github.com> * update changeset * Update changeset Co-authored-by: Daniel Cousens <413395+dcousens@users.noreply.github.com> Co-authored-by: mitchellhamilton <mitchell@hamil.town> Co-authored-by: Noviny BC <noviny@thinkmill.com.au> Co-authored-by: Daniel Cousens <dcousens@users.noreply.github.com> Co-authored-by: Noviny <ben@thinkmill.com.au> Co-authored-by: Josh Calder <josh.calder@thinkmill.com.au> Co-authored-by: gwyneplaine <cc.lee@live.com.au> Co-authored-by: Josh Calder <josh@opensaas.com.au>
keystonejs · May 26, 2022 · ae81dc1 · ae81dc1
1 parent ccbc248
commit ae81dc1
Show file tree

Hide file tree

Showing 61 changed files with 2,927 additions and 1,892 deletions.
diff --git a/.changeset/unlucky-ducks-raise.md b/.changeset/unlucky-ducks-raise.md
@@ -0,0 +1,127 @@
+---
+'@keystone-6/core': major
+---
+#### Breaking
+
+##### Move of `image` and `files` in the keystone config into new option `storage`
+
+The `image` and `files` config options have been removed from Keystone's config - the configuration has
+been moved into a new `storage` configuration object.
+
+Old:
+
+```ts
+export default config({
+  image: { upload: 'local' },
+  lists: {
+    Image: { fields: { image: image() } },
+    /* ... */
+  },
+  /* ... */
+});
+```
+
+New: 
+```ts
+export default config({
+  storage: {
+    my_images: {
+      kind: 'local',
+      type: 'image',
+      generateUrl: path => `http://localhost:3000/images${path}`,
+      serverRoute: { path: '/images' },
+      storagePath: 'public/images',
+    },
+  },
+  lists: {
+    Image: { fields: { image: image({ storage: 'my_images' }) } },
+    /* ... */
+  },
+  /* ... */
+});
+```
+
+You can also now store assets on S3:
+
+```ts
+export default config({
+  storage: {
+    my_image_storage: {
+      kind: 's3',
+      type: 'image',
+      bucketName: S3_BUCKET_NAME,
+      region: S3_REGION,
+      accessKeyId: S3_ACCESS_KEY_ID,
+      secretAccessKey: S3_SECRET_ACCESS_KEY,
+    },
+  },
+  lists: {
+    Image: { fields: { image: image({ storage: 'my_image_storage' }) } },
+    /* ... */
+  },
+  /* ... */
+});
+```
+
+##### Removal of refs for `images` and `files`
+
+Refs were an interesting situation! They allowed you to link images stored in your storage source (s3 or local), and use the same
+image anywhere else images are used. This causes a bunch of complication, and prevented Keystone ever reliably being able
+to remove images from your source, as it couldn't easily track where images were used.
+
+To simplify things for you and us, we're removing `refs` as a concept, but don't panic just yet, we are conceptually replacing
+them with something you are already familiar with: `relationships`.
+
+If you wanted refs, where images could be available in multiple places, our new recommendation is:
+
+```ts
+export default config({
+  storage: {
+    my_image_storage: {
+      /* ... */
+    },
+  },
+  lists: {
+    Image: { fields: { image: image({ storage: 'my_image_storage' }) } },
+    User: { fields: { avatar: relationship({ ref: 'Image' }) } },
+    Blog: { fields: { photos: relationship({ ref: 'Image', many: true }) } },
+    /* ... */
+  },
+  /* ... */
+});
+```
+
+This allows mirroring of the old functionality, while allowing us to add the below feature/breaking change.
+
+##### Images and Files will now be deleted when deleted
+
+Before this change, if you uploaded a file or image, Keystone would never remove it from where it was stored. The inability to tidy up unused
+files or images was unwelcome. With the removal of `ref`, we can now remove things from the source, and this will be done by default.
+
+If you don't want files or images removed, we recommend storing them as a `relationship`, rather than on items themselves, so the files
+or images persist separate to lists that use them.
+
+If you want the existing behaviour of keystone, set `preserve: true` on the storage instead.
+
+##### `file` and `image` URLs now use `generateUrl`, allowing more control over what is returned to the user
+
+##### Local images no longer need a baseUrl
+
+Previously, if you were using local storage (you scallywag), you needed to provide a path for keystone to host images on. You
+can still do this, but if you plan on serving them from another location, you can opt into not doing this.
+
+
+```diff
+{
+-   baseUrl: '/images'
++   serverRoute: {
++       path: '/images'
++   }
+}
+```
+
+#### New bits
+
+- S3 is now supported! See the `storage` config for all the options for S3.
+- `preserve` flag added to both `file` and `image` fields to allow removal of files from the source
+- Support for multiple `storage` sources - each `image` and `file` field can now use its own config you want.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -37,8 +37,8 @@ jobs:
       postgres:
         image: postgres:12
         env:
-          POSTGRES_USER: keystone5
-          POSTGRES_PASSWORD: k3yst0n3
+          POSTGRES_USER: testuser
+          POSTGRES_PASSWORD: testpass
           POSTGRES_DB: test_db
         ports:
           - 5432:5432
@@ -80,15 +80,15 @@ jobs:
 
       - name: Unit tests
         if: needs.should_run_tests.outputs.shouldRunTests == 'true'
-        run: yarn jest --ci --runInBand api-tests
+        run: yarn jest --ci --runInBand api-tests --testPathIgnorePatterns=examples-smoke-tests  --testPathIgnorePatterns=tests/api-tests/fields/crud
         env:
           CLOUDINARY_CLOUD_NAME: ${{ secrets.CLOUDINARY_CLOUD_NAME }}
           CLOUDINARY_KEY: ${{ secrets.CLOUDINARY_KEY }}
           CLOUDINARY_SECRET: ${{ secrets.CLOUDINARY_SECRET }}
           CI_NODE_TOTAL: 9
           CI_NODE_INDEX: ${{ matrix.index }}
           TEST_ADAPTER: ${{ matrix.adapter }}
-          DATABASE_URL: ${{ matrix.adapter == 'sqlite' && 'file:./dev.db' || 'postgres://keystone5:k3yst0n3@localhost:5432/test_db' }}
+          DATABASE_URL: ${{ matrix.adapter == 'sqlite' && 'file:./dev.db' || 'postgres://testuser:testpass@localhost:5432/test_db' }}
 
   unit_tests:
     name: Package Unit Tests
@@ -129,6 +129,89 @@ jobs:
           CLOUDINARY_KEY: ${{ secrets.CLOUDINARY_KEY }}
           CLOUDINARY_SECRET: ${{ secrets.CLOUDINARY_SECRET }}
 
+  field_crud_tests:
+    name: Field CRUD Tests
+    needs: should_run_tests
+    runs-on: ubuntu-latest
+    services:
+      postgres:
+        image: postgres:12
+        env:
+          POSTGRES_USER: testuser
+          POSTGRES_PASSWORD: testpass
+          POSTGRES_DB: test_db
+        ports:
+          - 5432:5432
+    strategy:
+      fail-fast: false
+      matrix:
+        adapter: ['postgresql', 'sqlite']
+    steps:
+      - name: Checkout Repo
+        if: needs.should_run_tests.outputs.shouldRunTests == 'true'
+        uses: actions/checkout@v2
+
+      - name: Setup Node.js LTS
+        if: needs.should_run_tests.outputs.shouldRunTests == 'true'
+        uses: actions/setup-node@main
+        with:
+          node-version: lts/*
+
+      - name: Get yarn cache directory path
+        if: needs.should_run_tests.outputs.shouldRunTests == 'true'
+        id: yarn-cache-dir-path
+        run: echo "::set-output name=dir::$(yarn cache dir)"
+
+      - uses: actions/cache@v2
+        if: needs.should_run_tests.outputs.shouldRunTests == 'true'
+        id: yarn-cache
+        with:
+          path: |
+            ${{ steps.yarn-cache-dir-path.outputs.dir }}
+            node_modules
+          key: ${{ runner.os }}-yarn-v5-${{ hashFiles('yarn.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-yarn-v5-
+
+      - name: Install Dependencies
+        if: needs.should_run_tests.outputs.shouldRunTests == 'true'
+        run: yarn
+      - name: Setup local S3 bucket
+        if: needs.should_run_tests.outputs.shouldRunTests == 'true'
+        run: |
+          docker run -d -p 9000:9000 --name minio \
+                     -e "MINIO_ACCESS_KEY=keystone" \
+                     -e "MINIO_SECRET_KEY=keystone" \
+                     -v /tmp/data:/data \
+                     -v /tmp/config:/root/.minio \
+                     minio/minio server /data
+
+          export AWS_ACCESS_KEY_ID=keystone
+          export AWS_SECRET_ACCESS_KEY=keystone
+          export AWS_EC2_METADATA_DISABLED=true
+
+
+
+          aws --endpoint-url http://127.0.0.1:9000/ s3 mb s3://keystone-test
+          aws --endpoint-url http://127.0.0.1:9000/ s3api put-bucket-policy --bucket keystone-test --policy file://tests/api-tests/s3-public-read-policy.json
+
+      - name: Unit tests
+        if: needs.should_run_tests.outputs.shouldRunTests == 'true'
+        run: yarn jest --ci --runInBand tests/api-tests/fields/crud
+        env:
+          CLOUDINARY_CLOUD_NAME: ${{ secrets.CLOUDINARY_CLOUD_NAME }}
+          CLOUDINARY_KEY: ${{ secrets.CLOUDINARY_KEY }}
+          CLOUDINARY_SECRET: ${{ secrets.CLOUDINARY_SECRET }}
+          S3_ENDPOINT: http://127.0.0.1:9000/
+          S3_FORCE_PATH_STYLE: true
+          S3_BUCKET_NAME: keystone-test
+          S3_ACCESS_KEY_ID: keystone
+          S3_SECRET_ACCESS_KEY: keystone
+          # this doesn't mean anything when we're using a custom s3 endpoint but the sdk wants something so we just give it something
+          S3_REGION: us-east-1
+          TEST_ADAPTER: ${{ matrix.adapter }}
+          DATABASE_URL: ${{ matrix.adapter == 'sqlite' && 'file:./dev.db' || 'postgres://testuser:testpass@localhost:5432/test_db' }}
+
   examples_tests:
     name: Testing example project
     needs: should_run_tests