Rewrite of Image and Files (breaking)

.changeset/unlucky-ducks-raise.md

@@ -0,0 +1,113 @@
+---
+'@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: 's3' },
+    lists: { 
+        images: { fields: { image() }
+        /* ... */
+    },
+    /* ... */
+})
+```
+
+New:
+
+```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: {
+        images: { fields: 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: {
+        images: { fields: { image({ storage: 'my_image_storage' }) } },
+        user: { fields: avatar: relationship({ ref: 'images' })}
+        blog: { fields: photos: relationship({ ref: 'images', 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 no 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.
+
+
+```ts
+{
+-   baseUrl: '/images'
++   serverRoute: {
++       path: '/images'
++   }
+}
+```
+
+#### New bits
+
+S3 support is no longer 'experimental'
+S3 supports proxying through the keystone server
+S3 signing is now supported
+`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 (or not)
+
+#### Changes
+
+

.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