Skip to content

Commit

Permalink
GAP-2509: Defend against timeout when publishing (#256)
Browse files Browse the repository at this point in the history
* GAP-2509: Only publish/unpublish to contentful when needed

* GAP-2509: Fix tests

* GAP-2509: Refetching data as updating rich text webclient call does not include sys fields

* GAP-2509: Fixing blank ID error
  • Loading branch information
dominicwest authored Mar 21, 2024
1 parent a4c21d7 commit 6f6e8f6
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@
import com.contentful.java.cda.CDAClient;
import com.contentful.java.cda.CDAEntry;
import com.contentful.java.cda.QueryOperation;
import com.contentful.java.cma.CMACallback;
import com.contentful.java.cma.CMAClient;
import com.contentful.java.cma.model.CMAEntry;
import com.contentful.java.cma.model.rich.CMARichDocument;
import gov.cabinetoffice.gap.adminbackend.config.ContentfulConfigProperties;
import gov.cabinetoffice.gap.adminbackend.config.FeatureFlagsConfigurationProperties;
import gov.cabinetoffice.gap.adminbackend.dtos.grantadvert.*;
import gov.cabinetoffice.gap.adminbackend.dtos.grantadvert.GetGrantAdvertPageResponseDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.grantadvert.GetGrantAdvertPublishingInformationResponseDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.grantadvert.GetGrantAdvertStatusResponseDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.grantadvert.GrantAdvertPageResponseValidationDto;
import gov.cabinetoffice.gap.adminbackend.entities.GrantAdmin;
import gov.cabinetoffice.gap.adminbackend.entities.GrantAdvert;
import gov.cabinetoffice.gap.adminbackend.entities.SchemeEntity;
Expand Down Expand Up @@ -280,12 +284,10 @@ public void deleteGrantAdvert(UUID grantAdvertId) {
throw new NotFoundException("Grant Advert not found with id of " + grantAdvertId);
}

@Transactional
public GrantAdvert publishAdvert(UUID advertId) {
final GrantAdvert advert = getAdvertById(advertId);

CMAEntry contentfulAdvert;

// if advert has not been published previously
if (advert.getFirstPublishedDate() == null) {
contentfulAdvert = createAdvertInContentful(advert);
Expand All @@ -296,24 +298,32 @@ public GrantAdvert publishAdvert(UUID advertId) {
advert.setLastPublishedDate(Instant.now());
}

contentfulAdvert = contentfulManagementClient.entries().publish(contentfulAdvert);
openSearchService.indexEntry(contentfulAdvert);

contentfulAdvert = contentfulManagementClient.entries().fetchOne(contentfulAdvert.getId());
advert.setStatus(GrantAdvertStatus.PUBLISHED);
advert.setContentfulSlug(contentfulAdvert.getField("label", CONTENTFUL_LOCALE));
advert.setContentfulEntryId(contentfulAdvert.getId());
updateGrantAdvertApplicationDates(advert);

if (Boolean.FALSE.equals(contentfulAdvert.isPublished())) {
contentfulManagementClient.entries().async().publish(contentfulAdvert, new CMACallback<>() {
@Override
protected void onSuccess(CMAEntry result) {
openSearchService.indexEntry(result);
}
});
}

updateGrantAdvertApplicationDates(advert);
return save(advert);
}

public void unpublishAdvert(UUID advertId) {
final GrantAdvert advert = this.getAdvertById(advertId);
final CMAEntry contentfulAdvert = contentfulManagementClient.entries().fetchOne(advert.getContentfulEntryId());

CMAEntry contentfulAdvert = contentfulManagementClient.entries().fetchOne(advert.getContentfulEntryId());

contentfulAdvert = contentfulManagementClient.entries().unPublish(contentfulAdvert);
openSearchService.removeIndexEntry(contentfulAdvert);
if (Boolean.TRUE.equals(contentfulAdvert.isPublished())) {
final CMAEntry unpublishedAd = contentfulManagementClient.entries().unPublish(contentfulAdvert);
openSearchService.removeIndexEntry(unpublishedAd);
}

advert.setStatus(GrantAdvertStatus.DRAFT);
advert.setUnpublishedDate(Instant.now());
Expand All @@ -330,19 +340,10 @@ private CMAEntry createAdvertInContentful(final GrantAdvert grantAdvert) {
contentfulAdvert.setField("grantName", CONTENTFUL_LOCALE, grantAdvert.getGrantAdvertName());
contentfulAdvert.setField("label", CONTENTFUL_LOCALE, generateUniqueSlug(grantAdvert));

final CMAEntry createdAAdvert = contentfulManagementClient.entries().create(CONTENTFUL_GRANT_TYPE_ID,
final CMAEntry createdAdvert = contentfulManagementClient.entries().create(CONTENTFUL_GRANT_TYPE_ID,
contentfulAdvert);
createRichTextQuestionsInContentful(grantAdvert, createdAAdvert);

/*
* hate this but because we create a new advert and then immediately update it the
* version number in contentful is bumped up, so we need to refresh the data to
* prevent errors when publishing the advert :(.
*
* Absolutely begging to be rate limited by getting this loose with the number of
* requests to their API.
*/
return contentfulManagementClient.entries().fetchOne(createdAAdvert.getId());
createRichTextQuestionsInContentful(grantAdvert, createdAdvert);
return createdAdvert;
}

private CMAEntry updateAdvertInContentful(final GrantAdvert grantAdvert) {
Expand All @@ -358,8 +359,7 @@ private CMAEntry updateAdvertInContentful(final GrantAdvert grantAdvert) {

final CMAEntry updatedAdvert = contentfulManagementClient.entries().update(contentfulAdvert);
createRichTextQuestionsInContentful(grantAdvert, updatedAdvert);

return contentfulManagementClient.entries().fetchOne(updatedAdvert.getId());
return updatedAdvert;
}

private String generateUniqueSlug(final GrantAdvert grantAdvert) {
Expand Down Expand Up @@ -414,39 +414,36 @@ private void addFieldToContentfulAdvert(final CMAEntry contentfulAdvert,
contentfulAdvert.setField(questionResponse.getId(), CONTENTFUL_LOCALE, contentfulValue);
}

private void createRichTextQuestionsInContentful(final GrantAdvert advert, final CMAEntry contentfulAdvert) {
private CMAEntry createRichTextQuestionsInContentful(final GrantAdvert advert, final CMAEntry contentfulAdvert) {
final List<GrantAdvertQuestionResponse> responses = getRichTextResponses(advert);
final String requestBody = buildRichTextPatchRequestBody(responses);
final String contentfulUrl = String.format("https://api.contentful.com/spaces/%1$s/environments/%2$s/entries/%3$s",
contentfulProperties.getSpaceId(), contentfulProperties.getEnvironmentId(),
contentfulAdvert.getId());

return webClientBuilder.build()
.patch()
.uri(contentfulUrl)
.headers(h -> {
h.set("Authorization", String.format("Bearer %s", contentfulProperties.getAccessToken()));
h.set("Content-Type", "application/json-patch+json");
h.set("X-Contentful-Version", contentfulAdvert.getVersion().toString());
})
.bodyValue(requestBody)
.retrieve()
.bodyToMono(CMAEntry.class)
.doOnError(exception -> log.error("createRichTextQuestionsInContentful failed on PATCH to {}, with message: {}", contentfulUrl, exception.getMessage()))
.block();
}

// get all the rich text responses
final List<GrantAdvertQuestionResponse> responses = advert.getResponse().getSections().stream()
private List<GrantAdvertQuestionResponse> getRichTextResponses(final GrantAdvert advert) {
return advert.getResponse().getSections().stream()
.flatMap(s -> s.getPages().stream()).flatMap(p -> p.getQuestions().stream())
.filter(q -> advertDefinition.getSections().stream().flatMap(s -> s.getPages().stream())
.flatMap(p -> p.getQuestions().stream())
.filter(qr -> qr.getResponseType().equals(AdvertDefinitionQuestionResponseType.RICH_TEXT))
.anyMatch(qr -> qr.getId().equals(q.getId())))
.toList();

if (!responses.isEmpty()) {
final String requestBody = buildRichTextPatchRequestBody(responses);

// send to contentful
final String url = String.format("https://api.contentful.com/spaces/%1$s/environments/%2$s/entries/%3$s",
contentfulProperties.getSpaceId(), contentfulProperties.getEnvironmentId(),
contentfulAdvert.getId());
log.debug(url);
webClientBuilder.build()
.patch()
.uri(url)
.headers(h -> {
h.set("Authorization", String.format("Bearer %s", contentfulProperties.getAccessToken()));
h.set("Content-Type", "application/json-patch+json");
h.set("X-Contentful-Version", contentfulAdvert.getVersion().toString());
})
.bodyValue(requestBody)
.retrieve()
.bodyToMono(Void.class)
.doOnError(exception -> log.error("createRichTextQuestionsInContentful failed on PATCH to {}, with message: {}", url, exception.getMessage()))
.block();
}
}

// built to replicate
Expand Down Expand Up @@ -570,7 +567,6 @@ private void updateGrantAdvertApplicationDates(final GrantAdvert grantAdvert) {
// set dates on advert
grantAdvert.setOpeningDate(openingDateInstant);
grantAdvert.setClosingDate(closingDateInstant);

}

public void unscheduleGrantAdvert(final UUID advertId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ class GrantAdvertServiceTest {
@Mock
private ModuleEntries contentfulEntries;

@Mock
private ModuleEntries.Async async;

@Mock
private FetchQuery mockedFetchQuery;

Expand Down Expand Up @@ -818,7 +821,7 @@ void publishAdvert_successfullyPublishedAdvert() {

when(contentfulEntries.fetchOne(contentfulAdvertId)).thenReturn(publishedContentfulAdvert);

when(contentfulEntries.publish(publishedContentfulAdvert)).thenReturn(publishedContentfulAdvert);
when(contentfulEntries.async()).thenReturn(async);

doReturn(mockGrantAdvert).when(grantAdvertService).save(any());

Expand All @@ -833,8 +836,7 @@ void publishAdvert_successfullyPublishedAdvert() {
when(requestBodyUriSpec.headers(any())).thenReturn(requestBodyUriSpec);
when(requestBodyUriSpec.bodyValue(any())).thenReturn(requestHeadersSpec);
when(requestHeadersSpec.retrieve()).thenReturn(responseSpec);
when(responseSpec.bodyToMono(Void.class)).thenReturn(Mono.empty());

when(responseSpec.bodyToMono(CMAEntry.class)).thenReturn(Mono.just(publishedContentfulAdvert));

final ArgumentCaptor<CMAEntry> entryCaptor = ArgumentCaptor.forClass(CMAEntry.class);

Expand All @@ -844,8 +846,6 @@ void publishAdvert_successfullyPublishedAdvert() {

verify(grantAdvertService).save(grantAdvertArgumentCaptor.capture());

verify(openSearchService).indexEntry(publishedContentfulAdvert);

GrantAdvert savedAdvert = grantAdvertArgumentCaptor.getValue();

assertThat(savedAdvert.getFirstPublishedDate()).isNotNull();
Expand All @@ -867,13 +867,13 @@ void publishAdvert_successfullyPublishedAdvert() {
verify(requestBodyUriSpec).headers(any());
verify(requestBodyUriSpec).bodyValue(any());
verify(requestHeadersSpec).retrieve();
verify(responseSpec).bodyToMono(Void.class);
verify(responseSpec).bodyToMono(CMAEntry.class);

// verify that we've refreshed the data after adding RTF data
verify(contentfulEntries).fetchOne(contentfulAdvertId);

// verify that we've published
verify(contentfulEntries).publish(publishedContentfulAdvert);
verify(async).publish(eq(publishedContentfulAdvert), any());
}

@Test
Expand All @@ -898,7 +898,7 @@ void publishAdvert_updatesExistingAdvert_IfFirstPublishedDateHasBeenSet() {
when(contentfulEntries.update(Mockito.any())).thenReturn(publishedContentfulAdvert);
when(contentfulEntries.fetchOne(contentfulAdvertId)).thenReturn(publishedContentfulAdvert,
publishedContentfulAdvert);
when(contentfulEntries.publish(publishedContentfulAdvert)).thenReturn(publishedContentfulAdvert);
when(contentfulEntries.async()).thenReturn(async);
doReturn(grantAvertInDatabase).when(grantAdvertService).save(any());

final WebClient webClient = mock(WebClient.class);
Expand All @@ -912,14 +912,13 @@ void publishAdvert_updatesExistingAdvert_IfFirstPublishedDateHasBeenSet() {
when(requestBodyUriSpec.headers(any())).thenReturn(requestBodyUriSpec);
when(requestBodyUriSpec.bodyValue(any())).thenReturn(requestHeadersSpec);
when(requestHeadersSpec.retrieve()).thenReturn(responseSpec);
when(responseSpec.bodyToMono(Void.class)).thenReturn(Mono.empty());
when(responseSpec.bodyToMono(CMAEntry.class)).thenReturn(Mono.just(publishedContentfulAdvert));

final ArgumentCaptor<GrantAdvert> grantAdvertArgumentCaptor = ArgumentCaptor.forClass(GrantAdvert.class);

grantAdvertService.publishAdvert(grantAdvertId);

verify(grantAdvertService).save(grantAdvertArgumentCaptor.capture());
verify(openSearchService).indexEntry(publishedContentfulAdvert);

final GrantAdvert savedAdvert = grantAdvertArgumentCaptor.getValue();

Expand All @@ -932,15 +931,15 @@ void publishAdvert_updatesExistingAdvert_IfFirstPublishedDateHasBeenSet() {
verify(requestBodyUriSpec).headers(any());
verify(requestBodyUriSpec).bodyValue(any());
verify(requestHeadersSpec).retrieve();
verify(responseSpec).bodyToMono(Void.class);
verify(responseSpec).bodyToMono(CMAEntry.class);

verify(contentfulEntries).update(publishedContentfulAdvert);

// verify that we've refreshed the data after adding RTF data
verify(contentfulEntries, atLeastOnce()).fetchOne(contentfulAdvertId);

// verify that we've published
verify(contentfulEntries).publish(publishedContentfulAdvert);
verify(async).publish(eq(publishedContentfulAdvert), any());
}

@Test
Expand All @@ -963,7 +962,7 @@ void publishAdvertThroughLambda_successfullyPublishedAdvert() {
when(requestBodyUriSpec.headers(any())).thenReturn(requestBodyUriSpec);
when(requestBodyUriSpec.bodyValue(any())).thenReturn(requestHeadersSpec);
when(requestHeadersSpec.retrieve()).thenReturn(responseSpec);
when(responseSpec.bodyToMono(Void.class)).thenReturn(Mono.empty());
when(responseSpec.bodyToMono(CMAEntry.class)).thenReturn(Mono.just(publishedContentfulAdvert));
doReturn(mockGrantAdvert).when(grantAdvertService).save(any());

when(advertDefinition.getSections()).thenReturn(definition.getSections());
Expand All @@ -985,7 +984,7 @@ void publishAdvertThroughLambda_successfullyPublishedAdvert() {

when(contentfulEntries.fetchOne(contentfulAdvertId)).thenReturn(publishedContentfulAdvert);

when(contentfulEntries.publish(publishedContentfulAdvert)).thenReturn(publishedContentfulAdvert);
when(contentfulEntries.async()).thenReturn(async);

final ArgumentCaptor<CMAEntry> entryCaptor = ArgumentCaptor.forClass(CMAEntry.class);

Expand All @@ -994,7 +993,6 @@ void publishAdvertThroughLambda_successfullyPublishedAdvert() {
grantAdvertService.publishAdvert(grantAdvertId);

verify(grantAdvertService).save(grantAdvertArgumentCaptor.capture());
verify(openSearchService).indexEntry(publishedContentfulAdvert);

GrantAdvert savedAdvert = grantAdvertArgumentCaptor.getValue();

Expand All @@ -1017,13 +1015,13 @@ void publishAdvertThroughLambda_successfullyPublishedAdvert() {
verify(requestBodyUriSpec).headers(any());
verify(requestBodyUriSpec).bodyValue(any());
verify(requestHeadersSpec).retrieve();
verify(responseSpec).bodyToMono(Void.class);
verify(responseSpec).bodyToMono(CMAEntry.class);

// verify that we've refreshed the data after adding RTF data
verify(contentfulEntries).fetchOne(contentfulAdvertId);

// verify that we've published
verify(contentfulEntries).publish(publishedContentfulAdvert);
verify(async).publish(eq(publishedContentfulAdvert), any());
}

}
Expand All @@ -1038,8 +1036,8 @@ class unpublishAdvert {
final GrantAdvert grantAdvert = RandomGrantAdvertGenerators.randomGrantAdvertEntity().id(grantAdvertId)
.contentfulEntryId(contentfulAdvertId).build();

@Mock
final CMAEntry contentfulAdvert = new CMAEntry().setId(contentfulAdvertId).setVersion(2);

@Test
@WithAdminSession
void unpublishAdvert_UnpublishesAdvertFromContentful_AndSetsStatusToDraftInDb() {
Expand All @@ -1053,6 +1051,7 @@ void unpublishAdvert_UnpublishesAdvertFromContentful_AndSetsStatusToDraftInDb()
when(contentfulEntries.fetchOne(contentfulAdvertId)).thenReturn(contentfulAdvert);
when(contentfulEntries.unPublish(contentfulAdvert)).thenReturn(contentfulAdvert);
doReturn(advert).when(grantAdvertService).save(any());
when(contentfulAdvert.isPublished()).thenReturn(true);

final ArgumentCaptor<GrantAdvert> advertCaptor = ArgumentCaptor.forClass(GrantAdvert.class);

Expand Down Expand Up @@ -1082,6 +1081,7 @@ void unpublishAdvertThroughLambda_successfullyUnpublishedAdvert() {
when(contentfulEntries.fetchOne(contentfulAdvertId)).thenReturn(contentfulAdvert);
when(contentfulEntries.unPublish(contentfulAdvert)).thenReturn(contentfulAdvert);
doReturn(advert).when(grantAdvertService).save(any());
when(contentfulAdvert.isPublished()).thenReturn(true);

final ArgumentCaptor<GrantAdvert> advertCaptor = ArgumentCaptor.forClass(GrantAdvert.class);

Expand Down

0 comments on commit 6f6e8f6

Please sign in to comment.