From ab2bf61034790a5f9a817d4ab13fb68713fdd6f2 Mon Sep 17 00:00:00 2001 From: Adam Korynta Date: Thu, 31 Oct 2024 13:35:35 -0700 Subject: [PATCH 1/5] add missing office where clause in Clob getAll query --- cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java index d76e33b3b..e166a44ad 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java @@ -134,10 +134,10 @@ public Clobs getClobs(String cursor, int pageSize, String officeLike, includeValues ? v_clob.VALUE : DSL.inline("").as(v_clob.VALUE) ) .from(v_clob) - //.innerJoin(forLimit).on(forLimit.field(v_clob.ID).eq(v_clob.ID)) .join(v_office).on(v_clob.OFFICE_CODE.eq(v_office.OFFICE_CODE)) .where(JooqDao.caseInsensitiveLikeRegex(v_clob.ID,idRegex)) .and(DSL.upper(v_clob.ID).greaterThan(clobCursor)) + .and(officeLike == null ? noCondition() : DSL.upper(v_office.OFFICE_ID).like(officeLike.toUpperCase())) .orderBy(v_clob.ID).limit(pageSize); From 723e38ffd59589c38a5996f83907e12ef889bfd6 Mon Sep 17 00:00:00 2001 From: Adam Korynta Date: Thu, 7 Nov 2024 13:22:50 -0800 Subject: [PATCH 2/5] remove unused methods --- .../main/java/cwms/cda/data/dao/AuthDao.java | 5 ---- .../main/java/cwms/cda/data/dao/BlobDao.java | 27 ------------------- .../src/main/java/cwms/cda/data/dao/Dao.java | 3 --- .../main/java/cwms/cda/data/dao/JooqDao.java | 5 ---- 4 files changed, 40 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/AuthDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/AuthDao.java index bc3673b27..6e57260a9 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/AuthDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/AuthDao.java @@ -138,11 +138,6 @@ public static AuthDao getInstance(DSLContext dsl) { return getInstance(dsl, null); } - @Override - public List getAll(String limitToOffice) { - throw new UnsupportedOperationException("Unimplemented method 'getAll'"); - } - /** * Reserved for future use, get user principal by presented unique name and office. * (Also required by Dao<DataApiPrincipal>) diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/BlobDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/BlobDao.java index c3ab3ecd4..c432f33a9 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/BlobDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/BlobDao.java @@ -110,33 +110,6 @@ private static void handleResultSet(ResultSet resultSet, BlobConsumer consumer) consumer.accept(blob, mediaType); } - - @Override - public List getAll(String officeId) { - String queryStr = "SELECT AT_BLOB.ID, AT_BLOB.DESCRIPTION, CWMS_MEDIA_TYPE.MEDIA_TYPE_ID, CWMS_OFFICE.OFFICE_ID\n" - + " FROM CWMS_20.AT_BLOB \n" - + "join CWMS_20.CWMS_MEDIA_TYPE on AT_BLOB.MEDIA_TYPE_CODE = CWMS_MEDIA_TYPE.MEDIA_TYPE_CODE \n" - + "join CWMS_20.CWMS_OFFICE on AT_BLOB.OFFICE_CODE=CWMS_OFFICE.OFFICE_CODE \n" - ; - - ResultQuery query; - if (officeId != null) { - queryStr = queryStr + " and upper(CWMS_OFFICE.OFFICE_ID) = upper(?)"; - query = dsl.resultQuery(queryStr, officeId); - } else { - query = dsl.resultQuery(queryStr); - } - - return query.fetch(r -> { - String rId = r.get("ID", String.class); - String rOffice = r.get("OFFICE_ID", String.class); - String rDesc = r.get("DESCRIPTION", String.class); - String rMedia = r.get("MEDIA_TYPE_ID", String.class); - - return new Blob(rOffice, rId, rDesc, rMedia, null); - }); - } - public List getAll(String officeId, String like) { String queryStr = "SELECT AT_BLOB.ID, AT_BLOB.DESCRIPTION, CWMS_MEDIA_TYPE.MEDIA_TYPE_ID, CWMS_OFFICE.OFFICE_ID\n" + " FROM CWMS_20.AT_BLOB \n" diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/Dao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/Dao.java index 34cecaa8a..1557e580e 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/Dao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/Dao.java @@ -107,9 +107,6 @@ protected void setOffice(Connection c, String office) throws SQLException { CWMS_ENV_PACKAGE.call_SET_SESSION_OFFICE_ID(DSL.using(c).configuration(), office); } - - public abstract List getAll(String office); - public abstract Optional getByUniqueName(String uniqueName, String office); } diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/JooqDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/JooqDao.java index cdbd10089..dcb18ed9e 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/JooqDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/JooqDao.java @@ -155,11 +155,6 @@ private static Connection setClientInfo(Context ctx, Connection connection) { return connection; } - @Override - public List getAll(String officeId) { - throw new UnsupportedOperationException("Not supported yet."); - } - @Override public Optional getByUniqueName(String uniqueName, String officeId) { throw new UnsupportedOperationException("Not supported yet."); From a9d602f8a7f18c1dbd2881e1cc03dc9d30d8ebb9 Mon Sep 17 00:00:00 2001 From: Adam Korynta Date: Thu, 7 Nov 2024 13:23:02 -0800 Subject: [PATCH 3/5] fix ordering and add integration test --- .../main/java/cwms/cda/data/dao/ClobDao.java | 81 +++++-------------- .../cwms/cda/api/ClobControllerTestIT.java | 53 +++++++++++- 2 files changed, 71 insertions(+), 63 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java index e166a44ad..270656fa4 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java @@ -42,29 +42,6 @@ public ClobDao(DSLContext dsl) { super(dsl); } - - // Yikes, I hate this method - it retrieves all the clobs? That could be gigabytes of data. - // Not returning Value or Desc fields until a useful way of working with this method is - // figured out. - @Override - public List getAll(String limitToOffice) { - AV_CLOB ac = AV_CLOB.AV_CLOB; - AV_OFFICE ao = AV_OFFICE.AV_OFFICE; - - Condition whereCond = noCondition(); - if (limitToOffice != null && !limitToOffice.isEmpty()) { - whereCond = ao.OFFICE_ID.eq(limitToOffice); - } - - return dsl.select(ac.ID, ao.OFFICE_ID) - .from(ac.join(ao).on(ac.OFFICE_CODE.eq(ao.OFFICE_CODE))) - .where(whereCond) - .fetch(joinRecord -> - new Clob(joinRecord.get(ao.OFFICE_ID), - joinRecord.get(ac.ID), null, null)); - - } - @Override public Optional getByUniqueName(String uniqueName, String office) { AV_CLOB ac = AV_CLOB.AV_CLOB; @@ -88,11 +65,6 @@ public Optional getByUniqueName(String uniqueName, String office) { .fetchOptional(mapper); } - public Clobs getClobs(String cursor, int pageSize, String officeLike, - boolean includeValues) { - return getClobs(cursor, pageSize, officeLike, includeValues, ".*"); - } - public Clobs getClobs(String cursor, int pageSize, String officeLike, boolean includeValues, String idRegex) { int total = 0; @@ -100,16 +72,17 @@ public Clobs getClobs(String cursor, int pageSize, String officeLike, AV_CLOB v_clob = AV_CLOB.AV_CLOB; AV_OFFICE v_office = AV_OFFICE.AV_OFFICE; + Condition whereClause = JooqDao.caseInsensitiveLikeRegex(v_clob.ID, idRegex) + .and(officeLike == null ? noCondition() : JooqDao.caseInsensitiveLikeRegex(v_office.OFFICE_ID, officeLike)); if (cursor == null || cursor.isEmpty()) { - - SelectConditionStep> count = - dsl.select(count(asterisk())) - .from(v_clob) - .join(v_office).on(v_clob.OFFICE_CODE.eq(v_office.OFFICE_CODE)) - .where(JooqDao.caseInsensitiveLikeRegex(v_clob.ID, idRegex)) - .and(officeLike == null ? noCondition() : DSL.upper(v_office.OFFICE_ID).like(officeLike.toUpperCase())); - - total = count.fetchOne().value1(); + SelectConditionStep> count = dsl.select(count(asterisk())) + .from(v_clob) + .join(v_office).on(v_clob.OFFICE_CODE.eq(v_office.OFFICE_CODE)) + .where(whereClause); + Record1 rec = count.fetchOne(); + if(rec != null) { + total = rec.value1(); + } } else { final String[] parts = CwmsDTOPaginated.decodeCursor(cursor, "||"); @@ -128,17 +101,17 @@ public Clobs getClobs(String cursor, int pageSize, String officeLike, } SelectLimitPercentStep> query = dsl.select( - v_office.OFFICE_ID, - v_clob.ID, - v_clob.DESCRIPTION, - includeValues ? v_clob.VALUE : DSL.inline("").as(v_clob.VALUE) - ) - .from(v_clob) - .join(v_office).on(v_clob.OFFICE_CODE.eq(v_office.OFFICE_CODE)) - .where(JooqDao.caseInsensitiveLikeRegex(v_clob.ID,idRegex)) - .and(DSL.upper(v_clob.ID).greaterThan(clobCursor)) - .and(officeLike == null ? noCondition() : DSL.upper(v_office.OFFICE_ID).like(officeLike.toUpperCase())) - .orderBy(v_clob.ID).limit(pageSize); + v_office.OFFICE_ID, + v_clob.ID, + v_clob.DESCRIPTION, + includeValues ? v_clob.VALUE : DSL.inline("").as(v_clob.VALUE) + ) + .from(v_clob) + .join(v_office).on(v_clob.OFFICE_CODE.eq(v_office.OFFICE_CODE)) + .where(whereClause) + .and(DSL.upper(v_clob.ID).greaterThan(clobCursor)) + .orderBy(v_office.OFFICE_ID, v_clob.ID) + .limit(pageSize); Clobs.Builder builder = new Clobs.Builder(clobCursor, pageSize, total); @@ -181,18 +154,6 @@ public List getClobsLike(String office, String idLike) { ac.join(ao).on(ac.OFFICE_CODE.eq(ao.OFFICE_CODE))).where(cond).fetch(mapper); } - public String getClobValue(String office, String id) { - AV_CLOB ac = AV_CLOB.AV_CLOB; - AV_OFFICE ao = AV_OFFICE.AV_OFFICE; - - Condition cond = ac.ID.eq(id).and(ao.OFFICE_ID.eq(office)); - - Record1 clobRecord = dsl.select(ac.VALUE).from( - ac.join(ao).on(ac.OFFICE_CODE.eq(ao.OFFICE_CODE))).where(cond).fetchOne(); - - return clobRecord.value1(); - } - public void create(Clob clob, boolean failIfExists) { String pFailIfExists = getBoolean(failIfExists); diff --git a/cwms-data-api/src/test/java/cwms/cda/api/ClobControllerTestIT.java b/cwms-data-api/src/test/java/cwms/cda/api/ClobControllerTestIT.java index c25c81ba5..808ea4016 100644 --- a/cwms-data-api/src/test/java/cwms/cda/api/ClobControllerTestIT.java +++ b/cwms-data-api/src/test/java/cwms/cda/api/ClobControllerTestIT.java @@ -1,9 +1,9 @@ package cwms.cda.api; import static io.restassured.RestAssured.given; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import cwms.cda.data.dto.Clob; import cwms.cda.formatters.Formats; @@ -13,7 +13,6 @@ import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import javax.servlet.http.HttpServletResponse; - import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -29,7 +28,7 @@ public class ClobControllerTestIT extends DataApiTestIT { private static final String EXISTING_CLOB_DESC = "test description"; @BeforeAll - static void createExistingClob() throws Exception + static void createExistingClobs() throws Exception { Clob clob = new Clob(SPK, EXISTING_CLOB_ID, EXISTING_CLOB_DESC, EXISTING_CLOB_VALUE); ObjectMapper om = JsonV2.buildObjectMapper(); @@ -51,6 +50,26 @@ static void createExistingClob() throws Exception .log().ifValidationFails(LogDetail.ALL,true) .assertThat() .statusCode(is(HttpServletResponse.SC_CREATED)); + //Need to verify that getAll filters to a specific office + user = TestAccounts.KeyUser.SWT_NORMAL; + clob = new Clob(user.getOperatingOffice(), EXISTING_CLOB_ID, EXISTING_CLOB_DESC, EXISTING_CLOB_VALUE); + serializedClob = om.writeValueAsString(clob); + given() + .log().ifValidationFails(LogDetail.ALL,true) + .accept(Formats.JSONV2) + .contentType(Formats.JSONV2) + .body(serializedClob) + .header("Authorization",user.toHeaderValue()) + .queryParam("office",SPK) + .queryParam("fail-if-exists",false) + .when() + .redirects().follow(true) + .redirects().max(3) + .post("/clobs/") + .then() + .log().ifValidationFails(LogDetail.ALL,true) + .assertThat() + .statusCode(is(HttpServletResponse.SC_CREATED)); } @Test @@ -131,6 +150,34 @@ void test_getOne_plain_text() .body( is(EXISTING_CLOB_VALUE)); } + @Test + void test_getAll_specific_office() + { + given() + .log().ifValidationFails(LogDetail.ALL,true) + .queryParam(Controllers.OFFICE, SPK) + .accept(Formats.JSON) + .when() + .get("/clobs/") + .then() + .log().ifValidationFails(LogDetail.ALL,true) + .assertThat() + .statusCode(is(HttpServletResponse.SC_OK)) + .body("clobs.office-id", hasItem(SPK)); + + given() + .log().ifValidationFails(LogDetail.ALL,true) + .queryParam(Controllers.OFFICE, TestAccounts.KeyUser.SWT_NORMAL.getOperatingOffice()) + .accept(Formats.JSON) + .when() + .get("/clobs/") + .then() + .log().ifValidationFails(LogDetail.ALL,true) + .assertThat() + .statusCode(is(HttpServletResponse.SC_OK)) + .body("clobs.office-id", hasItem(TestAccounts.KeyUser.SWT_NORMAL.getOperatingOffice())); + } + @ParameterizedTest @EnumSource(GetAllTest.class) From d6f48b6e1ea51161261460a345b1b072f5841f43 Mon Sep 17 00:00:00 2001 From: Adam Korynta Date: Fri, 8 Nov 2024 14:35:00 -0800 Subject: [PATCH 4/5] update to use office id in the clob cursor --- .../main/java/cwms/cda/data/dao/ClobDao.java | 17 ++++--- .../main/java/cwms/cda/data/dto/Clobs.java | 46 ++++++++++++++++--- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java b/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java index 270656fa4..70328b642 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dao/ClobDao.java @@ -68,12 +68,13 @@ public Optional getByUniqueName(String uniqueName, String office) { public Clobs getClobs(String cursor, int pageSize, String officeLike, boolean includeValues, String idRegex) { int total = 0; - String clobCursor = "*"; + String cursorOffice = null; + String cursorClobId = null; AV_CLOB v_clob = AV_CLOB.AV_CLOB; AV_OFFICE v_office = AV_OFFICE.AV_OFFICE; Condition whereClause = JooqDao.caseInsensitiveLikeRegex(v_clob.ID, idRegex) - .and(officeLike == null ? noCondition() : JooqDao.caseInsensitiveLikeRegex(v_office.OFFICE_ID, officeLike)); + .and(JooqDao.caseInsensitiveLikeRegexNullTrue(v_office.OFFICE_ID, officeLike)); if (cursor == null || cursor.isEmpty()) { SelectConditionStep> count = dsl.select(count(asterisk())) .from(v_clob) @@ -92,9 +93,8 @@ public Clobs getClobs(String cursor, int pageSize, String officeLike, } if (parts.length > 1) { - clobCursor = parts[0].split(";")[0]; - clobCursor = clobCursor.substring(clobCursor.indexOf("/") + 1); // ditch the - // officeId that's embedded in + cursorOffice = Clobs.getOffice(cursor); + cursorClobId = Clobs.getId(cursor); total = Integer.parseInt(parts[1]); pageSize = Integer.parseInt(parts[2]); } @@ -109,12 +109,15 @@ public Clobs getClobs(String cursor, int pageSize, String officeLike, .from(v_clob) .join(v_office).on(v_clob.OFFICE_CODE.eq(v_office.OFFICE_CODE)) .where(whereClause) - .and(DSL.upper(v_clob.ID).greaterThan(clobCursor)) + .and(cursorClobId == null ? DSL.noCondition() : + DSL.upper(v_clob.ID).greaterThan(cursorClobId.toUpperCase())) + .and(cursorOffice == null ? DSL.noCondition() : + DSL.upper(v_office.OFFICE_ID).greaterThan(cursorOffice.toUpperCase())) .orderBy(v_office.OFFICE_ID, v_clob.ID) .limit(pageSize); - Clobs.Builder builder = new Clobs.Builder(clobCursor, pageSize, total); + Clobs.Builder builder = new Clobs.Builder(cursor, pageSize, total); logger.atFine().log(query.getSQL(ParamType.INLINED)); diff --git a/cwms-data-api/src/main/java/cwms/cda/data/dto/Clobs.java b/cwms-data-api/src/main/java/cwms/cda/data/dto/Clobs.java index 14c143473..dd2dc5728 100644 --- a/cwms-data-api/src/main/java/cwms/cda/data/dto/Clobs.java +++ b/cwms-data-api/src/main/java/cwms/cda/data/dto/Clobs.java @@ -44,19 +44,53 @@ private void addClob(Clob clob) { clobs.add(clob); } + /** + * Extract the office from the cursor. + * + * @param cursor the cursor + * @return office + */ + public static String getOffice(String cursor) { + String[] parts = CwmsDTOPaginated.decodeCursor(cursor); + if (parts.length > 1) { + String[] idAndOffice = CwmsDTOPaginated.decodeCursor(parts[0]); + if (idAndOffice.length > 0) { + return idAndOffice[0]; + } + } + return null; + } + + /** + * Extract the id from the cursor. + * + * @param cursor the cursor + * @return id + */ + public static String getId(String cursor) { + String[] parts = CwmsDTOPaginated.decodeCursor(cursor); + if (parts.length > 1) { + String[] idAndOffice = CwmsDTOPaginated.decodeCursor(parts[0]); + if (idAndOffice.length > 1) { + return idAndOffice[1]; + } + } + return null; + } + public static class Builder { - private Clobs workingClobs; + private final Clobs workingClobs; public Builder(String cursor, int pageSize, int total) { workingClobs = new Clobs(cursor, pageSize, total); } public Clobs build() { - if (this.workingClobs.clobs.size() == this.workingClobs.pageSize) { - this.workingClobs.nextPage = encodeCursor( - this.workingClobs.clobs.get(this.workingClobs.clobs.size() - 1).toString().toUpperCase(), - this.workingClobs.pageSize, - this.workingClobs.total); + if (this.workingClobs.clobs.size() == this.workingClobs.pageSize && !this.workingClobs.clobs.isEmpty()) { + Clob lastClob = this.workingClobs.clobs.get(this.workingClobs.clobs.size() - 1); + String cursor = encodeCursor(CwmsDTOPaginated.delimiter, lastClob.getOfficeId(), + lastClob.getId()); + this.workingClobs.nextPage = encodeCursor(cursor, this.workingClobs.pageSize, this.workingClobs.total); } else { this.workingClobs.nextPage = null; } From a0a60977125ce187bdde0a33b5741561937ea6f9 Mon Sep 17 00:00:00 2001 From: Adam Korynta Date: Mon, 11 Nov 2024 09:45:29 -0800 Subject: [PATCH 5/5] fix failing unit test --- cwms-data-api/src/test/java/cwms/cda/data/dto/ClobTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cwms-data-api/src/test/java/cwms/cda/data/dto/ClobTest.java b/cwms-data-api/src/test/java/cwms/cda/data/dto/ClobTest.java index b8443bd97..785aa6143 100644 --- a/cwms-data-api/src/test/java/cwms/cda/data/dto/ClobTest.java +++ b/cwms-data-api/src/test/java/cwms/cda/data/dto/ClobTest.java @@ -102,7 +102,9 @@ void testRoundtripXML2Clobs(String format) { assertEquals(1, clobs2.getPageSize()); assertEquals(1, clobs2.getTotal()); assertEquals("Y3Vyc29yfHwxfHwx", clobs2.getPage()); - assertEquals("TVlPRkZJQ0UvTVlJRDtERVNDUklQVElPTj1NWURFU0N8fDF8fDE=", clobs2.getNextPage()); + assertEquals("VFZsUFJrWkpRMFY4ZkUxWlNVUT18fDF8fDE=", clobs2.getNextPage()); + assertEquals(clob.getOfficeId(), Clobs.getOffice(clobs2.getNextPage())); + assertEquals(clob.getId(), Clobs.getId(clobs2.getNextPage())); assertEquals(clob.getId(), clobs2.getClobs().get(0).getId()); assertEquals(clob.getOfficeId(), clobs2.getClobs().get(0).getOfficeId()); assertEquals(clob.getDescription(), clobs2.getClobs().get(0).getDescription());