Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MODINVSTOR-1284 User can delete local Subject types/sources when it linked to Instance #1119

Merged
merged 28 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5367813
MODINVSTOR-1284 User can delete local Subject types/sources when it l…
JavokhirAbdullayev Nov 27, 2024
c23fc00
add test
JavokhirAbdullayev Nov 27, 2024
57c293c
Merge branch 'master' into MODINVSTOR-1284
JavokhirAbdullayev Nov 27, 2024
4c83421
fix checkstyle
JavokhirAbdullayev Nov 27, 2024
51501f7
Merge remote-tracking branch 'origin/MODINVSTOR-1284' into MODINVSTOR…
JavokhirAbdullayev Nov 27, 2024
306c7e4
add additional test
JavokhirAbdullayev Nov 27, 2024
a50135c
upd msg
JavokhirAbdullayev Nov 28, 2024
c365bc0
code review
JavokhirAbdullayev Dec 3, 2024
01b0774
fix codestyle
JavokhirAbdullayev Dec 3, 2024
5a9a3b9
add indexes
JavokhirAbdullayev Dec 4, 2024
f9c45f9
add indexes
JavokhirAbdullayev Dec 4, 2024
2c14d86
remove redundant changes
JavokhirAbdullayev Dec 6, 2024
75da8ee
Merge branch 'master' into MODINVSTOR-1284
JavokhirAbdullayev Dec 6, 2024
65822c2
some changes for clean up
JavokhirAbdullayev Dec 6, 2024
b432b01
increase coverage
JavokhirAbdullayev Dec 10, 2024
d825183
increase coverage
JavokhirAbdullayev Dec 10, 2024
47d937f
Update NEWS.md
JavokhirAbdullayev Dec 10, 2024
c521ce2
pr review
JavokhirAbdullayev Dec 10, 2024
4d6aa91
Merge remote-tracking branch 'origin/MODINVSTOR-1284' into MODINVSTOR…
JavokhirAbdullayev Dec 10, 2024
57d6c96
clean up
JavokhirAbdullayev Dec 10, 2024
986be9a
add batch operations
JavokhirAbdullayev Dec 10, 2024
ddb394e
fix checkstyle
JavokhirAbdullayev Dec 10, 2024
aa90f35
add more test, update create batch instances
JavokhirAbdullayev Dec 11, 2024
ad93982
Merge branch 'master' into MODINVSTOR-1284
JavokhirAbdullayev Dec 11, 2024
64a6c1d
make creation of instance and linking subjects transactional
JavokhirAbdullayev Dec 13, 2024
4991881
checkstyle
JavokhirAbdullayev Dec 13, 2024
e5b78f1
issues with asyn migration job
JavokhirAbdullayev Dec 13, 2024
05eb880
decrease number of recs in async test, to prevent timeout errors
JavokhirAbdullayev Dec 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Requires `API_NAME vX.Y`

