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

Cleaner will remove artifacts without reference in suites e.g after rerun one test #336

Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a8de89b
improve rerun to remove orphan artifacts
plutasnyy Aug 23, 2018
ba2cd9d
Merge remote-tracking branch 'origin/feature/rerun' into feature/clea…
plutasnyy Aug 23, 2018
53d9d97
cleaning in cleaner after tests
plutasnyy Aug 23, 2018
ee4c2ad
changed returened value by arftifacts dao mock in get artifacts ids
plutasnyy Aug 24, 2018
b5e85cb
added description to getArtifactsId in interfac and changed _id on ID…
plutasnyy Aug 24, 2018
f8d728d
removed unused functionality
plutasnyy Aug 24, 2018
903b24b
craeted tests for remove artifacts processor test
plutasnyy Aug 27, 2018
8f07000
add default constructor for artifatsDao in remove artifacts processor
plutasnyy Aug 27, 2018
cd9f968
fixed bug with removing all suites exclude first
plutasnyy Aug 27, 2018
130a069
changed way to agregating suites in cleaner, removed one step in router
plutasnyy Aug 27, 2018
94ded09
Merge remote-tracking branch 'origin/feature/rerun' into feature/clea…
plutasnyy Aug 27, 2018
03b8ea3
changed information in log
plutasnyy Aug 27, 2018
2d03479
updated documentation
plutasnyy Aug 27, 2018
05dbffd
removed constructor from SuiteAggregationCounter, added and removed b…
plutasnyy Aug 28, 2018
3e0303d
removed blank line
plutasnyy Aug 28, 2018
3115a95
Merge branch 'feature/rerun' into feature/cleaner-removes-artifacts-a…
plutasnyy Aug 28, 2018
4bfedf0
renamed getArtifactsIds
plutasnyy Aug 28, 2018
1e367f6
renamed shouldBeKept
plutasnyy Aug 28, 2018
af28aa4
renamed parameter of setArftifactsIdToKeep in remove arftifacts proce…
plutasnyy Aug 28, 2018
47acf66
removed line
plutasnyy Aug 28, 2018
c1e494b
changed way to count suites for aggregate
plutasnyy Aug 28, 2018
dfd2e0d
updated changelog
plutasnyy Aug 28, 2018
e5f88e5
updated diagram
plutasnyy Aug 28, 2018
72aa7fa
Merge remote-tracking branch 'origin/master' into feature/cleaner-rem…
plutasnyy Sep 4, 2018
278227b
Merge remote-tracking branch 'origin/master' into feature/cleaner-rem…
plutasnyy Sep 17, 2018
05a60cd
removed EMPTY SET from remove artifacts procesor tests
plutasnyy Sep 17, 2018
f20cf28
Merge branch 'feature/rerun' into feature/cleaner-removes-artifacts-a…
plutasnyy Sep 17, 2018
87fede7
Merge remote-tracking branch 'origin/master' into feature/cleaner-rem…
plutasnyy Oct 2, 2018
1878579
merge with milestone/rerun, fixed conflicts
plutasnyy Oct 2, 2018
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
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ public interface ArtifactsDAO extends Serializable {
*/
String getArtifactAsString(DBKey dbKey, String objectID) throws IOException;

/**
* @param dbKey - key with project and company name
* @return Set of all artifacts id contained in database as String set or empty set
*/
Set<String> getArtifactsId(DBKey dbKey);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getArtifactsIds

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


/**
* @param dbKey - key with project and company name
* @param objectID - suite run identificator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@ public class SuiteAggregationCounter {

private final int suitesToAggregate;

public SuiteAggregationCounter(int suitesToAggregate) {
this.suitesToAggregate = suitesToAggregate;
public SuiteAggregationCounter() {
this.suitesToAggregate = 0;
}

public int getSuitesToAggregate() {
return suitesToAggregate;
}

public int addSuitesToAggregate(int suitesToAggregate) {
return this.suitesToAggregate + suitesToAggregate;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be here += ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.cognifide.aet.cleaner.processors;

import com.cognifide.aet.cleaner.context.CleanerContext;
import com.cognifide.aet.cleaner.context.SuiteAggregationCounter;
import com.cognifide.aet.cleaner.processors.exchange.AllSuiteVersionsMessageBody;
import com.cognifide.aet.communication.api.metadata.Suite;
import com.cognifide.aet.vs.DBKey;
Expand Down Expand Up @@ -73,6 +74,7 @@ public AllSuiteVersionsMessageBody apply(Map.Entry<String, Collection<Suite>> in
}).toList();

exchange.getOut().setBody(body);
exchange.getOut().setHeader(SuiteAggregationCounter.NAME_KEY, new SuiteAggregationCounter());
exchange.getOut().setHeader(CleanerContext.KEY_NAME, cleanerContext);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ public void process(Exchange exchange) throws Exception {
ReferencedArtifactsMessageBody body = new ReferencedArtifactsMessageBody(
messageBody.getData().getName(),
messageBody.getDbKey());
if (messageBody.shouldBeRemoved()) {
body.setArtifactsToRemove(metatadaArtifacts);
} else {
if (messageBody.shouldBeKeeped()) {
body.setArtifactsToKeep(metatadaArtifacts);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
import com.cognifide.aet.cleaner.processors.exchange.ReferencedArtifactsMessageBody;
import com.cognifide.aet.vs.ArtifactsDAO;
import com.google.common.collect.Sets;
import com.google.common.collect.Sets.SetView;
import java.util.ArrayList;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.camel.Exchange;
import org.apache.camel.Processor;
import org.osgi.service.component.annotations.Component;
Expand All @@ -34,6 +39,15 @@ public class RemoveArtifactsProcessor implements Processor {
@Reference
private ArtifactsDAO artifactsDAO;

public RemoveArtifactsProcessor() {
//default constructor
}

// for unit tests
public RemoveArtifactsProcessor(ArtifactsDAO artifactsDAO) {
this.artifactsDAO = artifactsDAO;
}

@Override
@SuppressWarnings("unchecked")
public void process(Exchange exchange) throws Exception {
Expand All @@ -42,8 +56,7 @@ public void process(Exchange exchange) throws Exception {
final ReferencedArtifactsMessageBody messageBody = exchange.getIn()
.getBody(ReferencedArtifactsMessageBody.class);

final Sets.SetView<String> artifactsToRemove =
Sets.difference(messageBody.getArtifactsToRemove(), messageBody.getArtifactsToKeep());
Set<String> artifactsToRemove = getArtifactsIdsToRemove(artifactsDAO, messageBody);

LOGGER.debug("Artifacts that will be removed: {}", artifactsToRemove);
if (!cleanerContext.isDryRun()) {
Expand All @@ -57,4 +70,11 @@ public void process(Exchange exchange) throws Exception {
artifactsToRemove.size(), messageBody.getDbKey(), messageBody.getData());
}
}

public static Set<String> getArtifactsIdsToRemove(ArtifactsDAO artifactsDAO,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't have to be public.

ReferencedArtifactsMessageBody messageBody) {
Set<String> artifactsToRemove = artifactsDAO.getArtifactsId(messageBody.getDbKey());
artifactsToRemove.removeAll(messageBody.getArtifactsToKeep());
return artifactsToRemove;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ public void process(Exchange exchange) throws Exception {
final Collection<Suite> suiteVersions = allSuiteVersions.getAllVersions();
final DBKey dbKey = allSuiteVersions.getDbKey();

SuiteAggregationCounter suiteAggregationCounter = exchange.getIn()
.getHeader(SuiteAggregationCounter.NAME_KEY, SuiteAggregationCounter.class);
suiteAggregationCounter.addSuitesToAggregate(suiteVersions.size());

LOGGER.info("Processing suite `{}` with {} version(s) in {}", allSuiteVersions.getData(),
suiteVersions.size(), dbKey);

Expand All @@ -65,7 +69,7 @@ public SuiteMessageBody apply(Suite suite) {

exchange.getOut().setBody(body);
exchange.getOut().setHeader(SuiteAggregationCounter.NAME_KEY,
new SuiteAggregationCounter(suiteVersions.size()));
suiteAggregationCounter);
exchange.getOut().setHeader(CleanerContext.KEY_NAME, cleanerContext);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@

public class ReferencedArtifactsMessageBody extends MessageBody<String> {

private static final long serialVersionUID = 3811933888080393100L;

private Set<String> artifactsToRemove;
private static final long serialVersionUID = 3748474548512567069L;

private Set<String> artifactsToKeep;

Expand All @@ -36,15 +34,6 @@ public String getId() {
return getDbKey().getCompany() + "_" + getDbKey().getProject() + "|" + getData();
}

public Set<String> getArtifactsToRemove() {
return artifactsToRemove != null ? Collections.unmodifiableSet(artifactsToRemove)
: Collections.<String>emptySet();
}

public void setArtifactsToRemove(Set<String> artifactsToRemove) {
this.artifactsToRemove = artifactsToRemove;
}

public Set<String> getArtifactsToKeep() {
return artifactsToKeep != null ? Collections.unmodifiableSet(artifactsToKeep)
: Collections.<String>emptySet();
Expand All @@ -55,11 +44,6 @@ public void setArtifactsToKeep(Set<String> artifactsToKeep) {
}

public void update(ReferencedArtifactsMessageBody body) {
if (artifactsToRemove == null) {
artifactsToRemove = new HashSet<>();
}
artifactsToRemove.addAll(body.getArtifactsToRemove());

if (artifactsToKeep == null) {
artifactsToKeep = new HashSet<>();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public SuiteMessageBody(Suite data, DBKey dbKey, boolean toRemove) {
this.toRemove = toRemove;
}

public boolean shouldBeRemoved() {
return toRemove;
public boolean shouldBeKeeped() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldBeKept()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return !toRemove;
}

public Set<String> getSuiteArtifacts() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void configure() throws Exception {
.process(suitesRemovePredicateProcessor)
.split(body())
.choice()
.when(body().method("shouldBeRemoved").isEqualTo(true)).to(direct("removeMetadata"))
.when(body().method("shouldBeKeeped").isEqualTo(false)).to(direct("removeMetadata"))
.otherwise().to(direct("getMetadataArtifacts"))
.endChoice();

Expand All @@ -86,7 +86,7 @@ public void configure() throws Exception {
.to(direct(AGGREGATE_SUITES_STEP));

from(direct(AGGREGATE_SUITES_STEP))
.aggregate(body().method("getId"), new SuitesAggregationStrategy())
.aggregate(body().method("getDbKey"), new SuitesAggregationStrategy())
.completionSize(header(SuiteAggregationCounter.NAME_KEY).method("getSuitesToAggregate"))
.completionTimeout(60000L).forceCompletionOnStop()
.to(direct("removeArtifacts"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,6 @@ private boolean isFirstAggregation(Exchange oldExchange) {
public void onCompletion(Exchange exchange) {
ReferencedArtifactsMessageBody body = exchange.getIn()
.getBody(ReferencedArtifactsMessageBody.class);
LOGGER.debug("Finished aggregating {}", body.getId());
LOGGER.debug("Finished aggregating {}", body.getData());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
/**
* AET
*
* Copyright (C) 2013 Cognifide Limited
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package com.cognifide.aet.cleaner.processors;

import static org.junit.Assert.assertEquals;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.cognifide.aet.cleaner.context.CleanerContext;
import com.cognifide.aet.cleaner.processors.exchange.ReferencedArtifactsMessageBody;
import com.cognifide.aet.vs.ArtifactsDAO;
import com.cognifide.aet.vs.DBKey;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import org.apache.camel.CamelContext;
import org.apache.camel.Exchange;
import org.apache.camel.Message;
import org.apache.camel.impl.DefaultCamelContext;
import org.apache.camel.impl.DefaultExchange;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;

@RunWith(MockitoJUnitRunner.class)
public class RemoveArtifactsProcessorTest {

private static final Set<String> ONE_TO_SEVEN_ARTIFACTS_ID_SET = new HashSet<>(Arrays
.asList("1", "2", "3", "4", "5", "6", "7"));

private static final Set<String> ONE_TO_FIVE_ARTIFACTS_ID_SET = new HashSet<>(Arrays
.asList("1", "2", "3", "4", "5"));

private static final Set<String> SIX_TO_SEVEN_ARTIFACTS_ID_SET = new HashSet<>(Arrays
.asList("6", "7"));

private static final Set<String> EMPTY_ARTIFACTS_ID_SET = new HashSet<>();

private Exchange exchange;

@Mock
private ArtifactsDAO artifactDAO;

@Mock
private CleanerContext cleanerContext;

@Mock
private DBKey dbKey;

private RemoveArtifactsProcessor removeArtifactsProcessor;

@Before
public void setUp() {
when(cleanerContext.isDryRun()).thenReturn(false);

CamelContext ctx = new DefaultCamelContext();
exchange = new DefaultExchange(ctx);

Message in = exchange.getIn();
in.setBody(new ReferencedArtifactsMessageBody("", dbKey));
in.setHeader(CleanerContext.KEY_NAME, cleanerContext);
}

@Test
public void check_ifRemoveArtifactsWasCalled_expectTrue() throws Exception {
when(artifactDAO.getArtifactsId(any(DBKey.class)))
.thenReturn(new HashSet<>(ONE_TO_SEVEN_ARTIFACTS_ID_SET));
setArtifactsIdToKeep(ONE_TO_FIVE_ARTIFACTS_ID_SET);

removeArtifactsProcessor = new RemoveArtifactsProcessor(artifactDAO);
removeArtifactsProcessor.process(exchange);

verify(artifactDAO, times(1)).removeArtifacts(any(DBKey.class), any(Set.class));
verify(artifactDAO, times(1)).getArtifactsId(any(DBKey.class));
}

@Test
public void check_ifRemoveArtifactsWasCalled_expectFalse() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are testing dryRun here, please add it to the name of this test.

when(cleanerContext.isDryRun()).thenReturn(true);
when(artifactDAO.getArtifactsId(any(DBKey.class)))
.thenReturn(new HashSet<>(ONE_TO_SEVEN_ARTIFACTS_ID_SET));
setArtifactsIdToKeep(ONE_TO_FIVE_ARTIFACTS_ID_SET);

removeArtifactsProcessor = new RemoveArtifactsProcessor(artifactDAO);
removeArtifactsProcessor.process(exchange);

verify(artifactDAO, times(0)).removeArtifacts(any(DBKey.class), any(Set.class));
verify(artifactDAO, times(1)).getArtifactsId(any(DBKey.class));
}

@Test
public void check_substractArtifactsSets_expectSetOfTwoVariables() throws Exception {
when(artifactDAO.getArtifactsId(any(DBKey.class)))
.thenReturn(new HashSet<>(ONE_TO_SEVEN_ARTIFACTS_ID_SET));
setArtifactsIdToKeep(ONE_TO_FIVE_ARTIFACTS_ID_SET);
ReferencedArtifactsMessageBody messageBody = (ReferencedArtifactsMessageBody) exchange.getIn()
.getBody();
assertEquals(SIX_TO_SEVEN_ARTIFACTS_ID_SET,
removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody));
}

@Test
public void check_substractArtifactsSets_expectEmptySet() throws Exception {
when(artifactDAO.getArtifactsId(any(DBKey.class)))
.thenReturn(new HashSet<>(ONE_TO_FIVE_ARTIFACTS_ID_SET));
setArtifactsIdToKeep(ONE_TO_SEVEN_ARTIFACTS_ID_SET);
ReferencedArtifactsMessageBody messageBody = (ReferencedArtifactsMessageBody) exchange.getIn()
.getBody();
assertEquals(EMPTY_ARTIFACTS_ID_SET,
removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be easier to just check if removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody) is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm checking it there: https://github.com/Cognifide/aet/pull/336/files#diff-4b628417ba96f36a0a4eec9f4817ba27R144. Here I wanted to check substraction ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant:
assertTrue(removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody).isEmpty()) :)

}

@Test
public void check_substractArtifactsSets_expectSetOfFiveVariables() throws Exception {
when(artifactDAO.getArtifactsId(any(DBKey.class)))
.thenReturn(new HashSet<>(ONE_TO_SEVEN_ARTIFACTS_ID_SET));
setArtifactsIdToKeep(SIX_TO_SEVEN_ARTIFACTS_ID_SET);

ReferencedArtifactsMessageBody messageBody = (ReferencedArtifactsMessageBody) exchange.getIn()
.getBody();

assertEquals(ONE_TO_FIVE_ARTIFACTS_ID_SET,
removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody));
}

@Test
public void check_substractArtifactsSetsWhenDbIsEmpty_expectEmptySet() throws Exception {
when(artifactDAO.getArtifactsId(any(DBKey.class)))
.thenReturn(new HashSet<>(EMPTY_ARTIFACTS_ID_SET));
setArtifactsIdToKeep(SIX_TO_SEVEN_ARTIFACTS_ID_SET);
ReferencedArtifactsMessageBody messageBody = (ReferencedArtifactsMessageBody) exchange.getIn()
.getBody();
assertEquals(EMPTY_ARTIFACTS_ID_SET,
removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody));
}

private void setArtifactsIdToKeep(Set<String> artifactsIdToRemove) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is to keep or remove :) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be af28aa4 🎃

ReferencedArtifactsMessageBody body = (ReferencedArtifactsMessageBody) exchange.getIn()
.getBody();
body.setArtifactsToKeep(artifactsIdToRemove);
}

}
Loading