fix bug: change sql-query in localArtifactRepository (#820)

Also-by: Nazife Basbaz <nazife.basbaz@bosch-si.com>
Signed-off-by: Ahmed Sayed <ahmed.sayed@bosch-si.com>
This commit is contained in:
Nazife Basbaz
2019-06-05 16:07:07 +02:00
committed by Jeroen Laverman
parent b8ca7d24c8
commit fde0cbdf16
31 changed files with 237 additions and 136 deletions

View File

@@ -40,6 +40,7 @@ import org.springframework.data.domain.Pageable;
import org.springframework.retry.annotation.Backoff;
import org.springframework.retry.annotation.Retryable;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.StringUtils;
import org.springframework.validation.annotation.Validated;
/**
@@ -98,20 +99,35 @@ public class JpaArtifactManagement implements ArtifactManagement {
public Artifact create(final ArtifactUpload artifactUpload) {
final String filename = artifactUpload.getFilename();
final long moduleId = artifactUpload.getModuleId();
AbstractDbArtifact result = null;
final SoftwareModule softwareModule = getModuleAndThrowExceptionIfThatFails(moduleId);
final Artifact existing = checkForExistingArtifact(filename, artifactUpload.overrideExisting(),
softwareModule);
final Artifact existing = checkForExistingArtifact(filename, artifactUpload.overrideExisting(), softwareModule);
assertArtifactQuota(moduleId, 1);
assertMaxArtifactSizeQuota(filename, moduleId, artifactUpload.getFilesize());
assertMaxArtifactStorageQuota(filename, artifactUpload.getFilesize());
return getOrCreateArtifact(artifactUpload)
.map(artifact -> storeArtifactMetadata(softwareModule, filename, artifact, existing))
.orElse(null);
}
private Optional<AbstractDbArtifact> getOrCreateArtifact(final ArtifactUpload artifactUpload) {
final String providedSha1Sum = artifactUpload.getProvidedSha1Sum();
AbstractDbArtifact artifact = null;
if (!StringUtils.isEmpty(providedSha1Sum)) {
artifact = artifactRepository.getArtifactBySha1(tenantAware.getCurrentTenant(), providedSha1Sum);
}
artifact = (artifact == null) ? storeArtifact(artifactUpload) : artifact;
return Optional.ofNullable(artifact);
}
private AbstractDbArtifact storeArtifact(final ArtifactUpload artifactUpload) {
try {
result = artifactRepository.store(tenantAware.getCurrentTenant(), artifactUpload.getInputStream(), filename,
artifactUpload.getContentType(),
return artifactRepository.store(tenantAware.getCurrentTenant(), artifactUpload.getInputStream(),
artifactUpload.getFilename(), artifactUpload.getContentType(),
new DbArtifactHash(artifactUpload.getProvidedSha1Sum(), artifactUpload.getProvidedMd5Sum()));
} catch (final ArtifactStoreException e) {
throw new ArtifactUploadFailedException(e);
@@ -122,11 +138,6 @@ public class JpaArtifactManagement implements ArtifactManagement {
throw new InvalidMD5HashException(e.getMessage(), e);
}
}
if (result == null) {
return null;
}
return storeArtifactMetadata(softwareModule, filename, result, existing);
}
private void assertArtifactQuota(final long id, final int requested) {
@@ -166,12 +177,12 @@ public class JpaArtifactManagement implements ArtifactManagement {
@Retryable(include = {
ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
public boolean clearArtifactBinary(final String sha1Hash, final long moduleId) {
if (localArtifactRepository.existsWithSha1HashAndSoftwareModuleIdIsNot(sha1Hash, moduleId)) {
final long count = localArtifactRepository.countBySha1HashAndTenantAndSoftwareModuleDeletedIsFalse(sha1Hash,
tenantAware.getCurrentTenant());
if (count > 1) {
// there are still other artifacts that need the binary
return false;
}
try {
LOG.debug("deleting artifact from repository {}", sha1Hash);
artifactRepository.deleteBySha1(tenantAware.getCurrentTenant(), sha1Hash);

View File

@@ -56,19 +56,18 @@ public interface LocalArtifactRepository extends BaseEntityRepository<JpaArtifac
List<Artifact> findBySha1Hash(String sha1Hash);
/**
* Verifies if an artifact exists that has given hash and is still related
* to a {@link SoftwareModule} other than a given one and not
* {@link SoftwareModule#isDeleted()}.
*
* Counts current elements based on the sha1 and tenant, as well as having
* the {@link SoftwareModule} property 'deleted' with value 'false
*
* @param sha1
* to search for
* @param moduleId
* to ignore in relationship check
* the sha1 of the {@link Artifact}
* @param tenant
* the current tenant
*
* @return <code>true</code> if such an artifact exists
* @return the count of the elements
*/
@Query("SELECT CASE WHEN COUNT(a)>0 THEN 'true' ELSE 'false' END FROM JpaArtifact a WHERE a.sha1Hash = :sha1 AND a.softwareModule.id != :moduleId AND a.softwareModule.deleted = 0")
boolean existsWithSha1HashAndSoftwareModuleIdIsNot(@Param("sha1") String sha1, @Param("moduleId") Long moduleId);
long countBySha1HashAndTenantAndSoftwareModuleDeletedIsFalse(@Param("sha1") String sha1,
@Param("tenant") String tenant);
/**
* Searches for a {@link Artifact} based on given gridFsFileName.

View File

@@ -469,7 +469,7 @@ public final class RSQLUtility {
// values JPA can do it by it's own but not for classes like enums.
// So we need to transform the given value string into the enum
// class.
final Class<? extends Object> javaType = fieldPath.getJavaType();
final Class<?> javaType = fieldPath.getJavaType();
if (javaType != null && javaType.isEnum()) {
return transformEnumValue(node, value, javaType);
}
@@ -485,7 +485,7 @@ public final class RSQLUtility {
}
private Object convertBooleanValue(final ComparisonNode node, final String value,
final Class<? extends Object> javaType) {
final Class<?> javaType) {
try {
return simpleTypeConverter.convertIfNecessary(value, javaType);
} catch (final TypeMismatchException e) {
@@ -513,7 +513,7 @@ public final class RSQLUtility {
// https://jira.sonarsource.com/browse/SONARJAVA-1478
@SuppressWarnings({ "rawtypes", "unchecked", "squid:S2095" })
private static Object transformEnumValue(final ComparisonNode node, final String value,
final Class<? extends Object> javaType) {
final Class<?> javaType) {
final Class<? extends Enum> tmpEnumType = (Class<? extends Enum>) javaType;
try {
return Enum.valueOf(tmpEnumType, value.toUpperCase());
@@ -680,13 +680,10 @@ public final class RSQLUtility {
final char escapeChar) {
final String escaped;
switch (database) {
case SQL_SERVER:
if (database == Database.SQL_SERVER) {
escaped = transformedValue.replace("%", "[%]").replace("_", "[_]");
break;
default:
} else {
escaped = transformedValue.replace("%", escapeChar + "%").replace("_", escapeChar + "_");
break;
}
return escaped.replace(LIKE_WILDCARD, '%');

View File

@@ -17,13 +17,15 @@ import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URISyntaxException;
import java.security.NoSuchAlgorithmException;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.Callable;
import javax.validation.ConstraintViolationException;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.RandomStringUtils;
import org.eclipse.hawkbit.artifact.repository.model.AbstractDbArtifact;
import org.eclipse.hawkbit.im.authentication.SpPermission;
import org.eclipse.hawkbit.repository.ArtifactManagement;
import org.eclipse.hawkbit.repository.event.remote.SoftwareModuleDeletedEvent;
@@ -38,6 +40,7 @@ import org.eclipse.hawkbit.repository.model.SoftwareModule;
import org.eclipse.hawkbit.repository.test.matcher.Expect;
import org.eclipse.hawkbit.repository.test.matcher.ExpectEvents;
import org.eclipse.hawkbit.repository.test.util.HashGeneratorUtils;
import org.eclipse.hawkbit.repository.test.util.WithSpringAuthorityRule;
import org.eclipse.hawkbit.repository.test.util.WithUser;
import org.junit.Test;
@@ -98,7 +101,7 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
@Test
@Description("Test if a local artifact can be created by API including metadata.")
public void createArtifact() throws NoSuchAlgorithmException, IOException {
public void createArtifact() throws IOException {
// check baseline
assertThat(softwareModuleRepository.findAll()).hasSize(0);
@@ -162,7 +165,7 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
@Test
@Description("Verifies that the quota specifying the maximum number of artifacts per software module is enforced.")
public void createArtifactsUntilQuotaIsExceeded() throws NoSuchAlgorithmException, IOException {
public void createArtifactsUntilQuotaIsExceeded() throws IOException {
// create a software module
final long smId = softwareModuleRepository.save(new JpaSoftwareModule(osType, "sm1", "1.0", null, null))
@@ -193,7 +196,7 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
@Test
@Description("Verifies that the quota specifying the maximum artifact storage is enforced (across software modules).")
public void createArtifactsUntilStorageQuotaIsExceeded() throws NoSuchAlgorithmException, IOException {
public void createArtifactsUntilStorageQuotaIsExceeded() throws IOException {
// create as many small artifacts as possible w/o violating the storage
// quota
@@ -224,7 +227,7 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
@Test
@Description("Verifies that the quota specifying the maximum artifact storage is enforced (across software modules).")
public void createArtifactWhichExceedsMaxStorage() throws NoSuchAlgorithmException, IOException {
public void createArtifactWhichExceedsMaxStorage() throws IOException {
// create one artifact which exceeds the storage quota at once
final long maxBytes = quotaManagement.getMaxArtifactStorage();
@@ -236,7 +239,7 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
@Test
@Description("Verifies that you cannot create artifacts which exceed the configured maximum size.")
public void createArtifactFailsIfTooLarge() throws NoSuchAlgorithmException, IOException {
public void createArtifactFailsIfTooLarge() {
// create a software module
final JpaSoftwareModule sm1 = softwareModuleRepository
@@ -250,7 +253,7 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
@Test
@Description("Tests hard delete directly on repository.")
public void hardDeleteSoftwareModule() throws NoSuchAlgorithmException, IOException {
public void hardDeleteSoftwareModule() throws IOException {
final JpaSoftwareModule sm = softwareModuleRepository
.save(new JpaSoftwareModule(osType, "name 1", "version 1", null, null));
@@ -268,12 +271,10 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
* .
*
* @throws IOException
* @throws NoSuchAlgorithmException
*/
@Test
@Description("Tests the deletion of a local artifact including metadata.")
public void deleteArtifact() throws NoSuchAlgorithmException, IOException {
public void deleteArtifact() throws IOException {
final JpaSoftwareModule sm = softwareModuleRepository
.save(new JpaSoftwareModule(osType, "name 1", "version 1", null, null));
final JpaSoftwareModule sm2 = softwareModuleRepository
@@ -304,6 +305,8 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
artifactManagement.delete(artifact1.getId());
assertThat(artifactRepository.findAll()).hasSize(1);
assertThat(
binaryArtifactRepository.getArtifactBySha1(tenantAware.getCurrentTenant(), artifact1.getSha1Hash()))
.isNull();
@@ -323,7 +326,7 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
@Test
@Description("Test the deletion of an artifact metadata where the binary is still linked to another "
+ "metadata element. The expected result is that the metadata is deleted but the binary kept.")
public void deleteDuplicateArtifacts() throws NoSuchAlgorithmException, IOException {
public void deleteDuplicateArtifacts() throws IOException {
final JpaSoftwareModule sm = softwareModuleRepository
.save(new JpaSoftwareModule(osType, "name 1", "version 1", null, null));
@@ -347,21 +350,92 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
assertThat(
binaryArtifactRepository.getArtifactBySha1(tenantAware.getCurrentTenant(), artifact1.getSha1Hash()))
.isNotNull();
artifactManagement.delete(artifact1.getId());
assertThat(
binaryArtifactRepository.getArtifactBySha1(tenantAware.getCurrentTenant(), artifact1.getSha1Hash()))
.isNotNull();
assertThat(artifactRepository.findAll()).hasSize(1);
assertThat(artifactRepository.existsById(artifact1.getId())).isFalse();
artifactManagement.delete(artifact2.getId());
assertThat(
binaryArtifactRepository.getArtifactBySha1(tenantAware.getCurrentTenant(), artifact1.getSha1Hash()))
.isNull();
assertThat(artifactRepository.findAll()).hasSize(0);
}
}
@Test
@Description("Verifies that you cannot delete an artifact which exists with the same hash, in the same tenant and the SoftwareModule is not deleted .")
public void deleteArtifactWithSameHashAndSoftwareModuleIsNotDeletedInSameTenants() throws IOException {
final JpaSoftwareModule sm = softwareModuleRepository
.save(new JpaSoftwareModule(osType, "name 1", "version 1", null, null));
final JpaSoftwareModule sm2 = softwareModuleRepository
.save(new JpaSoftwareModule(osType, "name 2", "version 2", null, null));
final int artifactSize = 5 * 1024;
final byte[] randomBytes = randomBytes(artifactSize);
try (final InputStream inputStream1 = new ByteArrayInputStream(randomBytes);
final InputStream inputStream2 = new ByteArrayInputStream(randomBytes)) {
final Artifact artifact1 = createArtifactForSoftwareModule("file1", sm.getId(), artifactSize, inputStream1);
final Artifact artifact2 = createArtifactForSoftwareModule("file2", sm2.getId(), artifactSize,
inputStream2);
assertEqualFileContents(artifactManagement.loadArtifactBinary(artifact2.getSha1Hash()), randomBytes);
assertThat(artifactRepository.findAll()).hasSize(2);
assertThat(artifact1.getSha1Hash()).isEqualTo(artifact2.getSha1Hash());
assertThat(artifactRepository.countBySha1HashAndTenantAndSoftwareModuleDeletedIsFalse(
artifact1.getSha1Hash(), artifact1.getTenant())).isGreaterThan(1);
artifactRepository.deleteById(artifact1.getId());
assertThat(artifactRepository.findAll()).hasSize(1);
assertThat(artifactRepository.countBySha1HashAndTenantAndSoftwareModuleDeletedIsFalse(
artifact2.getSha1Hash(), artifact2.getTenant())).isLessThanOrEqualTo(1);
artifactRepository.deleteById(artifact2.getId());
assertThat(artifactRepository.findAll()).hasSize(0);
}
}
@Test
@Description("Verifies that you can not delete artifacts from another tenant which exists in another tenant with the same hash and the SoftwareModule is not deleted")
public void deleteArtifactWithSameHashAndSoftwareModuleIsNotDeletedInDifferentTenants() throws Exception {
final String tenant1 = "mytenant";
final String tenant2 = "tenant2";
final SoftwareModule module = createSoftwareModuleForTenant(tenant1);
final SoftwareModule module2 = createSoftwareModuleForTenant(tenant2);
createArtifactForTenant(tenant1, "myInput", module.getId(), "myFirstFile");
createArtifactForTenant(tenant2, "myInput", module2.getId(), "mySecondFile");
final Artifact artifactTenant2 = createArtifactForTenant(tenant2, "myInput", module2.getId(), "myThirdFile");
verifyTenantArtifactCountIs(tenant1, 1);
verifyTenantArtifactCountIs(tenant2, 2);
assertThat(runAsTenant(tenant1, () -> artifactRepository
.countBySha1HashAndTenantAndSoftwareModuleDeletedIsFalse(artifactTenant2.getSha1Hash(), tenant2)))
.isLessThanOrEqualTo(1);
runAsTenant(tenant2, () -> {
artifactRepository.deleteById(artifactTenant2.getId());
return null;
});
verifyTenantArtifactCountIs(tenant1, 1);
verifyTenantArtifactCountIs(tenant2, 1);
}
@Test
@Description("Loads an local artifact based on given ID.")
public void findArtifact() throws NoSuchAlgorithmException, IOException {
public void findArtifact() throws IOException {
final int artifactSize = 5 * 1024;
try (final InputStream inputStream = new RandomGeneratedInputStream(artifactSize)) {
final Artifact artifact = createArtifactForSoftwareModule("file1",
@@ -372,17 +446,13 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
@Test
@Description("Loads an artifact binary based on given ID.")
public void loadStreamOfArtifact() throws NoSuchAlgorithmException, IOException {
public void loadStreamOfArtifact() throws IOException {
final int artifactSize = 5 * 1024;
final byte[] randomBytes = randomBytes(artifactSize);
try (final InputStream input = new ByteArrayInputStream(randomBytes)) {
final Artifact artifact = createArtifactForSoftwareModule("file1",
testdataFactory.createSoftwareModuleOs().getId(), artifactSize, input);
try (final InputStream inputStream = artifactManagement.loadArtifactBinary(artifact.getSha1Hash()).get()
.getFileInputStream()) {
assertTrue("The stored binary matches the given binary",
IOUtils.contentEquals(new ByteArrayInputStream(randomBytes), inputStream));
}
assertEqualFileContents(artifactManagement.loadArtifactBinary(artifact.getSha1Hash()), randomBytes);
}
}
@@ -425,7 +495,6 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
createArtifactForSoftwareModule("file2", sm.getId(), artifactSize, inputStream2);
assertThat(artifactManagement.getByFilenameAndSoftwareModule("file1", sm.getId())).isPresent();
}
}
private Artifact createArtifactForSoftwareModule(final String filename, final long moduleId, final int artifactSize)
@@ -437,11 +506,38 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
}
private Artifact createArtifactForSoftwareModule(final String filename, final long moduleId, final int artifactSize,
final InputStream inputStream) throws IOException {
final InputStream inputStream) {
return artifactManagement.create(new ArtifactUpload(inputStream, moduleId, filename, false, artifactSize));
}
private static byte[] randomBytes(final int len) {
return RandomStringUtils.randomAlphanumeric(len).getBytes();
}
private <T> T runAsTenant(final String tenant, final Callable<T> callable) throws Exception {
return securityRule.runAs(WithSpringAuthorityRule.withUserAndTenant("user", tenant), callable);
}
private SoftwareModule createSoftwareModuleForTenant(final String tenant) throws Exception {
return runAsTenant(tenant, () -> testdataFactory.createSoftwareModuleApp());
}
private Artifact createArtifactForTenant(final String tenant, final String artifactData, final long moduleId,
final String filename) throws Exception {
return runAsTenant(tenant, () -> testdataFactory.createArtifact(artifactData, moduleId, filename));
}
private void verifyTenantArtifactCountIs(final String tenant, final int count) throws Exception {
assertThat(runAsTenant(tenant, () -> artifactRepository.findAll())).hasSize(count);
}
private void assertEqualFileContents(final Optional<AbstractDbArtifact> artifact, final byte[] randomBytes)
throws IOException {
try (final InputStream inputStream = artifactManagement.loadArtifactBinary(artifact.get().getHashes().getSha1())
.get().getFileInputStream()) {
assertTrue("The stored binary matches the given binary",
IOUtils.contentEquals(new ByteArrayInputStream(randomBytes), inputStream));
}
}
}

View File

@@ -13,6 +13,7 @@ import static org.assertj.core.api.Assertions.fail;
import java.util.Arrays;
import java.util.Collection;
import java.util.concurrent.Callable;
import org.eclipse.hawkbit.repository.exception.EntityNotFoundException;
import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest;
@@ -59,7 +60,6 @@ public class MultiTenancyEntityTest extends AbstractJpaIntegrationTest {
assertThat(findTargetsForAnotherTenant).hasSize(1);
assertThat(findTargetsForAnotherTenant.getContent().get(0).getTenant().toUpperCase())
.isEqualTo(anotherTenant.toUpperCase());
}
@Test
@@ -163,34 +163,32 @@ public class MultiTenancyEntityTest extends AbstractJpaIntegrationTest {
assertThat(findDistributionSetsForAnotherTenant).hasSize(1);
assertThat(findDistributionSetsForAnotherTenant.getContent().get(0).getTenant().toUpperCase())
.isEqualTo(anotherTenant.toUpperCase());
}
private <T> T runAsTenant(final String tenant, final Callable<T> callable) throws Exception {
return securityRule.runAs(WithSpringAuthorityRule.withUserAndTenant("user", tenant), callable);
}
private Target createTargetForTenant(final String controllerId, final String tenant) throws Exception {
return securityRule.runAs(WithSpringAuthorityRule.withUserAndTenant("user", tenant),
() -> testdataFactory.createTarget(controllerId));
return runAsTenant(tenant, () -> testdataFactory.createTarget(controllerId));
}
private Slice<Target> findTargetsForTenant(final String tenant) throws Exception {
return securityRule.runAs(WithSpringAuthorityRule.withUserAndTenant("user", tenant),
() -> targetManagement.findAll(PAGE));
return runAsTenant(tenant, () -> targetManagement.findAll(PAGE));
}
private void deleteTargetsForTenant(final String tenant, final Collection<Long> targetIds) throws Exception {
securityRule.runAs(WithSpringAuthorityRule.withUserAndTenant("user", tenant), () -> {
runAsTenant(tenant, () -> {
targetManagement.delete(targetIds);
return null;
});
}
private DistributionSet createDistributionSetForTenant(final String tenant) throws Exception {
return securityRule.runAs(WithSpringAuthorityRule.withUserAndTenant("user", tenant),
() -> testdataFactory.createDistributionSet());
return runAsTenant(tenant, () -> testdataFactory.createDistributionSet());
}
private Page<DistributionSet> findDistributionSetForTenant(final String tenant) throws Exception {
return securityRule.runAs(WithSpringAuthorityRule.withUserAndTenant("user", tenant),
() -> distributionSetManagement.findByCompleted(PAGE, true));
return runAsTenant(tenant, () -> distributionSetManagement.findByCompleted(PAGE, true));
}
}