From 91a359ca0aaa09d20d3932559a69e29d2ead601e Mon Sep 17 00:00:00 2001 From: Eric Wittmann Date: Mon, 21 Mar 2022 18:00:31 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20when=20getting=20an=20artifact=20version?= =?UTF-8?q?=20by=20content=20the=20SQL=20was=20failing=20=E2=80=A6=20(#235?= =?UTF-8?q?8)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: when getting an artifact version by content the SQL was failing if the artifact had multiple versions with the same matching content. Fixes #2357 * Ensure that the versions are ordered by globalId for consistent results in all DBs --- .../impl/sql/AbstractSqlRegistryStorage.java | 2 +- .../storage/impl/sql/CommonSqlStatements.java | 4 ++-- .../storage/impl/sql/jdb/MappedQuery.java | 2 ++ .../storage/impl/sql/jdb/MappedQueryImpl.java | 21 ++++++++++++++++ .../registry/rbac/RegistryClientTest.java | 24 +++++++++++++++++++ 5 files changed, 50 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractSqlRegistryStorage.java b/app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractSqlRegistryStorage.java index a8762be017..7dc138e866 100644 --- a/app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractSqlRegistryStorage.java +++ b/app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractSqlRegistryStorage.java @@ -1281,7 +1281,7 @@ public ArtifactVersionMetaDataDto getArtifactVersionMetaData(String groupId, Str .bind(2, artifactId) .bind(3, hash) .map(ArtifactVersionMetaDataDtoMapper.instance) - .findOne(); + .findLast(); return res.orElseThrow(() -> new ArtifactNotFoundException(groupId, artifactId)); }); } catch (ArtifactNotFoundException e) { diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/sql/CommonSqlStatements.java b/app/src/main/java/io/apicurio/registry/storage/impl/sql/CommonSqlStatements.java index 3135e3843e..2c648baa05 100644 --- a/app/src/main/java/io/apicurio/registry/storage/impl/sql/CommonSqlStatements.java +++ b/app/src/main/java/io/apicurio/registry/storage/impl/sql/CommonSqlStatements.java @@ -207,7 +207,7 @@ public String selectArtifactVersionMetaDataByContentHash() { return "SELECT v.*, a.type FROM versions v " + "JOIN content c ON v.contentId = c.contentId AND v.tenantId = c.tenantId " + "JOIN artifacts a ON v.tenantId = a.tenantId AND v.groupId = a.groupId AND v.artifactId = a.artifactId " - + "WHERE v.tenantId = ? AND v.groupId = ? AND v.artifactId = ? AND c.contentHash = ?"; + + "WHERE v.tenantId = ? AND v.groupId = ? AND v.artifactId = ? AND c.contentHash = ? ORDER BY v.globalId"; } @Override @@ -226,7 +226,7 @@ public String selectArtifactVersionMetaDataByCanonicalHash() { return "SELECT v.*, a.type FROM versions v " + "JOIN content c ON v.contentId = c.contentId AND v.tenantId = c.tenantId " + "JOIN artifacts a ON v.tenantId = a.tenantId AND v.groupId = a.groupId AND v.artifactId = a.artifactId " - + "WHERE v.tenantId = ? AND v.groupId = ? AND v.artifactId = ? AND c.canonicalHash = ?"; + + "WHERE v.tenantId = ? AND v.groupId = ? AND v.artifactId = ? AND c.canonicalHash = ? ORDER BY v.globalId"; } /** diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/sql/jdb/MappedQuery.java b/app/src/main/java/io/apicurio/registry/storage/impl/sql/jdb/MappedQuery.java index 77a66baf3e..2b9d5a4e78 100644 --- a/app/src/main/java/io/apicurio/registry/storage/impl/sql/jdb/MappedQuery.java +++ b/app/src/main/java/io/apicurio/registry/storage/impl/sql/jdb/MappedQuery.java @@ -33,6 +33,8 @@ public interface MappedQuery { public Optional findFirst(); + public Optional findLast(); + public List list(); public Stream stream(); diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/sql/jdb/MappedQueryImpl.java b/app/src/main/java/io/apicurio/registry/storage/impl/sql/jdb/MappedQueryImpl.java index 04c50412df..025850e408 100644 --- a/app/src/main/java/io/apicurio/registry/storage/impl/sql/jdb/MappedQueryImpl.java +++ b/app/src/main/java/io/apicurio/registry/storage/impl/sql/jdb/MappedQueryImpl.java @@ -136,6 +136,27 @@ public Optional findFirst() { return rval; } + /** + * @see io.apicurio.registry.storage.impl.sql.jdb.MappedQuery#findLast() + */ + @Override + public Optional findLast() { + Optional rval = null; + try { + while (this.resultSet.next()) { + rval = Optional.of(this.mapper.map(resultSet)); + } + if (rval == null) { + rval = Optional.empty(); + } + } catch (SQLException e) { + throw new RuntimeSqlException(e); + } finally { + close(); + } + return rval; + } + /** * @see io.apicurio.registry.storage.impl.sql.jdb.MappedQuery#list() */ diff --git a/app/src/test/java/io/apicurio/registry/rbac/RegistryClientTest.java b/app/src/test/java/io/apicurio/registry/rbac/RegistryClientTest.java index 8d6c2af48a..7d9c70610c 100644 --- a/app/src/test/java/io/apicurio/registry/rbac/RegistryClientTest.java +++ b/app/src/test/java/io/apicurio/registry/rbac/RegistryClientTest.java @@ -19,6 +19,7 @@ import static io.apicurio.registry.utils.tests.TestUtils.retry; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -1509,4 +1510,27 @@ public void testClientRateLimitError() { } } + @Test + public void testGetArtifactVersionByContent_DuplicateContent() throws Exception { + //Preparation + final String groupId = "testGetArtifactVersionByContent_DuplicateContent"; + final String artifactId = generateArtifactId(); + + final ArtifactMetaData v1md = createArtifact(groupId, artifactId); + + final InputStream v2stream = IoUtil.toStream(ARTIFACT_CONTENT.getBytes(StandardCharsets.UTF_8)); + final VersionMetaData v2md = clientV2.createArtifactVersion(groupId, artifactId, null, v2stream); + + //Execution + final VersionMetaData vmd = clientV2.getArtifactVersionMetaDataByContent(groupId, artifactId, IoUtil.toStream(ARTIFACT_CONTENT.getBytes())); + + //Assertions + assertNotEquals(v1md.getGlobalId(), v2md.getGlobalId()); + assertEquals(v1md.getContentId(), v2md.getContentId()); + + assertEquals(v2md.getGlobalId(), vmd.getGlobalId()); + assertEquals(v2md.getId(), vmd.getId()); + assertEquals(v2md.getContentId(), vmd.getContentId()); + } + }