diff --git a/.github/workflows/deploy-image.yml b/.github/workflows/deploy-image.yml new file mode 100644 index 00000000..ed808f96 --- /dev/null +++ b/.github/workflows/deploy-image.yml @@ -0,0 +1,55 @@ +name: Deploy Image + +on: + workflow_call: + inputs: + env_tag: + required: true + type: string + image_tag: + required: true + type: string + +jobs: + deploy: + permissions: + id-token: write + contents: read + runs-on: ubuntu-latest + env: + ENV_TAG: ${{ inputs.env_tag }} + ECR_REPOSITORY: discovery-api + steps: + - name: Checkout repo + uses: actions/checkout@v3 + + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@v2 + with: + role-to-assume: arn:aws:iam::946183545209:role/GithubActionsDeployerRole + aws-region: us-east-1 + + - name: Log in to ECR + id: login-ecr + uses: aws-actions/amazon-ecr-login@v1 + + - name: Back up previous image for rollback + run: | + MANIFEST=$(aws ecr batch-get-image --repository-name $ECR_REPOSITORY --image-ids imageTag="${ENV_TAG}-latest" --output json | jq --raw-output --join-output '.images[0].imageManifest') + PREVIOUS_MANIFEST=$(aws ecr batch-get-image --repository-name $ECR_REPOSITORY --image-ids imageTag="${ENV_TAG}-previous" --output json | jq --raw-output --join-output '.images[0].imageManifest') + if [ "$MANIFEST" != "$PREVIOUS_MANIFEST" ]; then + aws ecr put-image --repository-name $ECR_REPOSITORY --image-tag "${ENV_TAG}-previous" --image-manifest "$MANIFEST" + fi + + - name: Build, tag, and push image to Amazon ECR + env: + ECR_REGISTRY: ${{ steps.login-ecr.outputs.registry }} + run: | + docker build -t $ECR_REGISTRY/$ECR_REPOSITORY:${{ inputs.image_tag }} . + docker push $ECR_REGISTRY/$ECR_REPOSITORY:${{ inputs.image_tag }} + docker tag $ECR_REGISTRY/$ECR_REPOSITORY:${{ inputs.image_tag }} $ECR_REGISTRY/$ECR_REPOSITORY:${ENV_TAG}-latest + docker push $ECR_REGISTRY/$ECR_REPOSITORY:${ENV_TAG}-latest + + - name: Force ECS Update + run: | + aws ecs update-service --cluster discovery-api-${ENV_TAG} --service discovery-api-${ENV_TAG} --force-new-deployment diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index a89f2d98..a2e49007 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -1,4 +1,4 @@ -name: Deploy +name: Test-Deploy-Int-Roll on: push: @@ -21,40 +21,27 @@ jobs: permissions: id-token: write contents: read - runs-on: ubuntu-latest needs: tests - steps: - - name: Checkout repo - uses: actions/checkout@v3 + uses: ./.github/workflows/deploy-image.yml + with: + env_tag: ${{ github.ref_name }} + image_tag: ${{ github.sha }} - - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@v2 - with: - role-to-assume: arn:aws:iam::946183545209:role/GithubActionsDeployerRole - aws-region: us-east-1 - - - name: Log in to ECR - id: login-ecr - uses: aws-actions/amazon-ecr-login@v1 - - - name: Back up previous image for rollback - run: | - MANIFEST=$(aws ecr batch-get-image --repository-name ${{ env.ECR_REPOSITORY }} --image-ids imageTag="${{ env.ENV_TAG }}-latest" --output json | jq --raw-output --join-output '.images[0].imageManifest') - PREVIOUS_MANIFEST=$(aws ecr batch-get-image --repository-name ${{ env.ECR_REPOSITORY }} --image-ids imageTag="${{ env.ENV_TAG }}-previous" --output json | jq --raw-output --join-output '.images[0].imageManifest') - if [ "$MANIFEST" != "$PREVIOUS_MANIFEST" ]; then - aws ecr put-image --repository-name ${{ env.ECR_REPOSITORY }} --image-tag "${{ env.ENV_TAG }}-previous" --image-manifest "$MANIFEST" - fi - - - name: Build, tag, and push image to Amazon ECR - env: - ECR_REGISTRY: ${{ steps.login-ecr.outputs.registry }} - IMAGE_TAG: ${{ github.sha }} - run: | - docker build -t $ECR_REGISTRY/${{ env.ECR_REPOSITORY }}:$IMAGE_TAG . - docker push $ECR_REGISTRY/${{ env.ECR_REPOSITORY }}:$IMAGE_TAG - docker tag $ECR_REGISTRY/${{ env.ECR_REPOSITORY }}:$IMAGE_TAG $ECR_REGISTRY/${{ env.ECR_REPOSITORY }}:${{ env.ENV_TAG }}-latest - docker push $ECR_REGISTRY/${{ env.ECR_REPOSITORY }}:${{ env.ENV_TAG }}-latest + smoke-test: + permissions: + id-token: write + contents: read + needs: deploy + uses: ./.github/workflows/integration-tests.yml + with: + branch: ${{ github.ref_name }} - - name: Force ECS Update - run: | - aws ecs update-service --cluster discovery-api-${{ env.ENV_TAG }} --service discovery-api-${{ env.ENV_TAG }} --force-new-deployment \ No newline at end of file + rollback: + permissions: + id-token: write + contents: read + needs: smoke-test + if: ${{ failure() && needs.smoke-test.result == 'failure' && github.ref_name != 'production' }} + uses: ./.github/workflows/rollback-qa2.yml + with: + branch: ${{ github.ref_name }} \ No newline at end of file diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index eb0bc040..cc2c391b 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -1,17 +1,18 @@ name: Run Smoke Tests on: - workflow_run: - workflows: ["Deploy"] - types: [completed] - branches: [production, qa] + workflow_call: + inputs: + branch: + required: true + type: string permissions: id-token: write contents: read env: - ENV: ${{ github.event.workflow_run.head_branch }} + ENV: ${{ inputs.branch }} jobs: integration-test: diff --git a/.github/workflows/rollback-qa2.yml b/.github/workflows/rollback.yml similarity index 64% rename from .github/workflows/rollback-qa2.yml rename to .github/workflows/rollback.yml index 925d8b4f..6025a6a3 100644 --- a/.github/workflows/rollback-qa2.yml +++ b/.github/workflows/rollback.yml @@ -1,8 +1,11 @@ -name: Rollback qa2 +name: Rollback ${{inputs.branch}} on: - workflow_dispatch: - + workflow_call: + inputs: + branch: + required: true + type: string jobs: # Rollback job in case of failure (Revert qa to the previous task definition) rollback: @@ -36,10 +39,10 @@ jobs: ECR_REGISTRY: ${{ steps.login-ecr.outputs.registry }} ECR_REPOSITORY: discovery-api run: | - docker pull $ECR_REGISTRY/$ECR_REPOSITORY:qa2-previous - docker tag $ECR_REGISTRY/$ECR_REPOSITORY:qa2-previous $ECR_REGISTRY/$ECR_REPOSITORY:qa2-latest - docker push $ECR_REGISTRY/$ECR_REPOSITORY:qa2-latest + docker pull $ECR_REGISTRY/$ECR_REPOSITORY:${{inputs.branch}}-previous + docker tag $ECR_REGISTRY/$ECR_REPOSITORY:${{inputs.branch}}-previous $ECR_REGISTRY/$ECR_REPOSITORY:${{inputs.branch}}-latest + docker push $ECR_REGISTRY/$ECR_REPOSITORY:${{inputs.branch}}-latest - name: Force ECS Update run: | - aws ecs update-service --cluster discovery-api-qa2 --service discovery-api-qa2 --force-new-deployment \ No newline at end of file + aws ecs update-service --cluster discovery-api-${{inputs.branch}} --service discovery-api-${{inputs.branch}} --force-new-deployment \ No newline at end of file diff --git a/lib/api-request.js b/lib/api-request.js index 73779f1c..aaf7070a 100644 --- a/lib/api-request.js +++ b/lib/api-request.js @@ -8,7 +8,7 @@ const QUOTE_CHARS = '"\u201C\u201D\u201E\u201F\u2033\u2036' const IN_QUOTES_PATTERN = new RegExp(`^[${QUOTE_CHARS}][^${QUOTE_CHARS}]+[${QUOTE_CHARS}]$`) class ApiRequest { - static ADVANCED_SEARCH_PARAMS = ['title', 'subject', 'contributor', 'callnumber', 'standard_number'] + static ADVANCED_SEARCH_PARAMS = ['title', 'subject', 'contributor', 'callnumber', 'standard_number', 'series', 'genre'] static IDENTIFIER_NUMBER_PARAMS = ['isbn', 'issn', 'lccn', 'oclc'] constructor (params) { diff --git a/lib/elasticsearch/config.js b/lib/elasticsearch/config.js index bb74f353..c499a78a 100644 --- a/lib/elasticsearch/config.js +++ b/lib/elasticsearch/config.js @@ -43,17 +43,17 @@ const SEARCH_SCOPES = { 'titleAlt.folded', 'uniformTitle.folded', 'titleDisplay.folded', - 'seriesStatement.folded', 'contentsTitle.folded', 'donor.folded', 'parallelTitle.folded^5', 'parallelTitleDisplay.folded', - 'parallelSeriesStatement.folded', 'parallelTitleAlt.folded', 'parallelCreatorLiteral.folded', 'parallelUniformTitle', 'formerTitle', - 'addedAuthorTitle' + 'addedAuthorTitle', + 'seriesUniformTitle', + 'parallelSeriesUniformTitle' ] }, contributor: { @@ -63,7 +63,21 @@ const SEARCH_SCOPES = { fields: ['subjectLiteral^2', 'subjectLiteral.folded', 'parallelSubjectLiteral.folded'] }, series: { - fields: ['seriesStatement.folded'] + fields: [ + 'series^2', + 'series.folded', + 'parallelSeries^2', + 'parallelSeries.folded', + 'seriesUniformTitle', + 'parallelSeriesUniformTitle^2', + 'parallelSeriesUniformTitle.folded', + 'seriesAddedEntry', + 'parallelSeriesAddedEntry^2', + 'parallelSeriesAddedEntry.folded' + ] + }, + genre: { + fields: ['genreForm^2', 'genreForm.folded'] }, journal_title: { fields: null @@ -107,7 +121,7 @@ const FILTER_CONFIG = { titleAlt: { operator: 'match', field: ['titleAlt.raw', 'parallelTitleAlt.raw'], repeatable: true }, uniformTitle: { operator: 'match', field: ['uniformTitle.raw', 'parallelUniformTitle.raw'], repeatable: true }, addedAuthorTitle: { operator: 'match', field: ['addedAuthorTitle.raw', 'parallelAddedAuthorTitle.raw'], repeatable: true }, - series: { operator: 'match', field: ['series', 'parallelSeries'], repeatable: true }, + series: { operator: 'match', field: ['series', 'parallelSeries', 'seriesUniformTitle', 'parallelSeriesUniformTitle', 'seriesAddedEntry', 'parallelSeriesAddedEntry'], repeatable: true }, placeOfPublication: { operator: 'match', field: ['placeOfPublication', 'parallelPlaceOfPublication'], repeatable: true }, dateFrom: { operator: 'custom', diff --git a/lib/elasticsearch/elastic-query-builder.js b/lib/elasticsearch/elastic-query-builder.js index d5e0d1b0..29b7a05e 100644 --- a/lib/elasticsearch/elastic-query-builder.js +++ b/lib/elasticsearch/elastic-query-builder.js @@ -30,6 +30,12 @@ class ElasticQueryBuilder { case 'subject': this.buildSubjectQuery() break + case 'series': + this.buildSeriesQuery() + break + case 'genre': + this.buildGenreQuery() + break case 'standard_number': this.buildStandardNumberQuery() break @@ -163,6 +169,36 @@ class ElasticQueryBuilder { this.boostSubjectMatches() } + /** + * Build ES query for 'series' searches + */ + buildSeriesQuery () { + if (!this.request.hasSearch()) return + + // Require a basic multi-match on series fields: + this.requireMultiMatch(SEARCH_SCOPES.series.fields) + + // Apply common boosts: + this.boostNyplOwned() + this.boostOnSite() + this.boostPopular() + } + + /** + * Build ES query for 'genre' searches + */ + buildGenreQuery () { + if (!this.request.hasSearch()) return + + // Require a basic multi-match on genre fields: + this.requireMultiMatch(SEARCH_SCOPES.genre.fields) + + // Apply common boosts: + this.boostNyplOwned() + this.boostOnSite() + this.boostPopular() + } + applySubjectPrefix () { const q = this.request.params.subject_prefix this.query.addMust({ diff --git a/package.json b/package.json index 48803e25..d58cc938 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,6 @@ }, "scripts": { "test": "./node_modules/.bin/standard --env mocha && NODE_ENV=test ./node_modules/.bin/mocha test --exit", - "test-integration": "./node_modules/.bin/mocha test/integration", "start": "node server.js", "deploy-development": "git checkout development && git pull origin development && eb deploy discovery-api-dev --profile nypl-sandbox", "deploy-qa": "git checkout qa && git pull origin qa && eb deploy discovery-api-qa --profile nypl-digital-dev", diff --git a/test/api-request.test.js b/test/api-request.test.js index 9a1eca68..d1bfdc38 100644 --- a/test/api-request.test.js +++ b/test/api-request.test.js @@ -31,7 +31,7 @@ describe('ApiRequest', function () { request = ApiRequest.fromParams({ q: '"toast"' }) expect(request.queryIsFullyQuoted()).to.eq(true) - // Test smart-quotes, the great scorge + // Test smart-quotes, the great scourge request = ApiRequest.fromParams({ q: '“toast“' }) expect(request.queryIsFullyQuoted()).to.eq(true) @@ -54,5 +54,8 @@ describe('ApiRequest', function () { request = ApiRequest.fromParams({ title: '"toast"', foo: 'bar' }) expect(request.advancedSearchParamsThatAreAlsoScopes()).to.deep.eq(['title']) + + request = ApiRequest.fromParams({ series: '"Greek volumes"', foo: 'bar' }) + expect(request.advancedSearchParamsThatAreAlsoScopes()).to.deep.eq(['series']) }) }) diff --git a/test/elastic-query-builder.test.js b/test/elastic-query-builder.test.js index 062a9ec0..bced9fd8 100644 --- a/test/elastic-query-builder.test.js +++ b/test/elastic-query-builder.test.js @@ -216,6 +216,42 @@ describe('ElasticQueryBuilder', () => { }) }) + describe('search_scope genre', () => { + it('generates a "genre" query', () => { + const request = new ApiRequest({ q: 'toast', search_scope: 'genre' }) + const inst = ElasticQueryBuilder.forApiRequest(request) + + // Expect a multi_match on term: + expect(inst.query.toJson()).to.nested + .include({ 'bool.must[0].multi_match.type': 'cross_fields' }) + .include({ 'bool.must[0].multi_match.query': 'toast' }) + .include({ 'bool.must[0].multi_match.fields[0]': 'genreForm^2' }) + + // Expect only common boosting clauses + expect(inst.query.toJson().bool.should) + .to.be.a('array') + .have.lengthOf(4) + }) + }) + + describe('search_scope series', () => { + it('generates a "series" query', () => { + const request = new ApiRequest({ q: 'toast', search_scope: 'series' }) + const inst = ElasticQueryBuilder.forApiRequest(request) + + // Expect a multi_match on term: + expect(inst.query.toJson()).to.nested + .include({ 'bool.must[0].multi_match.type': 'cross_fields' }) + .include({ 'bool.must[0].multi_match.query': 'toast' }) + .include({ 'bool.must[0].multi_match.fields[0]': 'series^2' }) + + // Expect only common boosting clauses + expect(inst.query.toJson().bool.should) + .to.be.a('array') + .have.lengthOf(4) + }) + }) + describe('multiple id query', () => { it('supports ids=x,y,z', () => { const request = new ApiRequest({ q: '', ids: ['id_a', 'id_b'] }) @@ -371,6 +407,43 @@ describe('ElasticQueryBuilder', () => { }) }) + describe('series=', () => { + it('applies series clauses to query', () => { + const request = new ApiRequest({ series: 'toast' }) + const inst = ElasticQueryBuilder.forApiRequest(request) + + const query = inst.query.toJson() + + // Assert there's a multi-match: + expect(query).to.nested + .include({ + // Multi-match on series fields: + 'bool.must[0].bool.must[0].multi_match.fields[0]': 'series^2', + 'bool.must[0].bool.must[0].multi_match.fields[4]': 'seriesUniformTitle', + 'bool.must[0].bool.must[0].multi_match.fields[7]': 'seriesAddedEntry', + 'bool.must[0].bool.must[0].multi_match.query': 'toast' + }) + }) + }) + + describe('genre=', () => { + it('applies genre clauses to query', () => { + const request = new ApiRequest({ genre: 'toast' }) + const inst = ElasticQueryBuilder.forApiRequest(request) + + const query = inst.query.toJson() + + // Assert there's a multi-match: + expect(query).to.nested + .include({ + // Multi-match on genre fields: + 'bool.must[0].bool.must[0].multi_match.fields[0]': 'genreForm^2', + 'bool.must[0].bool.must[0].multi_match.fields[1]': 'genreForm.folded', + 'bool.must[0].bool.must[0].multi_match.query': 'toast' + }) + }) + }) + describe('subject_prefix=', () => { it('applies subject_prefix clauses to query', () => { const request = new ApiRequest({ subject_prefix: 'toast' }) @@ -517,7 +590,7 @@ describe('ElasticQueryBuilder', () => { } }) - // Asset filter clauses: + // Assert filter clauses: expect(query).to.nested.include({ 'bool.filter[0].bool.should[0].nested.path': 'dates', 'bool.filter[0].bool.should[0].nested.query.range.dates\\.range.gte': '2020', @@ -545,7 +618,7 @@ describe('ElasticQueryBuilder', () => { const query = inst.query.toJson() - // Asset filter clauses: + // Assert filter clauses: expect(query).to.nested.include({ 'bool.filter[0].bool.should[0].nested.path': 'dates', 'bool.filter[0].bool.should[0].nested.query.range.dates\\.range.gte': '2020-12-31', @@ -568,7 +641,7 @@ describe('ElasticQueryBuilder', () => { const query = inst.query.toJson() - // Asset filter clauses: + // Assert filter clauses: expect(query).to.nested.include({ 'bool.filter[0].bool.should[0].nested.path': 'dates', 'bool.filter[0].bool.should[0].nested.query.range.dates\\.range.gte': '2020-12-31', @@ -591,7 +664,7 @@ describe('ElasticQueryBuilder', () => { const query = inst.query.toJson() - // Asset filter clauses: + // Assert filter clauses: expect(query).to.nested.include({ 'bool.filter[0].bool.should[0].nested.path': 'dates', 'bool.filter[0].bool.should[0].nested.query.range.dates\\.range.gte': '2020-01', @@ -614,7 +687,7 @@ describe('ElasticQueryBuilder', () => { const query = inst.query.toJson() - // Asset filter clauses: + // Assert filter clauses: expect(query).to.nested.include({ 'bool.filter[0].bool.should[0].nested.path': 'dates', 'bool.filter[0].bool.should[0].nested.query.range.dates\\.range.gte': '2020-01', @@ -637,7 +710,7 @@ describe('ElasticQueryBuilder', () => { const query = inst.query.toJson() - // Asset filter clauses: + // Assert filter clauses: expect(query).to.nested.include({ 'bool.filter[0].bool.should[0].nested.path': 'dates', 'bool.filter[0].bool.should[0].nested.query.range.dates\\.range.gte': '2020-01',