Skip to content

Commit

Permalink
Merge pull request #19 from devolo/fix/revert-rollout-target-sorting
Browse files Browse the repository at this point in the history
Revert sorting rollout targets by address
  • Loading branch information
krishna-devolo committed Sep 19, 2023
2 parents daa68e7 + f0a08c2 commit 2b3be9f
Show file tree
Hide file tree
Showing 23 changed files with 15 additions and 130 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,6 @@ RolloutCreate targetFilterQuery(
*/
RolloutCreate startAt(Long startAt);

/**
* set isSortedByAddress
*
* @param isSortedByAddress
* for {@link Rollout#getIsSortedByAddress()} ()}
* @return updated builder instance
*/
RolloutCreate isSortedByAddress(boolean isSortedByAddress);

/**
* @return peek on current state of {@link Rollout} in the builder
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ public interface Rollout extends NamedEntity {

boolean getIsCleanedUp();

boolean getIsSortedByAddress();

/**
* @return {@link DistributionSet} that is rolled out
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,6 @@ public static class TenantConfigurationKey {
*/
public static final String ROLLOUT_APPROVAL_ENABLED = "rollout.approval.enabled";

/**
* Represents setting if targets within a rollout group should be sorted by address
*/
public static final String ROLLOUT_SORT_OPTION_ENALBED = "rollout.sort.enabled";

/**
* Option setting for text search in target attributes
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public abstract class AbstractRolloutUpdateCreate<T> extends AbstractNamedEntity
protected ActionType actionType;
protected Long forcedTime;
protected Long startAt;
protected boolean isSortedByAddress;

@Min(Action.WEIGHT_MIN)
@Max(Action.WEIGHT_MAX)
Expand Down Expand Up @@ -112,18 +111,6 @@ public T startAt(final Long startAt) {
return (T) this;
}

/**
* Set start of the Rollout
*
* @param isSortedByAddress
* start time point
* @return this builder
*/
public T isSortedByAddress(final boolean isSortedByAddress) {
this.isSortedByAddress = isSortedByAddress;
return (T) this;
}

public Optional<Long> getSet() {
return Optional.ofNullable(set);
}
Expand All @@ -143,8 +130,4 @@ public Optional<Integer> getWeight() {
public Optional<Long> getStartAt() {
return Optional.ofNullable(startAt);
}

public boolean getIsSortedByAddress() {
return isSortedByAddress;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -574,21 +574,10 @@ private Long assignTargetsToGroupInNewTransaction(final JpaRollout rollout, fina
final PageRequest pageRequest = PageRequest.of(0, Math.toIntExact(limit));
final List<Long> readyGroups = RolloutHelper.getGroupsByStatusIncludingGroup(rollout.getRolloutGroups(),
RolloutGroupStatus.READY, group);
final boolean useAddressForSorting = rollout.getIsSortedByAddress();

final Comparator<Target> addressStringComparator = Comparator.comparing(o -> o.getAddress().toString());
final Slice<Target> targets = targetManagement.findByTargetFilterQueryAndNotInRolloutGroupsAndCompatible(
pageRequest, readyGroups, targetFilter, rollout.getDistributionSet().getType());
List<Target> targetList = targets.stream().collect(Collectors.toList());

if (useAddressForSorting) {
targetList = targetList.stream().sorted(addressStringComparator).collect(Collectors.toList());
}

LOGGER.debug("Assigning {} targets to rollout with Id {}", targets.getNumberOfElements(), rollout.getId());
LOGGER.debug("Targets in the rollout group are \n{}", targetList.stream().map(target -> target.getId() + ":" + target.getAddress()).collect(Collectors.joining(", ", "{", "}")));

createAssignmentOfTargetsToGroup(targetList, group);
createAssignmentOfTargetsToGroup(targets, group);
return Long.valueOf(targets.getNumberOfElements());
});
}
Expand Down Expand Up @@ -652,7 +641,7 @@ private Long createActionsForTargetsInNewTransaction(final long rolloutId, final
});
}

