Skip to content

Commit 858a074

Browse files
php-coderphp-coder
committed
feat: a request that failed to download a file, can be retried.
Fix #1254
1 parent cda4468 commit 858a074

File tree

15 files changed

+231
-14
lines changed

15 files changed

+231
-14
lines changed

‎NEWS.txt

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
0.x (upcoming release)
2+
- (feature) a request that failed to download a file, can be retried
23
- (infrastructure) Migrate to Spring Boot 2
34

45
0.4.2

‎src/main/java/ru/mystamps/web/feature/series/importing/SeriesImportController.java

+21
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@
2929
import org.springframework.web.bind.annotation.ModelAttribute;
3030
import org.springframework.web.bind.annotation.PathVariable;
3131
import org.springframework.web.bind.annotation.PostMapping;
32+
import org.springframework.web.bind.annotation.RequestParam;
3233
import ru.mystamps.web.common.LocaleUtils;
3334
import ru.mystamps.web.feature.participant.EntityWithIdDto;
3435
import ru.mystamps.web.feature.participant.ParticipantService;
3536
import ru.mystamps.web.feature.series.CatalogUtils;
3637
import ru.mystamps.web.feature.series.SeriesController;
3738
import ru.mystamps.web.feature.series.SeriesUrl;
3839
import ru.mystamps.web.feature.series.importing.event.ImportRequestCreated;
40+
import ru.mystamps.web.feature.series.importing.event.RetryDownloading;
3941
import ru.mystamps.web.feature.series.importing.sale.SeriesSaleParsedDataDto;
4042
import ru.mystamps.web.feature.series.importing.sale.SeriesSalesImportService;
4143
import ru.mystamps.web.support.spring.security.CurrentUser;
@@ -96,6 +98,25 @@ public String processRequestImportForm(
9698

9799
return redirectTo(SeriesImportUrl.REQUEST_IMPORT_PAGE, requestId);
98100
}
101+
102+
// @todo #1254 Update workflow to mention RetryDownloading event
103+
@PostMapping(path = SeriesImportUrl.REQUEST_IMPORT_SERIES_PAGE, params = "requestId")
104+
public String rerunImport(
105+
@RequestParam("requestId") Integer requestId,
106+
HttpServletResponse response)
107+
throws IOException {
108+
109+
ImportRequestDto request = seriesImportService.findById(requestId);
110+
if (request == null) {
111+
response.sendError(HttpServletResponse.SC_NOT_FOUND);
112+
return null;
113+
}
114+
115+
RetryDownloading retryDownloading = new RetryDownloading(this, requestId);
116+
eventPublisher.publishEvent(retryDownloading);
117+
118+
return redirectTo(SeriesImportUrl.REQUEST_IMPORT_PAGE, requestId);
119+
}
99120

100121
@SuppressWarnings({ "PMD.ModifiedCyclomaticComplexity", "PMD.NPathComplexity" })
101122
@GetMapping(SeriesImportUrl.REQUEST_IMPORT_PAGE)

‎src/main/java/ru/mystamps/web/feature/series/importing/SeriesImportService.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Integer addSeries(
3535
);
3636
void changeStatus(Integer requestId, String oldStatus, String newStatus);
3737
ImportRequestDto findById(Integer requestId);
38-
void saveDownloadedContent(Integer requestId, String content);
38+
void saveDownloadedContent(Integer requestId, String content, boolean retry);
3939
String getDownloadedContent(Integer requestId);
4040
void saveParsedData(Integer requestId, SeriesExtractedInfo seriesInfo, String imageUrl);
4141
SeriesParsedDataDto getParsedData(Integer requestId, String lang);

