diff options
| -rw-r--r-- | python/tests/api_entity_editing.py | 93 | ||||
| -rw-r--r-- | python/tests/api_entity_state.py | 175 | ||||
| -rw-r--r-- | python/tests/fixtures.py | 11 | ||||
| -rw-r--r-- | rust/src/api_entity_crud.rs | 42 | ||||
| -rw-r--r-- | rust/src/api_wrappers.rs | 26 |
5 files changed, 259 insertions, 88 deletions
diff --git a/python/tests/api_entity_editing.py b/python/tests/api_entity_editing.py new file mode 100644 index 00000000..3c0baa8e --- /dev/null +++ b/python/tests/api_entity_editing.py @@ -0,0 +1,93 @@ + +import json +import pytest +from copy import copy + +from fatcat_client import * +from fatcat_client.rest import ApiException +from fixtures import * + + +def test_multiple_edits_same_group(api): + + c1 = CreatorEntity(display_name="test updates") + + # create + eg = quick_eg(api) + c1 = api.get_creator(api.create_creator(c1, editgroup=eg.id).ident) + api.accept_editgroup(eg.id) + + # try multiple edits in the same group + eg = quick_eg(api) + c2 = CreatorEntity(display_name="left") + c3 = CreatorEntity(display_name="right") + edit = api.update_creator(c1.ident, c2, editgroup=eg.id) + # should fail with existing + with pytest.raises(fatcat_client.rest.ApiException): + api.update_creator(c1.ident, c3, editgroup=eg.id) + # ... but succeed after deleting + api.delete_creator_edit(edit.edit_id) + api.update_creator(c1.ident, c3, editgroup=eg.id) + api.accept_editgroup(eg.id) + res = api.get_creator(c1.ident) + assert res.display_name == "right" + eg = api.get_editgroup(eg.id) + assert len(eg.edits.creators) == 1 + + # cleanup + eg = quick_eg(api) + api.delete_creator(c1.ident) + api.accept_editgroup(eg.id) + + +def test_edit_deletion(api): + + c1 = CreatorEntity(display_name="test edit updates") + + # create + eg = quick_eg(api) + c1 = api.get_creator(api.create_creator(c1, editgroup=eg.id).ident) + api.accept_editgroup(eg.id) + + # try multiple edits in the same group + c2 = CreatorEntity(display_name="update one") + eg = quick_eg(api) + eg = api.get_editgroup(eg.id) + assert len(eg.edits.creators) == 0 + edit = api.update_creator(c1.ident, c2, editgroup=eg.id) + eg = api.get_editgroup(eg.id) + assert len(eg.edits.creators) == 1 + api.delete_creator_edit(edit.edit_id) + eg = api.get_editgroup(eg.id) + assert len(eg.edits.creators) == 0 + + api.accept_editgroup(eg.id) + res = api.get_creator(c1.ident) + assert res.display_name == "test edit updates" + eg = api.get_editgroup(eg.id) + assert len(eg.edits.creators) == 0 + + # cleanup + eg = quick_eg(api) + api.delete_creator(c1.ident) + api.accept_editgroup(eg.id) + + +def test_empty_editgroup(api): + eg = quick_eg(api) + api.accept_editgroup(eg.id) + + +def test_delete_accepted_edit(api): + + c1 = CreatorEntity(display_name="test edit updates") + + # create + eg = quick_eg(api) + edit = api.create_creator(c1, editgroup=eg.id) + api.accept_editgroup(eg.id) + + # try to delete + with pytest.raises(fatcat_client.rest.ApiException): + api.delete_creator_edit(edit.edit_id) + diff --git a/python/tests/api_entity_state.py b/python/tests/api_entity_state.py index 208688e9..35a8527a 100644 --- a/python/tests/api_entity_state.py +++ b/python/tests/api_entity_state.py @@ -1,23 +1,10 @@ -import json import pytest -from copy import copy from fatcat_client import * from fatcat_client.rest import ApiException from fixtures import * -def test_get_changelog_entry(api): - """ - Basically just to check that fixture is working - """ - cl = api.get_changelog_entry(1) - assert cl - -def quick_eg(api_inst): - eg = api_inst.create_editgroup( - fatcat_client.Editgroup(editor_id='aaaaaaaaaaaabkvkaaaaaaaaae')) - return eg def test_redirect_entity(api): """ @@ -122,6 +109,7 @@ def test_redirect_entity(api): api.delete_creator(c2.ident) api.accept_editgroup(eg.id) + def test_delete_entity(api): offset = 0 @@ -169,9 +157,7 @@ def test_delete_entity(api): #api.accept_editgroup(eg.id) assert False except fatcat_client.rest.ApiException as e: - # error is 4xx - print(e) - assert 400 <= e.status < 500 + assert 400 <= e.status < 500 # error is 4xx # undelete eg = quick_eg(api) @@ -189,72 +175,6 @@ def test_delete_entity(api): api.delete_creator(c1.ident) api.accept_editgroup(eg.id) -def test_multiple_edits_same_group(api): - - c1 = CreatorEntity(display_name="test updates") - - # create - eg = quick_eg(api) - c1 = api.get_creator(api.create_creator(c1, editgroup=eg.id).ident) - api.accept_editgroup(eg.id) - - # try multiple edits in the same group - eg = quick_eg(api) - c2 = CreatorEntity(display_name="left") - c3 = CreatorEntity(display_name="right") - edit = api.update_creator(c1.ident, c2, editgroup=eg.id) - # should fail with existing - with pytest.raises(fatcat_client.rest.ApiException): - api.update_creator(c1.ident, c3, editgroup=eg.id) - # ... but succeed after deleting - api.delete_creator_edit(edit.edit_id) - api.update_creator(c1.ident, c3, editgroup=eg.id) - api.accept_editgroup(eg.id) - res = api.get_creator(c1.ident) - assert res.display_name == "right" - eg = api.get_editgroup(eg.id) - assert len(eg.edits.creators) == 1 - - # cleanup - eg = quick_eg(api) - api.delete_creator(c1.ident) - api.accept_editgroup(eg.id) - -def test_edit_deletion(api): - - c1 = CreatorEntity(display_name="test edit updates") - - # create - eg = quick_eg(api) - c1 = api.get_creator(api.create_creator(c1, editgroup=eg.id).ident) - api.accept_editgroup(eg.id) - - # try multiple edits in the same group - c2 = CreatorEntity(display_name="update one") - eg = quick_eg(api) - eg = api.get_editgroup(eg.id) - assert len(eg.edits.creators) == 0 - edit = api.update_creator(c1.ident, c2, editgroup=eg.id) - eg = api.get_editgroup(eg.id) - assert len(eg.edits.creators) == 1 - api.delete_creator_edit(edit.edit_id) - eg = api.get_editgroup(eg.id) - assert len(eg.edits.creators) == 0 - - api.accept_editgroup(eg.id) - res = api.get_creator(c1.ident) - assert res.display_name == "test edit updates" - eg = api.get_editgroup(eg.id) - assert len(eg.edits.creators) == 0 - - # cleanup - eg = quick_eg(api) - api.delete_creator(c1.ident) - api.accept_editgroup(eg.id) - -def test_empty_editgroup(api): - eg = quick_eg(api) - api.accept_editgroup(eg.id) def test_recursive_redirects_entity(api): @@ -401,6 +321,7 @@ def test_recursive_redirects_entity(api): # c3 already deleted api.accept_editgroup(eg.id) + def test_self_redirect(api): c1 = CreatorEntity(display_name="test self-redirect") @@ -415,3 +336,93 @@ def test_self_redirect(api): eg = quick_eg(api) with pytest.raises(fatcat_client.rest.ApiException): merge_edit = api.update_creator(c1.ident, c1_redirect, editgroup=eg.id) + + +def test_wip_redirect(api): + + # create first + c1 = CreatorEntity(display_name="test one") + eg = quick_eg(api) + c1 = api.get_creator(api.create_creator(c1, editgroup=eg.id).ident) + api.accept_editgroup(eg.id) + + # start creating second entity + c2 = CreatorEntity(display_name="test two") + eg = quick_eg(api) + c2 = api.get_creator(api.create_creator(c2, editgroup=eg.id).ident) + assert c2.state == "wip" + + # redirect first to second (should fail) + eg = quick_eg(api) + c1_redirect = CreatorEntity(redirect=c2.ident) + try: + api.update_creator(c1.ident, c1_redirect, editgroup=eg.id) + assert False + except fatcat_client.rest.ApiException as e: + assert 400 <= e.status < 500 + assert "WIP" in e.body + + +def test_create_redirect(api): + + # create first + c1 = CreatorEntity(display_name="test one") + eg = quick_eg(api) + c1 = api.get_creator(api.create_creator(c1, editgroup=eg.id).ident) + api.accept_editgroup(eg.id) + + # create second + c2 = CreatorEntity(display_name="blah", redirect=c1.ident) + eg = quick_eg(api) + try: + api.create_creator(c2, editgroup=eg.id) + assert False + except fatcat_client.rest.ApiException as e: + assert 400 <= e.status < 500 + assert "redirect" in e.body + + # again with releases + r1 = ReleaseEntity(title="test one") + eg = quick_eg(api) + r1 = api.get_release(api.create_release(r1, editgroup=eg.id).ident) + api.accept_editgroup(eg.id) + r2 = ReleaseEntity(title="blah", redirect=c1.ident) + eg = quick_eg(api) + try: + api.create_release(r2, editgroup=eg.id) + assert False + except fatcat_client.rest.ApiException as e: + assert 400 <= e.status < 500 + assert "redirect" in e.body + + +def test_required_entity_fields(api): + eg = quick_eg(api) + + # Creator + try: + c1 = CreatorEntity() + api.create_creator(c1, editgroup=eg.id) + assert False + except fatcat_client.rest.ApiException as e: + assert 400 <= e.status < 500 + assert "display_name" in e.body + + # Container + try: + c1 = ContainerEntity() + api.create_container(c1, editgroup=eg.id) + assert False + except fatcat_client.rest.ApiException as e: + assert 400 <= e.status < 500 + assert "name" in e.body + + # Release + try: + c1 = ReleaseEntity() + api.create_release(c1, editgroup=eg.id) + assert False + except fatcat_client.rest.ApiException as e: + assert 400 <= e.status < 500 + assert "title" in e.body + diff --git a/python/tests/fixtures.py b/python/tests/fixtures.py index 509c2093..567e9132 100644 --- a/python/tests/fixtures.py +++ b/python/tests/fixtures.py @@ -25,10 +25,19 @@ def api(): api_client = fatcat_client.DefaultApi(fatcat_client.ApiClient(conf)) return api_client +def test_get_changelog_entry(api): + """Check that fixture is working""" + cl = api.get_changelog_entry(1) + assert cl ## Helpers ################################################################## -# TODO: what are these even here for? +def quick_eg(api_inst): + eg = api_inst.create_editgroup( + fatcat_client.Editgroup(editor_id='aaaaaaaaaaaabkvkaaaaaaaaae')) + return eg + +# TODO: what are these even here for? def check_entity_fields(e): for key in ('rev', 'is_live', 'redirect_id'): assert key in e diff --git a/rust/src/api_entity_crud.rs b/rust/src/api_entity_crud.rs index 3b12a446..0315f0a7 100644 --- a/rust/src/api_entity_crud.rs +++ b/rust/src/api_entity_crud.rs @@ -136,6 +136,10 @@ macro_rules! generic_db_create { // TODO: this path should call generic_db_create_batch ($ident_table: ident, $edit_table: ident) => { fn db_create(&self, conn: &DbConn, edit_context: &EditContext) -> Result<Self::EditRow> { + if self.redirect.is_some() { + return Err(ErrorKind::OtherBadRequest( + "can't create an entity that redirects from the start".to_string()).into()); + } let rev_id = self.db_insert_rev(conn)?; let ident: Uuid = insert_into($ident_table::table) .values($ident_table::rev_id.eq(&rev_id)) @@ -160,6 +164,10 @@ macro_rules! generic_db_create_batch { edit_context: &EditContext, models: &[&Self], ) -> Result<Vec<Self::EditRow>> { + if models.iter().any(|m| m.redirect.is_some()) { + return Err(ErrorKind::OtherBadRequest( + "can't create an entity that redirects from the start".to_string()).into()); + } let rev_ids: Vec<Uuid> = Self::db_insert_revs(conn, models)?; let ident_ids: Vec<Uuid> = insert_into($ident_table::table) .values( @@ -221,9 +229,10 @@ macro_rules! generic_db_update { } // TODO: if we get a diesel not-found here, should be a special error response? let target: Self::IdentRow = $ident_table::table.find(redirect_ident).first(conn)?; - if target.is_live == false { + if target.is_live != true { // there is no race condition on this check because WIP -> is_live=true is // a one-way operation + // XXX: return Err(ErrorKind::OtherBadRequest( "attempted to redirect to a WIP entity".to_string()).into()); } @@ -642,12 +651,17 @@ impl EntityCrud for ContainerEntity { } } + if models.iter().any(|m| m.name.is_none()) { + return Err(ErrorKind::OtherBadRequest( + "name is required for all Container entities".to_string()).into()); + } + let rev_ids: Vec<Uuid> = insert_into(container_rev::table) .values( models .iter() .map(|model| ContainerRevNewRow { - name: model.name.clone().unwrap(), // XXX: unwrap + name: model.name.clone().unwrap(), // unwrap checked above publisher: model.publisher.clone(), issnl: model.issnl.clone(), wikidata_qid: model.wikidata_qid.clone(), @@ -746,12 +760,17 @@ impl EntityCrud for CreatorEntity { } } + if models.iter().any(|m| m.display_name.is_none()) { + return Err(ErrorKind::OtherBadRequest( + "display_name is required for all Creator entities".to_string()).into()); + } + let rev_ids: Vec<Uuid> = insert_into(creator_rev::table) .values( models .iter() - .map(|model| CreatorRevNewRow { - display_name: model.display_name.clone().unwrap(), // XXX: unwrap + .map(|model|CreatorRevNewRow { + display_name: model.display_name.clone().unwrap(), // unwrapped checked above given_name: model.given_name.clone(), surname: model.surname.clone(), orcid: model.orcid.clone(), @@ -1010,6 +1029,10 @@ impl EntityCrud for ReleaseEntity { } fn db_create(&self, conn: &DbConn, edit_context: &EditContext) -> Result<Self::EditRow> { + if self.redirect.is_some() { + return Err(ErrorKind::OtherBadRequest( + "can't create an entity that redirects from the start".to_string()).into()); + } let mut edits = Self::db_create_batch(conn, edit_context, &[self])?; // probably a more elegant way to destroy the vec and take first element Ok(edits.pop().unwrap()) @@ -1022,6 +1045,10 @@ impl EntityCrud for ReleaseEntity { ) -> Result<Vec<Self::EditRow>> { // This isn't the generic implementation because we need to create Work entities for each // of the release entities passed (at least in the common case) + if models.iter().any(|m| m.redirect.is_some()) { + return Err(ErrorKind::OtherBadRequest( + "can't create an entity that redirects from the start".to_string()).into()); + } // Generate the set of new work entities to insert (usually one for each release, but some // releases might be pointed to a work already) @@ -1239,13 +1266,18 @@ impl EntityCrud for ReleaseEntity { } } + if models.iter().any(|m| m.title.is_none()) { + return Err(ErrorKind::OtherBadRequest( + "title is required for all Release entities".to_string()).into()); + } + let rev_ids: Vec<Uuid> = insert_into(release_rev::table) .values( models .iter() .map(|model| { Ok(ReleaseRevNewRow { - title: model.title.clone().unwrap(), // XXX: unwrap() + title: model.title.clone().unwrap(), // titles checked above release_type: model.release_type.clone(), release_status: model.release_status.clone(), release_date: model.release_date, diff --git a/rust/src/api_wrappers.rs b/rust/src/api_wrappers.rs index 545a2079..aa7f9ec3 100644 --- a/rust/src/api_wrappers.rs +++ b/rust/src/api_wrappers.rs @@ -66,6 +66,8 @@ macro_rules! wrap_entity_handlers { $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $get_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -103,6 +105,8 @@ macro_rules! wrap_entity_handlers { $post_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => $post_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $post_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $post_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -140,6 +144,8 @@ macro_rules! wrap_entity_handlers { $post_batch_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => $post_batch_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $post_batch_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $post_batch_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -183,6 +189,8 @@ macro_rules! wrap_entity_handlers { $update_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::InvalidEntityStateTransform(e), _)) => $update_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $update_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $update_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -224,6 +232,8 @@ macro_rules! wrap_entity_handlers { $delete_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::InvalidEntityStateTransform(e), _)) => $delete_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $delete_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $delete_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -253,6 +263,8 @@ macro_rules! wrap_entity_handlers { Err(Error(ErrorKind::InvalidFatcatId(e), _)) => $get_history_resp::BadRequest(ErrorResponse { message: ErrorKind::InvalidFatcatId(e).to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $get_history_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $get_history_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -294,6 +306,8 @@ macro_rules! wrap_entity_handlers { $get_rev_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::MalformedExternalId(e), _)) => $get_rev_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $get_rev_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $get_rev_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -316,6 +330,8 @@ macro_rules! wrap_entity_handlers { $get_edit_resp::FoundEdit(edit), Err(Error(ErrorKind::Diesel(::diesel::result::Error::NotFound), _)) => $get_edit_resp::NotFound(ErrorResponse { message: format!("No such {} entity edit: {}", stringify!($model), edit_id) }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $get_edit_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $get_edit_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -341,6 +357,8 @@ macro_rules! wrap_entity_handlers { $delete_edit_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::EditgroupAlreadyAccepted(e), _)) => $delete_edit_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $delete_edit_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $delete_edit_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -370,6 +388,8 @@ macro_rules! wrap_entity_handlers { Err(Error(ErrorKind::InvalidFatcatId(e), _)) => $get_redirects_resp::BadRequest(ErrorResponse { message: ErrorKind::InvalidFatcatId(e).to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $get_redirects_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $get_redirects_resp::GenericError(ErrorResponse { message: e.to_string() }) @@ -410,6 +430,8 @@ macro_rules! wrap_lookup_handler { $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::MissingOrMultipleExternalId(e), _)) => { $get_resp::BadRequest(ErrorResponse { message: e.to_string(), }) }, + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $get_resp::BadRequest(ErrorResponse { message: e.to_string() }) @@ -441,6 +463,8 @@ macro_rules! wrap_fcid_handler { $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::NotInControlledVocabulary(e), _)) => $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $get_resp::BadRequest(ErrorResponse { message: e.to_string() }) @@ -477,6 +501,8 @@ macro_rules! wrap_fcid_hide_handler { $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(Error(ErrorKind::NotInControlledVocabulary(e), _)) => $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), + Err(Error(ErrorKind::OtherBadRequest(e), _)) => + $get_resp::BadRequest(ErrorResponse { message: e.to_string() }), Err(e) => { error!("{}", e); $get_resp::BadRequest(ErrorResponse { message: e.to_string() }) |