### Features
* User can delete local Subject types/sources when it linked to Instance ([MODINVSTOR-1284](https://folio-org.atlassian.net/browse/MODINVSTOR-1284))
JavokhirAbdullayev marked this conversation as resolved.
Show resolved Hide resolved
* Modify endpoint for bulk instances upsert with publish events flag ([MODINVSTOR-1283](https://folio-org.atlassian.net/browse/MODINVSTOR-1283))
* Change Kafka event publishing keys for holdings and items ([MODINVSTOR-1281](https://folio-org.atlassian.net/browse/MODINVSTOR-1281))
* Merge custom ECS TLR feature branch into master ([MODINVSTOR-1262](https://folio-org.atlassian.net/browse/MODINVSTOR-1262))
Expand Down
51 changes: 51 additions & 0 deletions src/main/java/org/folio/persist/InstanceRepository.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.folio.persist;

import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.folio.rest.impl.BoundWithPartApi.BOUND_WITH_TABLE;
import static org.folio.rest.impl.HoldingsStorageApi.HOLDINGS_RECORD_TABLE;
import static org.folio.rest.impl.ItemStorageApi.ITEM_TABLE;
Expand Down Expand Up @@ -35,11 +36,61 @@ public class InstanceRepository extends AbstractRepository<Instance> {
private static final String INSTANCE_SET_VIEW = "instance_set";
private static final String INSTANCE_HOLDINGS_ITEM_VIEW = "instance_holdings_item_view";
private static final String INVENTORY_VIEW_JSONB_FIELD = "inventory_view.jsonb";
private static final String INSTANCE_SUBJECT_SOURCE_TABLE = "instance_subject_source";
private static final String INSTANCE_SUBJECT_TYPE_TABLE = "instance_subject_type";


public InstanceRepository(Context context, Map<String, String> okapiHeaders) {
super(postgresClient(context, okapiHeaders), INSTANCE_TABLE, Instance.class);
}

public Future<Boolean> doesInstanceExistBySubjectField(String field, String value) {
JavokhirAbdullayev marked this conversation as resolved.
Show resolved Hide resolved
var sql = new StringBuilder("SELECT exists ( ");
sql.append("SELECT 1 FROM ");
if (field.equals("sourceId")) {
sql.append(INSTANCE_SUBJECT_SOURCE_TABLE);
sql.append(String.format(" WHERE source_id = '%s' ", value));
} else {
sql.append(INSTANCE_SUBJECT_TYPE_TABLE);
sql.append(String.format(" WHERE type_id = '%s' ", value));
}
sql.append(") as record_exists");

return postgresClient.execute(sql.toString())
.map(rowSet -> {
if (rowSet != null && rowSet.iterator().hasNext()) {
Row row = rowSet.iterator().next();
return row.getBoolean("record_exists");
}
return Boolean.FALSE;
});
}

public void linkInstanceWithSubjectSourceAndType(Instance instance) {
instance.getSubjects().forEach(subject -> {
var sql = new StringBuilder();
if (subject.getSourceId() != null) {
sql.append(" INSERT INTO ");
sql.append(INSTANCE_SUBJECT_SOURCE_TABLE);
sql.append(" (instance_id, source_id) ");
sql.append(" VALUES ");
sql.append(String.format("( '%s' , '%s' ); ", instance.getId(), subject.getSourceId()));
}
if (subject.getTypeId() != null) {
sql.append(" INSERT INTO ");
sql.append(INSTANCE_SUBJECT_TYPE_TABLE);
sql.append(" (instance_id, type_id) ");
sql.append(" VALUES ");
sql.append(String.format("( '%s' , '%s' ); ", instance.getId(), subject.getTypeId()));
}
var sqlString = sql.toString();
if (!isBlank(sqlString)) {
postgresClient.execute(sql.toString());
}
});
}


public Future<RowStream<Row>> getAllIds(SQLConnection connection) {
return postgresClientFuturized.selectStream(connection,
"SELECT id FROM " + postgresClientFuturized.getFullTableName(INSTANCE_TABLE));
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/folio/rest/support/ResponseUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ public final class ResponseUtil {
"Illegal operation: Source field cannot be updated at non-consortium tenant";
public static final String SOURCE_CONSORTIUM_CANNOT_BE_APPLIED =
"Illegal operation: Source consortium cannot be applied at non-consortium tenant";
public static final String SOURCE_CANNOT_BE_DELETED_USED_BY_INSTANCE =
"Illegal operation: Source cannot be deleted, already exists on at least one instance record";
public static final String TYPE_CANNOT_BE_DELETED_USED_BY_INSTANCE =
"Illegal operation: Type cannot be deleted, already exists on at least one instance record";

private ResponseUtil() { }

Expand Down
25 changes: 25 additions & 0 deletions src/main/java/org/folio/rest/support/SubjectUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@

import static io.vertx.core.Future.succeededFuture;
import static org.folio.rest.jaxrs.resource.SubjectSources.PostSubjectSourcesResponse.respond422WithApplicationJson;
import static org.folio.rest.support.ResponseUtil.SOURCE_CANNOT_BE_DELETED_USED_BY_INSTANCE;
import static org.folio.rest.support.ResponseUtil.SOURCE_CANNOT_BE_FOLIO;
import static org.folio.rest.support.ResponseUtil.SOURCE_CANNOT_BE_UPDATED_AT_NON_ECS;
import static org.folio.rest.support.ResponseUtil.SOURCE_CONSORTIUM_CANNOT_BE_APPLIED;
import static org.folio.rest.support.ResponseUtil.SOURCE_FOLIO_CANNOT_BE_UPDATED;
import static org.folio.rest.support.ResponseUtil.TYPE_CANNOT_BE_DELETED_USED_BY_INSTANCE;
import static org.folio.rest.tools.utils.ValidationHelper.createValidationErrorMessage;

import io.vertx.core.Future;
import java.util.Map;
import java.util.Optional;
import javax.ws.rs.core.Response;
import org.folio.persist.InstanceRepository;
import org.folio.rest.jaxrs.model.Errors;
import org.folio.rest.jaxrs.model.SubjectSource;
import org.folio.services.consortium.ConsortiumService;
Expand Down Expand Up @@ -63,6 +66,28 @@ public static Future<Optional<Errors>> validateSubjectSourceUpdate(String incomi
return Future.succeededFuture(Optional.empty());
}

private static Future<Optional<Errors>> validateSubjectFieldDelete(String field, String value, String errorMsg,
InstanceRepository instanceRepository) {
return instanceRepository.doesInstanceExistBySubjectField(field, value)
.compose(exists -> {
if (exists) {
return Future.succeededFuture(getValidationErrorMessage(value, errorMsg));
}
return Future.succeededFuture(Optional.empty());
});
}

public static Future<Optional<Errors>> validateSubjectSourceDelete(String sourceId,
InstanceRepository instanceRepository) {
return validateSubjectFieldDelete("sourceId", sourceId,
SOURCE_CANNOT_BE_DELETED_USED_BY_INSTANCE, instanceRepository);
}

public static Future<Optional<Errors>> validateSubjectTypeDelete(String typeId,
InstanceRepository instanceRepository) {
return validateSubjectFieldDelete("typeId", typeId, TYPE_CANNOT_BE_DELETED_USED_BY_INSTANCE, instanceRepository);
}

public static Future<Response> sourceValidationError(Errors errors) {
return succeededFuture(respond422WithApplicationJson(errors));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,13 @@ public Future<Response> createInstance(Instance entity) {
// api client invoking this endpoint. The response is returned
// a little earlier so the api client can continue its processing
// while the domain event publish is satisfied.
.compose(response -> {
if (response.getEntity() instanceof Instance instanceResponse) {
instanceRepository.linkInstanceWithSubjectSourceAndType(instanceResponse);
JavokhirAbdullayev marked this conversation as resolved.
Show resolved Hide resolved
}
return Future.succeededFuture(response);
}
)
.onSuccess(domainEventPublisher.publishCreated());
})
.map(ResponseHandlerUtil::handleHridError);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
import static org.folio.rest.persist.PgUtil.put;
import static org.folio.rest.support.SubjectUtil.sourceValidationError;
import static org.folio.rest.support.SubjectUtil.validateSubjectSourceCreate;
import static org.folio.rest.support.SubjectUtil.validateSubjectSourceDelete;
import static org.folio.rest.support.SubjectUtil.validateSubjectSourceUpdate;

import io.vertx.core.Context;
import io.vertx.core.Future;
import java.util.Map;
import javax.ws.rs.core.Response;
import org.folio.persist.InstanceRepository;
import org.folio.persist.SubjectSourceRepository;
import org.folio.rest.exceptions.NotFoundException;
import org.folio.rest.jaxrs.model.SubjectSource;
Expand All @@ -36,10 +38,12 @@ public class SubjectSourceService {
private final SubjectSourceRepository repository;
private final SubjectSourceDomainEventPublisher domainEventService;
private final ConsortiumService consortiumService;
private final InstanceRepository instanceRepository;

public SubjectSourceService(Context context, Map<String, String> okapiHeaders) {
this.context = context;
this.okapiHeaders = okapiHeaders;
this.instanceRepository = new InstanceRepository(context, okapiHeaders);
this.repository = new SubjectSourceRepository(context, okapiHeaders);
this.domainEventService = new SubjectSourceDomainEventPublisher(context, okapiHeaders);
this.consortiumService = new ConsortiumServiceImpl(context.owner().createHttpClient(),
Expand Down Expand Up @@ -81,10 +85,17 @@ public Future<Response> update(String id, SubjectSource subjectSource) {

public Future<Response> delete(String id) {
return repository.getById(id)
.compose(oldSubjectSource -> deleteById(SUBJECT_SOURCE, id, okapiHeaders, context,
DeleteSubjectSourcesBySubjectSourceIdResponse.class)
.onSuccess(domainEventService.publishRemoved(oldSubjectSource))
);
.compose(subjectSource -> {
if (subjectSource != null) {
return validateSubjectSourceDelete(subjectSource.getId(), instanceRepository)
.compose(errors -> errors.isPresent()
? sourceValidationError(errors.get()) :
deleteById(SUBJECT_SOURCE, id, okapiHeaders, context, DeleteSubjectSourcesBySubjectSourceIdResponse.class)
.onSuccess(domainEventService.publishRemoved(subjectSource))
);
}
return Future.failedFuture(new NotFoundException("SubjectSource was not found"));
});
}

private Future<Response> createSubjectSource(SubjectSource subjectSource) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
import static org.folio.rest.support.SubjectUtil.sourceValidationError;
import static org.folio.rest.support.SubjectUtil.validateSubjectSourceCreate;
import static org.folio.rest.support.SubjectUtil.validateSubjectSourceUpdate;
import static org.folio.rest.support.SubjectUtil.validateSubjectTypeDelete;

import io.vertx.core.Context;
import io.vertx.core.Future;
import java.util.Map;
import javax.ws.rs.core.Response;
import org.folio.persist.InstanceRepository;
import org.folio.persist.SubjectTypeRepository;
import org.folio.rest.exceptions.NotFoundException;
import org.folio.rest.jaxrs.model.SubjectType;
Expand All @@ -35,10 +37,12 @@ public class SubjectTypeService {
private final SubjectTypeRepository repository;
private final SubjectTypeDomainEventPublisher domainEventService;
private final ConsortiumService consortiumService;
private final InstanceRepository instanceRepository;

public SubjectTypeService(Context context, Map<String, String> okapiHeaders) {
this.context = context;
this.okapiHeaders = okapiHeaders;
this.instanceRepository = new InstanceRepository(context, okapiHeaders);
this.repository = new SubjectTypeRepository(context, okapiHeaders);
this.domainEventService = new SubjectTypeDomainEventPublisher(context, okapiHeaders);
this.consortiumService = new ConsortiumServiceImpl(context.owner().createHttpClient(),
Expand Down Expand Up @@ -80,10 +84,18 @@ public Future<Response> update(String id, SubjectType subjectType) {

public Future<Response> delete(String id) {
return repository.getById(id)
.compose(oldSubjectType -> deleteById(SUBJECT_TYPE, id, okapiHeaders, context,
DeleteSubjectTypesBySubjectTypeIdResponse.class)
.onSuccess(domainEventService.publishRemoved(oldSubjectType))
);
.compose(oldSubjectType -> {
if (oldSubjectType != null) {
return validateSubjectTypeDelete(oldSubjectType.getId(), instanceRepository)
.compose(errors -> errors.isPresent()
? sourceValidationError(errors.get()) :
deleteById(SUBJECT_TYPE, id, okapiHeaders, context,
DeleteSubjectTypesBySubjectTypeIdResponse.class)
.onSuccess(domainEventService.publishRemoved(oldSubjectType))
);
}
return Future.failedFuture(new NotFoundException("SubjectSource was not found"));
});
}

private Future<Response> createSubjectType(SubjectType subjectType) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

CREATE TABLE IF NOT EXISTS ${myuniversity}_${mymodule}.instance_subject_source (
instance_id UUID,
source_id UUID,
CONSTRAINT fk_instance_id FOREIGN KEY(instance_id) REFERENCES ${myuniversity}_${mymodule}.instance(id) ON DELETE CASCADE,
CONSTRAINT fk_source_id FOREIGN KEY(source_id) REFERENCES ${myuniversity}_${mymodule}.subject_source(id)
);

CREATE INDEX IF NOT EXISTS instance_subject_source_source_idx
ON ${myuniversity}_${mymodule}.instance_subject_source(source_id);
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

CREATE TABLE IF NOT EXISTS ${myuniversity}_${mymodule}.instance_subject_type (
instance_id UUID,
type_id UUID,
CONSTRAINT fk_instance_id FOREIGN KEY(instance_id) REFERENCES ${myuniversity}_${mymodule}.instance(id) ON DELETE CASCADE,
CONSTRAINT fk_type_id FOREIGN KEY(type_id) REFERENCES ${myuniversity}_${mymodule}.subject_type(id)
);

CREATE INDEX IF NOT EXISTS instance_subject_type_type_idx
ON ${myuniversity}_${mymodule}.instance_subject_type(type_id);
10 changes: 10 additions & 0 deletions src/main/resources/templates/db_scripts/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1232,6 +1232,16 @@
"run": "after",
"snippetPath": "publication-period/migratePublicationPeriod.sql",
"fromModuleVersion": "28.0.0"
},
{
"run": "after",
"snippetPath": "createInstanceSubjectSourceTable.sql",
"fromModuleVersion": "28.1.0"
},
{
"run": "after",
"snippetPath": "createInstanceSubjectTypeTable.sql",
"fromModuleVersion": "28.1.0"
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@

@RunWith(VertxUnitRunner.class)
public class InstanceStorageTest extends TestBaseWithInventoryUtil {
public static final String SUBJECTS_KEY = "subjects";
private static final String INSTANCES_KEY = "instances";
private static final String TOTAL_RECORDS_KEY = "totalRecords";
private static final String METADATA_KEY = "metadata";
private static final String DATES_KEY = "dates";
private static final String SUBJECTS_KEY = "subjects";
private static final String TAG_VALUE = "test-tag";
private static final String STATUS_UPDATED_DATE_PROPERTY = "statusUpdatedDate";
private static final Logger log = LogManager.getLogger();
Expand Down
44 changes: 40 additions & 4 deletions src/test/java/org/folio/rest/api/SubjectSourceTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package org.folio.rest.api;

import static java.util.concurrent.TimeUnit.SECONDS;
import static org.folio.rest.api.InstanceStorageTest.SUBJECTS_KEY;
import static org.folio.rest.support.ResponseHandler.json;
import static org.folio.rest.support.ResponseUtil.SOURCE_CANNOT_BE_DELETED_USED_BY_INSTANCE;
import static org.folio.rest.support.http.InterfaceUrls.subjectSourcesUrl;
import static org.folio.utility.ModuleUtility.getClient;
import static org.folio.utility.ModuleUtility.prepareTenant;
Expand All @@ -17,13 +21,12 @@
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import junitparams.JUnitParamsRunner;
import lombok.SneakyThrows;
import org.folio.okapi.common.XOkapiHeaders;
import org.folio.rest.jaxrs.model.Subject;
import org.folio.rest.support.Response;
import org.folio.rest.support.ResponseHandler;
import org.folio.rest.support.http.ResourceClient;
import org.junit.AfterClass;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -83,9 +86,9 @@ public void cannotCreateSubjectSourceWithDuplicateCode()

CompletableFuture<Response> postCompleted = new CompletableFuture<>();
getClient().post(subjectSourcesUrl(""), subjectSource.put("name", "Test2"),
TENANT_ID, ResponseHandler.json(postCompleted));
TENANT_ID, json(postCompleted));

Response response = postCompleted.get(TIMEOUT, TimeUnit.SECONDS);
Response response = postCompleted.get(TIMEOUT, SECONDS);
assertThat(response.getStatusCode(), is(422));

JsonArray errors = response.getJson().getJsonArray("errors");
Expand Down Expand Up @@ -238,6 +241,34 @@ public void canUpdateSubjectSourceToLocalAtEcs() {
assertEquals(204, response.getStatusCode());
}

@Test
public void cannotDeleteSubjectSourceLinkedToInstance() {
var instanceId = UUID.randomUUID();

JsonObject subjectSource = new JsonObject()
.put("name", "Library Test " + UUID_INSTANCE_SUBJECT_SOURCE_ID)
.put("source", "local");

var subjectSourceId = createSubjectSource(subjectSource).getJson().getString("id");

var instance = instance(instanceId);
var subject = new Subject()
.withSourceId(subjectSourceId)
.withTypeId(UUID_INSTANCE_SUBJECT_TYPE_ID.toString())
.withValue("subject");
var subjects = new JsonArray().add(subject);
instance.put(SUBJECTS_KEY, subjects);

createInstanceRecord(instance);

Response response = deleteSubjectSource(UUID.fromString(subjectSourceId));

assertEquals(422, response.getStatusCode());
JsonArray errors = response.getJson().getJsonArray("errors");
assertThat(errors.size(), is(1));
assertTrue(errors.getJsonObject(0).getString("message").contains(SOURCE_CANNOT_BE_DELETED_USED_BY_INSTANCE));
}

private Response createSubjectSource(JsonObject object) {
return createSubjectSource(object, TENANT_ID);
}
Expand All @@ -253,4 +284,9 @@ private Response updateSubjectSource(String id, JsonObject object) {
private Response updateSubjectSource(String id, JsonObject object, String tenantId) {
return subjectSourceClient.attemptToReplace(id, object, tenantId, Map.of(XOkapiHeaders.URL, mockServer.baseUrl()));
}

private Response deleteSubjectSource(UUID id) {
return subjectSourceClient.attemptToDelete(id);
}

}
Loading