Skip to content

Commit 479c589

Browse files
psilabs-devpsilabs-dev
authored
Add locks to basic write operations (#1277)
* add category resource locks * add archive metadata write locks * add tankoubon metadata write locks * refactor exec with lock
1 parent ba0aaf3 commit 479c589

File tree

7 files changed

+386
-214
lines changed

7 files changed

+386
-214
lines changed

‎lib/LANraragi/Controller/Api/Archive.pm‎

Lines changed: 118 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use File::Temp qw(tempdir);
1212
use File::Basename;
1313
use File::Find;
1414

15-
use LANraragi::Utils::Generic qw(render_api_response is_archive get_bytelength);
15+
use LANraragi::Utils::Generic qw(render_api_response is_archive get_bytelength exec_with_lock);
1616
use LANraragi::Utils::Database qw(get_archive_json set_isnew);
1717
use LANraragi::Utils::Logging qw(get_logger);
1818

@@ -157,106 +157,86 @@ sub create_archive {
157157
my $uploadMime = $upload->headers->content_type;
158158
$filename = LANraragi::Utils::Database::redis_encode($filename);
159159

160-
# lock resource
161-
my $lock = $redis->setnx( "upload:$filename", 1 );
162-
if ( !$lock ) {
163-
return $self->render(
164-
json => {
165-
operation => "upload",
166-
success => 0,
167-
error => "Locked resource: $filename."
168-
},
169-
status => 423
170-
);
171-
}
172-
$redis->expire( "upload:$filename", 10 );
160+
return unless exec_with_lock( $self, $redis, "upload:$filename", "upload", $filename, sub {
161+
162+
# metadata extraction
163+
my $catid = $self->req->param('category_id');
164+
my $tags = $self->req->param('tags');
165+
my $title = $self->req->param('title');
166+
my $summary = $self->req->param('summary');
173167

174-
# metadata extraction
175-
my $catid = $self->req->param('category_id');
176-
my $tags = $self->req->param('tags');
177-
my $title = $self->req->param('title');
178-
my $summary = $self->req->param('summary');
168+
# return error if archive is not supported.
169+
if ( !is_archive($filename) ) {
170+
return $self->render(
171+
json => {
172+
operation => "upload",
173+
success => 0,
174+
error => "Unsupported file extension ($filename)"
175+
},
176+
status => 415
177+
);
178+
}
179179

180-
# return error if archive is not supported.
181-
if ( !is_archive($filename) ) {
182-
$redis->del("upload:$filename");
183-
$redis->quit();
184-
return $self->render(
185-
json => {
186-
operation => "upload",
187-
success => 0,
188-
error => "Unsupported file extension ($filename)"
189-
},
190-
status => 415
191-
);
192-
}
180+
# Move file to a temp folder (not the default LRR one)
181+
my $tempdir = tempdir();
193182

194-
# Move file to a temp folder (not the default LRR one)
195-
my $tempdir = tempdir();
183+
my ( $fn, $path, $ext ) = fileparse( $filename, qr/\.[^.]*/ );
184+
my $byte_limit = LANraragi::Model::Config->enable_cryptofs ? 143 : 255;
196185

197-
my ( $fn, $path, $ext ) = fileparse( $filename, qr/\.[^.]*/ );
198-
my $byte_limit = LANraragi::Model::Config->enable_cryptofs ? 143 : 255;
186+
$filename = $fn;
187+
while ( get_bytelength( $filename . $ext . ".upload" ) > $byte_limit ) {
188+
$filename = substr( $filename, 0, -1 );
189+
}
190+
$filename = $filename . $ext;
199191

200-
$filename = $fn;
201-
while ( get_bytelength( $filename . $ext . ".upload" ) > $byte_limit ) {
202-
$filename = substr( $filename, 0, -1 );
203-
}
204-
$filename = $filename . $ext;
192+
my $tempfile = $tempdir . '/' . $filename;
193+
if ( !$upload->move_to($tempfile) ) {
194+
$logger->error("Could not move uploaded file $filename to $tempfile");
195+
return $self->render(
196+
json => {
197+
operation => "upload",
198+
success => 0,
199+
error => "Couldn't move uploaded file to temporary location."
200+
},
201+
status => 500
202+
);
203+
}
205204

206-
my $tempfile = $tempdir . '/' . $filename;
207-
if ( !$upload->move_to($tempfile) ) {
208-
$logger->error("Could not move uploaded file $filename to $tempfile");
209-
$redis->del("upload:$filename");
210-
$redis->quit();
211-
return $self->render(
212-
json => {
213-
operation => "upload",
214-
success => 0,
215-
error => "Couldn't move uploaded file to temporary location."
205+
# Update $tempfile to the exact reference created by the host filesystem
206+
# This is done by finding the first (and only) file in $tempdir.
207+
find(
208+
sub {
209+
return if -d $_;
210+
$tempfile = $File::Find::name;
211+
$filename = $_;
216212
},
217-
status => 500
213+
$tempdir
218214
);
219-
}
220215

221-
# Update $tempfile to the exact reference created by the host filesystem
222-
# This is done by finding the first (and only) file in $tempdir.
223-
find(
224-
sub {
225-
return if -d $_;
226-
$tempfile = $File::Find::name;
227-
$filename = $_;
228-
},
229-
$tempdir
230-
);
216+
my ( $status_code, $id, $response_title, $message ) =
217+
LANraragi::Model::Upload::handle_incoming_file( $tempfile, $catid, $tags, $title, $summary );
231218

232-
my ( $status_code, $id, $response_title, $message ) =
233-
LANraragi::Model::Upload::handle_incoming_file( $tempfile, $catid, $tags, $title, $summary );
219+
unless ( $status_code == 200 ) {
220+
return $self->render(
221+
json => {
222+
operation => "upload",
223+
success => 0,
224+
error => $message,
225+
id => $id
226+
},
227+
status => $status_code
228+
);
229+
}
234230

235-
unless ( $status_code == 200 ) {
236-
$redis->del("upload:$filename");
237-
$redis->quit();
238231
return $self->render(
239232
json => {
240233
operation => "upload",
241-
success => 0,
242-
error => $message,
234+
success => 1,
243235
id => $id
244236
},
245-
status => $status_code
237+
status => 200
246238
);
247-
}
248-
249-
$redis->del("upload:$filename");
250-
$redis->quit();
251-
252-
return $self->render(
253-
json => {
254-
operation => "upload",
255-
success => 1,
256-
id => $id
257-
},
258-
status => 200
259-
);
239+
});
260240
}
261241

262242
# Serve an archive page from the temporary folder, using RenderFile.
@@ -289,31 +269,39 @@ sub clear_new {
289269
my $self = shift;
290270
my $id = check_id_parameter( $self, "clear_new" ) || return;
291271

292-
set_isnew( $id, "false" );
272+
my $redis = LANraragi::Model::Config->get_redis;
273+
274+
return unless exec_with_lock( $self, $redis, "archive-write:$id", "clear_new", $id, sub {
275+
set_isnew( $id, "false" );
293276

294-
$self->render(
295-
json => {
296-
operation => "clear_new",
297-
id => $id,
298-
success => 1
299-
}
300-
);
277+
$self->render(
278+
json => {
279+
operation => "clear_new",
280+
id => $id,
281+
success => 1
282+
}
283+
);
284+
});
301285
}
302286

303287
sub delete_archive {
304288
my $self = shift;
305289
my $id = check_id_parameter( $self, "delete_archive" ) || return;
306290

307-
my $delStatus = LANraragi::Model::Archive::delete_archive($id);
291+
my $redis = LANraragi::Model::Config->get_redis;
292+
293+
return unless exec_with_lock( $self, $redis, "archive-write:$id", "delete_archive", $id, sub {
294+
my $delStatus = LANraragi::Model::Archive::delete_archive($id);
308295

309-
$self->render(
310-
json => {
311-
operation => "delete_archive",
312-
id => $id,
313-
filename => $delStatus,
314-
success => $delStatus eq "0" ? 0 : 1
315-
}
316-
);
296+
$self->render(
297+
json => {
298+
operation => "delete_archive",
299+
id => $id,
300+
filename => $delStatus,
301+
success => $delStatus eq "0" ? 0 : 1
302+
}
303+
);
304+
});
317305
}
318306

319307
sub update_metadata {
@@ -324,16 +312,20 @@ sub update_metadata {
324312
my $tags = $self->req->param('tags');
325313
my $summary = $self->req->param('summary');
326314

327-
my $err = LANraragi::Model::Archive::update_metadata( $id, $title, $tags, $summary );
315+
my $redis = LANraragi::Model::Config->get_redis;
316+
317+
return unless exec_with_lock( $self, $redis, "archive-write:$id", "update_metadata", $id, sub {
318+
my $err = LANraragi::Model::Archive::update_metadata( $id, $title, $tags, $summary );
328319

329-
if ( $err eq "" ) {
330-
my $title = LANraragi::Model::Archive::get_title($id);
331-
my $successMessage = "Updated metadata for \"$title\"!";
320+
if ( $err eq "" ) {
321+
my $title = LANraragi::Model::Archive::get_title($id);
322+
my $successMessage = "Updated metadata for \"$title\"!";
332323

333-
render_api_response( $self, "update_metadata", undef, $successMessage );
334-
} else {
335-
render_api_response( $self, "update_metadata", $err );
336-
}
324+
render_api_response( $self, "update_metadata", undef, $successMessage );
325+
} else {
326+
render_api_response( $self, "update_metadata", $err );
327+
}
328+
});
337329
}
338330

339331
sub update_progress {
@@ -367,26 +359,25 @@ sub update_progress {
367359
return;
368360
}
369361

370-
# Just set the progress value.
371-
$redis->hset( $id, "progress", $page );
372-
$redis->hset( $id, "lastreadtime", $time );
362+
return unless exec_with_lock( $self, $redis, "archive-write:$id", "update_progress", $id, sub {
363+
# Just set the progress value.
364+
$redis->hset( $id, "progress", $page );
365+
$redis->hset( $id, "lastreadtime", $time );
373366

374-
# Update total pages read statistic
375-
$redis_cfg->incr("LRR_TOTALPAGESTAT");
376-
377-
$redis->quit();
378-
$redis_cfg->quit();
379-
380-
$self->render(
381-
json => {
382-
operation => "update_progress",
383-
id => $id,
384-
page => $page,
385-
lastreadtime => $time,
386-
success => 1
387-
}
388-
);
367+
# Update total pages read statistic
368+
$redis_cfg->incr("LRR_TOTALPAGESTAT");
369+
$redis_cfg->quit();
389370

371+
$self->render(
372+
json => {
373+
operation => "update_progress",
374+
id => $id,
375+
page => $page,
376+
lastreadtime => $time,
377+
success => 1
378+
}
379+
);
380+
});
390381
}
391382

392383
1;

0 commit comments

Comments
 (0)