private void createAssignmentOfTargetsToGroup(final List<Target> targets, final RolloutGroup group) {
private void createAssignmentOfTargetsToGroup(final Slice<Target> targets, final RolloutGroup group) {
targets.forEach(target -> rolloutTargetGroupRepository.save(new RolloutTargetGroup(group, target)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,6 @@ public Rollout update(final RolloutUpdate u) {
update.getActionType().ifPresent(rollout::setActionType);
update.getForcedTime().ifPresent(rollout::setForcedTime);
update.getWeight().ifPresent(rollout::setWeight);
rollout.setIsSortedByAddress(update.getIsSortedByAddress());

// reseting back to manual start is done by setting start at time to
// null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ public JpaRollout build() {
rollout.setTargetFilterQuery(targetFilterQuery);
rollout.setStartAt(startAt);
rollout.setWeight(weight);
rollout.setIsSortedByAddress(isSortedByAddress);

if (actionType != null) {
rollout.setActionType(actionType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,6 @@ public class JpaRollout extends AbstractJpaNamedEntity implements Rollout, Event
@Column(name = "is_cleaned_up", nullable = false)
private boolean isCleanedUp = false;

@Column(name = "is_sorted_by_address", nullable = false)
private boolean isSortedByAddress = false;

@Transient
private transient TotalTargetCountStatus totalTargetCountStatus;

Expand Down Expand Up @@ -232,10 +229,6 @@ public void setWeight(final Integer weight) {

public void setIsCleanedUp(final boolean isCleanedUp) { this.isCleanedUp = isCleanedUp; }

public boolean getIsSortedByAddress() { return isSortedByAddress; }

public void setIsSortedByAddress(final boolean isSortedByAddress) { this.isSortedByAddress = isSortedByAddress; }

@Override
public long getTotalTargets() {
return totalTargets;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE sp_rollout
DROP COLUMN is_sorted_by_address;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE sp_rollout
DROP COLUMN is_sorted_by_address;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE sp_rollout
DROP COLUMN is_sorted_by_address;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE sp_rollout
DROP COLUMN is_sorted_by_address;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE sp_rollout
DROP COLUMN is_sorted_by_address;
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ public ProxyRollout map(final Rollout rollout) {
proxyRollout.setActionType(rollout.getActionType());
proxyRollout.setTargetFilterQuery(rollout.getTargetFilterQuery());
proxyRollout.setStartAt(rollout.getStartAt());
proxyRollout.setIsSortedByAddress(rollout.getIsSortedByAddress());

return proxyRollout;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ public class ProxyRollout extends ProxyNamedEntity {

private ActionType actionType;

private boolean isSortedByAddress;

/**
* Constructor
*/
Expand Down Expand Up @@ -245,25 +243,6 @@ public void setStartAt(final Long startAt) {
this.startAt = startAt;
}

/**
* Gets the boolean isSortedByAddress
*
* @return isSortedByAddress
*/
public boolean getIsSortedByAddress() {
return isSortedByAddress;
}

/**
* Sets the boolean isSortedByAddress
*
* @param isSortedByAddress
* A boolean that indicates if targets within a rollout group should be sorted by IP address
*/
public void setIsSortedByAddress(final boolean isSortedByAddress) {
this.isSortedByAddress = isSortedByAddress;
}

/**
* Gets the total count of all the rollout status
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ public class ProxyRolloutForm implements Serializable, NameAware, DsIdAware, Tar
private Long forcedTime;
private AutoStartOption autoStartOption;
private Long startAt;
private boolean isSortedByAddress;

/**
* Gets the rollout form id
Expand Down Expand Up @@ -147,8 +146,4 @@ public Long getStartAt() {
public void setStartAt(final Long startAt) {
this.startAt = startAt;
}
public boolean getIsSortedByAddress() {
return isSortedByAddress;
}
public void setIsSortedByAddress(final boolean sortedByAddress) { this.isSortedByAddress = sortedByAddress; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public ProxyRolloutWindow(final ProxyRollout rollout) {
setTargetFilterQuery(rollout.getTargetFilterQuery());
setDistributionSetInfo(rollout.getDsInfo());
setNumberOfGroups(rollout.getNumberOfGroups());
setIsSortedByAddress(rollout.getIsSortedByAddress());
}

/**
Expand Down Expand Up @@ -471,23 +470,4 @@ public void setRolloutApproval(final ProxyRolloutApproval rolloutApproval) {
public enum GroupDefinitionMode {
SIMPLE, ADVANCED;
}

/**
* Sets the form isSortedByAddress
*
* @param isSortedByAddress
* rollout form isSortedByAddress
*/
public void setIsSortedByAddress(final boolean isSortedByAddress) {
rolloutForm.setIsSortedByAddress(isSortedByAddress);
}

/**
* Gets the rollout form isSortedByAddress
*
* @return form isSortedByAddress
*/
public boolean getIsSortedByAddress() {
return rolloutForm.getIsSortedByAddress();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ public class RolloutFormLayout extends ValidatableLayout {
private static final String TEXTFIELD_NAME = "textfield.name";
private static final String CAPTION_ROLLOUT_START_TYPE = "caption.rollout.start.type";
private static final String CAPTION_ROLLOUT_ACTION_TYPE = "caption.rollout.action.type";
private static final String CAPTION_ROLLOUT_SORT_OPTION = "caption.rollout.sort_option";

private static final int CAPTION_COLUMN = 0;
private static final int FIELD_COLUMN = 1;

Expand All @@ -65,8 +63,6 @@ public class RolloutFormLayout extends ValidatableLayout {
private final TextArea descriptionField;
private final BoundComponent<ActionTypeOptionGroupAssignmentLayout> actionTypeLayout;
private final BoundComponent<AutoStartOptionGroupLayout> autoStartOptionGroupLayout;
private final CheckBox sortOptionsCheckBox;

private Long rolloutId;
private Long totalTargets;

Expand Down Expand Up @@ -101,21 +97,11 @@ public RolloutFormLayout(final VaadinMessageSource i18n,
this.descriptionField = createDescription();
this.actionTypeLayout = createActionTypeOptionGroupLayout();
this.autoStartOptionGroupLayout = createAutoStartOptionGroupLayout();
this.sortOptionsCheckBox = createSortOptionsCheckBox();

addValueChangeListeners();
setValidationStatusByBinder(binder);
}

/**
* Create checkbox for sorting targets in rollout group
*
* @return input component
*/
private CheckBox createSortOptionsCheckBox() {
return FormComponentBuilder.createCheckBox(UIComponentIdProvider.ROLLOUT_SORT_ENABLED_CHECKBOX, binder, ProxyRolloutForm::getIsSortedByAddress, ProxyRolloutForm::setIsSortedByAddress);
}

/**
* Create name field.
*
Expand Down Expand Up @@ -277,9 +263,6 @@ private void addFormToLayout(final GridLayout layout, final Component targetFilt

layout.addComponent(SPUIComponentProvider.generateLabel(i18n, CAPTION_ROLLOUT_START_TYPE), CAPTION_COLUMN, 5);
layout.addComponent(autoStartOptionGroupLayout.getComponent(), FIELD_COLUMN, 5, lastColumn, 5);

layout.addComponent(SPUIComponentProvider.generateLabel(i18n, CAPTION_ROLLOUT_SORT_OPTION), CAPTION_COLUMN, 6);
layout.addComponent(sortOptionsCheckBox, FIELD_COLUMN, 6, lastColumn, 6);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ protected Rollout persistEntityInRepository(final ProxyRolloutWindow entity) {
.targetFilterQuery(entity.getTargetFilterQuery()).actionType(entity.getActionType())
.forcedTime(entity.getActionType() == ActionType.TIMEFORCED ? entity.getForcedTime()
: RepositoryModelConstants.NO_FORCE_TIME)
.startAt(entity.getStartAtByOption())
.isSortedByAddress(entity.getIsSortedByAddress());
.startAt(entity.getStartAtByOption());

final Rollout rolloutToCreate;
if (GroupDefinitionMode.SIMPLE == entity.getGroupDefinitionMode()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public AddRolloutWindowLayout(final RolloutWindowDependencies dependencies) {

@Override
protected void addComponents(final GridLayout rootLayout) {
rootLayout.setRows(8);
rootLayout.setRows(7);

final int lastRowIdx = rootLayout.getRows() - 1;
final int lastColumnIdx = rootLayout.getColumns() - 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public UpdateRolloutWindowLayout(final RolloutWindowDependencies dependencies) {

@Override
protected void addComponents(final GridLayout rootLayout) {
rootLayout.setRows(7);
rootLayout.setRows(6);

final int lastColumnIdx = rootLayout.getColumns() - 1;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1574,12 +1574,6 @@ public final class UIComponentIdProvider {
*/
public static final String ROLLOUT_APPROVAL_ENABLED_CHECKBOX = "rollout.approve.enabled.checkbox";

/**
* Configuration checkbox for
* {@link TenantConfigurationKey#ROLLOUT_SORT_OPTION_ENALBED}
*/
public static final String ROLLOUT_SORT_ENABLED_CHECKBOX = "rollout.sort.enabled.checkbox";

/**
* Configuration checkbox for
* {@link TenantConfigurationKey#TARGET_SEARCH_ATTRIBUTES_ENABLED}
Expand Down
1 change: 0 additions & 1 deletion hawkbit-ui/src/main/resources/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,6 @@ caption.rollout.action.type = Action type
message.rollout.remaining.targets.error = Not all targets are addressed
textfield.rollout.copied.name = Copy of {0}
label.rollout.targets.in.group = {0} in {1}
caption.rollout.sort_option = Sort targets by address
caption.rollout.start.type = Start type
caption.rollout.start.manual = Manual
caption.rollout.start.manual.desc = The user starts the rollout manually.
Expand Down

0 comments on commit 2b3be9f

Please sign in to comment.