‎src/main/java/ru/mystamps/web/feature/series/importing/SeriesImportServiceImpl.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public ImportRequestDto findById(Integer requestId) {
148148

149149
@Override
150150
@Transactional
151-
public void saveDownloadedContent(Integer requestId, String content) {
151+
public void saveDownloadedContent(Integer requestId, String content, boolean retry) {
152152
Validate.isTrue(requestId != null, "Request id must be non null");
153153
Validate.isTrue(StringUtils.isNotBlank(content), "Content must be non-blank");
154154

@@ -158,11 +158,11 @@ public void saveDownloadedContent(Integer requestId, String content) {
158158

159159
log.info("Request #{}: page were downloaded ({} characters)", requestId, content.length());
160160

161-
changeStatus(
162-
requestId,
163-
SeriesImportRequestStatus.UNPROCESSED,
164-
SeriesImportRequestStatus.DOWNLOADING_SUCCEEDED
165-
);
161+
String oldStatus = retry
162+
? SeriesImportRequestStatus.DOWNLOADING_FAILED
163+
: SeriesImportRequestStatus.UNPROCESSED;
164+
165+
changeStatus(requestId, oldStatus, SeriesImportRequestStatus.DOWNLOADING_SUCCEEDED);
166166
}
167167

168168
@Override

‎src/main/java/ru/mystamps/web/feature/series/importing/event/EventsConfig.java

+9
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,15 @@ public ApplicationListener<ImportRequestCreated> importRequestCreatedEventListen
8484
);
8585
}
8686

87+
@Bean
88+
public ApplicationListener<RetryDownloading> retryDownloadingEventListener() {
89+
return new RetryDownloadingEventListener(
90+
servicesConfig.getSeriesDownloaderService(),
91+
seriesImportService,
92+
eventPublisher
93+
);
94+
}
95+
8796
@Bean
8897
public ApplicationListener<DownloadingSucceeded> downloadingSucceededEventListener(
8998
SiteParserService siteParserService,

‎src/main/java/ru/mystamps/web/feature/series/importing/event/ImportRequestCreatedEventListener.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public void onApplicationEvent(ImportRequestCreated event) {
6666
return;
6767
}
6868

69-
seriesImportService.saveDownloadedContent(requestId, result.getDataAsString());
69+
seriesImportService.saveDownloadedContent(requestId, result.getDataAsString(), false);
7070

7171
eventPublisher.publishEvent(new DownloadingSucceeded(this, requestId, url));
7272
}

‎src/main/java/ru/mystamps/web/feature/series/importing/event/RetryDownloading.java

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright (C) 2009-2020 Slava Semushin <[email protected]>
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU General Public License as published by
6+
* the Free Software Foundation; either version 2 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License
15+
* along with this program; if not, write to the Free Software
16+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
17+
*/
18+
package ru.mystamps.web.feature.series.importing.event;
19+
20+
import lombok.Getter;
21+
import org.springframework.context.ApplicationEvent;
22+
23+
/**
24+
* Event occurs when download has failed and user requested to re-try.
25+
*/
26+
@Getter
27+
public class RetryDownloading extends ApplicationEvent {
28+
private final Integer requestId;
29+
30+
public RetryDownloading(Object source, Integer requestId) {
31+
super(source);
32+
this.requestId = requestId;
33+
}
34+
35+
}

‎src/main/java/ru/mystamps/web/feature/series/importing/event/RetryDownloadingEventListener.java

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* Copyright (C) 2009-2020 Slava Semushin <[email protected]>
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU General Public License as published by
6+
* the Free Software Foundation; either version 2 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License
15+
* along with this program; if not, write to the Free Software
16+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
17+
*/
18+
package ru.mystamps.web.feature.series.importing.event;
19+
20+
import lombok.RequiredArgsConstructor;
21+
import org.apache.commons.lang3.Validate;
22+
import org.slf4j.Logger;
23+
import org.slf4j.LoggerFactory;
24+
import org.springframework.context.ApplicationEventPublisher;
25+
import org.springframework.context.ApplicationListener;
26+
import ru.mystamps.web.feature.series.DownloadResult;
27+
import ru.mystamps.web.feature.series.DownloaderService;
28+
import ru.mystamps.web.feature.series.importing.ImportRequestDto;
29+
import ru.mystamps.web.feature.series.importing.SeriesImportDb.SeriesImportRequestStatus;
30+
import ru.mystamps.web.feature.series.importing.SeriesImportService;
31+
32+
/**
33+
* Listener of the {@link RetryDownloading} event.
34+
*
35+
* Downloads a file, saves it to database and publish the {@link DownloadingSucceeded} event.
36+
*
37+
* It is similar to {@link ImportRequestCreatedEventListener} with the following differences:
38+
* - it loads a request from database as we have only id
39+
* - it doesn't modify a request state when downloading fails
40+
* - it invokes {@code saveDownloadedContent()} with retry=true parameter
41+
*
42+
* @see ImportRequestCreatedEventListener
43+
* @see DownloadingSucceededEventListener
44+
*/
45+
@RequiredArgsConstructor
46+
public class RetryDownloadingEventListener implements ApplicationListener<RetryDownloading> {
47+
48+
private static final Logger LOG = LoggerFactory.getLogger(RetryDownloadingEventListener.class);
49+
50+
private final DownloaderService downloaderService;
51+
private final SeriesImportService seriesImportService;
52+
private final ApplicationEventPublisher eventPublisher;
53+
54+
@Override
55+
public void onApplicationEvent(RetryDownloading event) {
56+
Integer requestId = event.getRequestId();
57+
58+
ImportRequestDto request = seriesImportService.findById(requestId);
59+
if (request == null) {
60+
// FIXME: how to handle error? maybe publish UnexpectedErrorEvent?
61+
LOG.error("Request #{}: couldn't retry is it doesn't exist", requestId);
62+
return;
63+
}
64+
65+
String status = request.getStatus();
66+
if (SeriesImportRequestStatus.DOWNLOADING_FAILED.equals(status)) {
67+
LOG.warn("Request #{}: unexpected status '{}'. Abort a retry process", request, status);
68+
return;
69+
}
70+
71+
String url = request.getUrl();
72+
LOG.info("Request #{}: retry downloading '{}'", requestId, url);
73+
74+
DownloadResult result = downloaderService.download(url);
75+
if (result.hasFailed()) {
76+
LOG.info(
77+
"Request #{}: downloading of '{}' failed again: {}",
78+
requestId,
79+
url,
80+
result.getCode()
81+
);
82+
83+
// in case of failure we don't need to change request status as we assume that
84+
// it's already set to DownloadingFailed
85+
return;
86+
}
87+
88+
// FIXME: do we need updated_by field?
89+
seriesImportService.saveDownloadedContent(requestId, result.getDataAsString(), true);
90+
91+
eventPublisher.publishEvent(new DownloadingSucceeded(this, requestId, url));
92+
}
93+
94+
}

‎src/main/resources/liquibase/version/0.4.2.xml

+1
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,6 @@
77

88
<include file="0.4.2/2019-11-17--drop_unique_series_from_collection.xml" relativeToChangelogFile="true" />
99
<include file="0.4.2/2019-11-27--add_collections_series_id.xml" relativeToChangelogFile="true" />
10+
<include file="0.4.2/2020-02-11--downloading_failed_import_requests.xml" relativeToChangelogFile="true" />
1011

1112
</databaseChangeLog>

‎src/main/resources/liquibase/version/0.4.2/2020-02-11--downloading_failed_import_requests.xml

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<databaseChangeLog
3+
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
4+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
5+
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
6+
http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.1.xsd">
7+
8+
<changeSet id="add-failed-import-requests" author="php-coder" context="test-data">
9+
10+
<insert tableName="series_import_requests">
11+
<column name="url" value="http://127.0.0.1:8080/series/1?lang=en" />
12+
<column name="status_id" valueComputed="(SELECT id FROM series_import_request_statuses WHERE name = 'DownloadingFailed')" />
13+
<column name="requested_at" valueComputed="${NOW}" />
14+
<column name="requested_by" valueComputed="(SELECT id FROM users WHERE role = 'ADMIN' ORDER by id LIMIT 1)" />
15+
<column name="updated_at" valueComputed="${NOW}" />
16+
</insert>
17+
18+
</changeSet>
19+
20+
</databaseChangeLog>

‎src/main/resources/ru/mystamps/i18n/Messages.properties

+1
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ t_submit_request = Submit request
211211
t_import_request = import request
212212
t_status = Status
213213
t_imported_series = Imported series
214+
t_retry = Retry
214215
t_gathered_data = Gathered data
215216
t_import = Import
216217
t_seller_url = Seller URL

‎src/main/resources/ru/mystamps/i18n/Messages_ru.properties

+1
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ t_submit_request = Создать запрос
210210
t_import_request = запрос на импорт
211211
t_status = Статус
212212
t_imported_series = Импортированная серия
213+
t_retry = Повторить
213214
t_gathered_data = Собранные данные
214215
t_import = Импортировать
215216
t_seller_url = Страница продавца

‎src/main/webapp/WEB-INF/views/series/import/info.html

+10-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,16 @@ <h3 th:text="${#strings.capitalize(header)}">
9090
</dd>
9191
</dl>
9292
</div>
93-
93+
94+
<!--/*/
95+
<div class="col-sm-1 col-sm-offset-6" th:if="${request.status == 'DownloadingFailed'}">
96+
<form id="retry-import-series-form" method="post" th:action="@{${REQUEST_IMPORT_SERIES_PAGE}}">
97+
<input type="hidden" name="requestId" th:value="${id}" />
98+
<input type="submit" class="btn btn-primary btn-sm" value="Retry" th:value="#{t_retry}" />
99+
</form>
100+
</div>
101+
/*/-->
102+
94103
<div class="col-sm-4 col-sm-offset-4" th:if="${showForm}" th:with="disabled=${request.seriesId != null}">
95104
<h3 th:text="#{t_gathered_data}">
96105
Gathered data

‎src/test/groovy/ru/mystamps/web/feature/series/importing/SeriesImportServiceImplTest.groovy

+22-5
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import ru.mystamps.web.tests.Random
3535
import spock.lang.Specification
3636
import spock.lang.Unroll
3737

38+
import static io.qala.datagen.RandomShortApi.bool
3839
import static io.qala.datagen.RandomShortApi.english
3940
import static io.qala.datagen.RandomShortApi.nullOr
4041
import static io.qala.datagen.RandomShortApi.nullOrBlank
@@ -365,7 +366,7 @@ class SeriesImportServiceImplTest extends Specification {
365366
given:
366367
String content = between(1, 10).english()
367368
when:
368-
service.saveDownloadedContent(null, content)
369+
service.saveDownloadedContent(null, content, bool())
369370
then:
370371
IllegalArgumentException ex = thrown()
371372
ex.message == 'Request id must be non null'
@@ -374,7 +375,7 @@ class SeriesImportServiceImplTest extends Specification {
374375
@Unroll
375376
def "saveDownloadedContent() should throw exception when content is '#content'"(String content) {
376377
when:
377-
service.saveDownloadedContent(Random.id(), content)
378+
service.saveDownloadedContent(Random.id(), content, bool())
378379
then:
379380
IllegalArgumentException ex = thrown()
380381
ex.message == 'Content must be non-blank'
@@ -390,7 +391,7 @@ class SeriesImportServiceImplTest extends Specification {
390391
Integer expectedRequestId = Random.id()
391392
String expectedContent = between(1, 10).english()
392393
when:
393-
service.saveDownloadedContent(expectedRequestId, expectedContent)
394+
service.saveDownloadedContent(expectedRequestId, expectedContent, bool())
394395
then:
395396
1 * seriesImportDao.addRawContent(
396397
expectedRequestId,
@@ -407,11 +408,11 @@ class SeriesImportServiceImplTest extends Specification {
407408
}
408409

409410
@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
410-
def 'saveDownloadedContent() should change status'() {
411+
def 'saveDownloadedContent() should change status for new request'() {
411412
given:
412413
Integer expectedRequestId = Random.id()
413414
when:
414-
service.saveDownloadedContent(expectedRequestId, between(1, 10).english())
415+
service.saveDownloadedContent(expectedRequestId, between(1, 10).english(), false)
415416
then:
416417
1 * seriesImportDao.changeStatus({ UpdateImportRequestStatusDbDto status ->
417418
assert status?.requestId == expectedRequestId
@@ -422,6 +423,22 @@ class SeriesImportServiceImplTest extends Specification {
422423
})
423424
}
424425

426+
@SuppressWarnings(['ClosureAsLastMethodParameter', 'UnnecessaryReturnKeyword'])
427+
def 'saveDownloadedContent() should change status for failed request'() {
428+
given:
429+
Integer expectedRequestId = Random.id()
430+
when:
431+
service.saveDownloadedContent(expectedRequestId, between(1, 10).english(), true)
432+
then:
433+
1 * seriesImportDao.changeStatus({ UpdateImportRequestStatusDbDto status ->
434+
assert status?.requestId == expectedRequestId
435+
assert DateUtils.roughlyEqual(status.date, new Date())
436+
assert status?.oldStatus == SeriesImportRequestStatus.DOWNLOADING_FAILED
437+
assert status?.newStatus == SeriesImportRequestStatus.DOWNLOADING_SUCCEEDED
438+
return true
439+
})
440+
}
441+
425442
//
426443
// Tests for getDownloadedContent()
427444
//

‎src/test/robotframework/series/import/request-logic.robot

+8
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,14 @@ Submit a request with a document that couldn't be parsed
141141
Submit Form id:import-series-form
142142
Element Text Should Be id:request-status ParsingFailed
143143

144+
Retry of downloading allows the import to be succeed
145+
[Documentation] Verify that a failed request can be re-triggered
146+
Go To ${SITE_URL}/series/import/request/1
147+
Element Text Should Be id:request-url http://127.0.0.1:8080/series/1?lang=en
148+
Element Text Should Be id:request-status DownloadingFailed
149+
Submit Form id:retry-import-series-form
150+
Element Text Should Be id:request-status ParsingSucceeded
151+
144152
*** Keywords ***
145153
Before Test Suite
146154
Open Browser ${SITE_URL}/account/auth ${BROWSER}

0 commit comments

Comments
 (0)