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

Increase target name to 128 and target controller id to 256 #849

Conversation

floruschbaschan
Copy link
Contributor

Signed-off-by: Florian Ruschbaschan Florian.Ruschbaschan@bosch-si.com

Signed-off-by: Florian Ruschbaschan <Florian.Ruschbaschan@bosch-si.com>
@hawkbit
Copy link

hawkbit bot commented Jun 6, 2019

Thanks for taking the time to contribute to hawkBit! We really appreciate this. Make yourself comfortable while I'm looking for a committer to help you with your contribution.
Please make sure you read the contribution guide and signed the Eclipse Contributor Agreement (ECA).

@hawkbit-bot
Copy link

Can one of the admins verify this patch?

Signed-off-by: Florian Ruschbaschan <Florian.Ruschbaschan@bosch-si.com>
Signed-off-by: Florian Ruschbaschan <Florian.Ruschbaschan@bosch-si.com>
Signed-off-by: Florian Ruschbaschan <Florian.Ruschbaschan@bosch-si.com>

# This profile adds basic configurations for a DB2 DB usage.
# Keep in mind that you need the DB2 driver in your classpath on compile.
# see https://www.eclipse.org/hawkbit/documentation/guide/runhawkbit.html
Copy link
Contributor

Choose a reason for hiding this comment

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

url is no longer valid, would you please change it to https://www.eclipse.org/hawkbit/guides/runhawkbit/ also in the other places where it might be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I have also changed the URL for application-mysql.properties and application-mssql.properties

ALTER TABLE sp_target_tag ALTER COLUMN name VARCHAR(128);


ALTER TABLE sp_target ALTER COLUMN controller_id VARCHAR(256);
Copy link
Contributor

Choose a reason for hiding this comment

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

add new line at the end of this file, for github

ALTER TABLE sp_target_tag ALTER COLUMN name SET DATA TYPE VARCHAR(128);


ALTER TABLE sp_target ALTER COLUMN controller_id SET DATA TYPE VARCHAR(256);
Copy link
Contributor

Choose a reason for hiding this comment

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

add new line at the end of this file, for github

ALTER TABLE sp_target_tag MODIFY name VARCHAR(128);


ALTER TABLE sp_target MODIFY controller_id VARCHAR(256);
Copy link
Contributor

Choose a reason for hiding this comment

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

add new line at the end of this file, for github

ALTER TABLE sp_target_tag ALTER COLUMN name VARCHAR(128);


ALTER TABLE sp_target ALTER COLUMN controller_id VARCHAR(256);
Copy link
Contributor

Choose a reason for hiding this comment

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

add new line at the end of this file, for github

@@ -251,7 +252,7 @@ private void createAndUpdateDistributionSetWithInvalidVersion(final Distribution

assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.create(entityFactory.distributionSet().create().name("a")
.version(RandomStringUtils.randomAlphanumeric(65))))
.version(RandomStringUtils.randomAlphanumeric(NamedEntity.NAME_MAX_SIZE + 1))))
Copy link
Contributor

@a-sayyed a-sayyed Jun 12, 2019

Choose a reason for hiding this comment

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

wrong field is being tested here, should be .name() instead of .version(), would you mind adding another block that tests the name and leaving the one that tests the version as is?

@@ -260,7 +261,7 @@ private void createAndUpdateDistributionSetWithInvalidVersion(final Distribution

assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.update(entityFactory.distributionSet().update(set.getId())
.version(RandomStringUtils.randomAlphanumeric(65))))
.version(RandomStringUtils.randomAlphanumeric(NamedEntity.NAME_MAX_SIZE + 1))))
Copy link
Contributor

@a-sayyed a-sayyed Jun 12, 2019

Choose a reason for hiding this comment

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

wrong field is being tested here, should be .name() instead of .version(), would you mind adding another block that tests the name and leaving the one that tests the version as is?

@@ -165,7 +166,7 @@ private void createAndUpdateDistributionSetWithInvalidVersion(final Distribution

assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.create(entityFactory.distributionSet().create().name("a")
.version(RandomStringUtils.randomAlphanumeric(65))))
.version(RandomStringUtils.randomAlphanumeric(NamedEntity.NAME_MAX_SIZE + 1))))
Copy link
Contributor

@a-sayyed a-sayyed Jun 12, 2019

Choose a reason for hiding this comment

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

wrong field is being tested here, should be .name() instead of .version(), would you mind adding another block that tests the name and leaving the one that tests the version as is?

@@ -184,7 +185,7 @@ private void createAndUpdateDistributionSetWithInvalidVersion(final Distribution

assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.update(entityFactory.distributionSet().update(set.getId())
.version(RandomStringUtils.randomAlphanumeric(65))))
.version(RandomStringUtils.randomAlphanumeric(NamedEntity.NAME_MAX_SIZE + 1))))
Copy link
Contributor

@a-sayyed a-sayyed Jun 12, 2019

Choose a reason for hiding this comment

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

wrong field is being tested here, should be .name() instead of .version(), would you mind adding another block that tests the name and leaving the one that tests the version as is?

@@ -281,7 +282,7 @@ private void createAndUpdateTargetWithInvalidSecurityToken(final Target target)

assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.create(entityFactory.target().create().controllerId("a")
.securityToken(RandomStringUtils.randomAlphanumeric(129))))
.securityToken(RandomStringUtils.randomAlphanumeric(NamedEntity.NAME_MAX_SIZE + 1))))
Copy link
Contributor

@a-sayyed a-sayyed Jun 12, 2019

Choose a reason for hiding this comment

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

wrong field is being tested here, should be .controllerId() instead of .securityToken(), would you mind adding another block that tests the controllerId and leaving the one that tests the securityToken as is?

@@ -291,7 +292,7 @@ private void createAndUpdateTargetWithInvalidSecurityToken(final Target target)

assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> targetManagement.update(entityFactory.target().update(target.getControllerId())
.securityToken(RandomStringUtils.randomAlphanumeric(129))))
.securityToken(RandomStringUtils.randomAlphanumeric(NamedEntity.NAME_MAX_SIZE + 1))))
Copy link
Contributor

@a-sayyed a-sayyed Jun 12, 2019

Choose a reason for hiding this comment

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

wrong field is being tested here, should be .controllerId() instead of .securityToken(), would you mind adding another block that tests the controllerId and leaving the one that tests the securityToken as is?

- Use correct constant field for junit tests
- Change Hawkbit documentation url of application-<db>.properties
- Add new line at the end of db migration scripts

Signed-off-by: Florian Ruschbaschan <Florian.Ruschbaschan@bosch-si.com>
@@ -252,7 +253,7 @@ private void createAndUpdateDistributionSetWithInvalidVersion(final Distribution

assertThatExceptionOfType(ConstraintViolationException.class)
.isThrownBy(() -> distributionSetManagement.create(entityFactory.distributionSet().create().name("a")
.version(RandomStringUtils.randomAlphanumeric(NamedEntity.NAME_MAX_SIZE + 1))))
.version(RandomStringUtils.randomAlphanumeric(NamedVersionedEntity.VERSION_MAX_SIZE + 1))))
.as("entity with too long name should not be created");
Copy link
Contributor

Choose a reason for hiding this comment

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

could you please fix the assertion error message, entity with too long version should not be created instead of entity with too long name should not be created. It's in multiple places in this file

Signed-off-by: Florian Ruschbaschan <Florian.Ruschbaschan@bosch-si.com>
@floruschbaschan floruschbaschan marked this pull request as ready for review June 13, 2019 09:32
@a-sayyed
Copy link
Contributor

looks good! 👍

@@ -1,5 +1,5 @@
#
# Copyright (c) 2015 Bosch Software Innovations GmbH and others.
# Copyright (c) 2019 Bosch Software Innovations GmbH and others.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert back to 2015, since there has been the original inception

@@ -1,5 +1,5 @@
#
# Copyright (c) 2018 Bosch Software Innovations GmbH and others.
# Copyright (c) 2019 Bosch Software Innovations GmbH and others.
Copy link
Contributor

Choose a reason for hiding this comment

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

see below

@laverman
Copy link
Contributor

@hawkbit-bot please verify

Signed-off-by: Florian Ruschbaschan <Florian.Ruschbaschan@bosch-si.com>
Signed-off-by: Florian Ruschbaschan <Florian.Ruschbaschan@bosch-si.com>
@stefbehl stefbehl self-requested a review June 18, 2019 07:27
Copy link
Contributor

@stefbehl stefbehl left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you!

@stefbehl stefbehl merged commit f6c0edf into eclipse:master Jun 18, 2019
@stefbehl stefbehl deleted the feature/IncreaseTargetControllerIdAndTargetNameLimit branch June 18, 2019 13:29
@schabdo schabdo added this to the 0.3.0M5 milestone Jul 1, 2019
AmmarBikic pushed a commit to bosch-io/hawkbit that referenced this pull request Oct 2, 2020
)

* Increase target name to 128 and target controller id to 256
* Fix test failures by using constant NamedEntity.NAME_MAX_SIZE + 1
* Use constant NamedEntity.NAME_MAX_SIZE + 1 for mgmt-resource tests
* Add db migration scripts to increase the controllerId and name limit
* Fix review issues
* Use correct constant field for junit tests
* Change Hawkbit documentation url of application-<db>.properties
* Add new line at the end of db migration scripts
* Update assertion description
* Revert copyright years to its creator year
* Add DDI-, AMQP- and controller management-tests

Signed-off-by: Florian Ruschbaschan <Florian.Ruschbaschan@bosch-si.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants