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.

Sign up for GitHub

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

Jump to bottom

External reference for actions and status-update events #830

Merged
merged 13 commits into from
Jul 25, 2019
Merged

External reference for actions and status-update events #830

merged 13 commits into from
Jul 25, 2019

Conversation

smy4kor
Copy link
Contributor

@smy4kor smy4kor commented May 2, 2019

This PR contains following changes:

  • Adding an field to actions to be able to add an external-reference-id
  • Provide the ability to update the actions-status with an internal-application event
  • Update securityContext to support running a callable under the context of a CONTROLLER role.

PS:
JpaControllerManagement in master was not formatted when this PR was being created. Hence we see more differences on this file.

@hawkbit
Copy link

hawkbit bot commented May 2, 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).

@schabdo schabdo added this to the 0.3.0M3 milestone May 2, 2019
@smy4kor smy4kor force-pushed the externalRef_for_actions branch 2 times, most recently from 0952b73 to 208f894 Compare May 2, 2019 14:09
@smy4kor smy4kor force-pushed the externalRef_for_actions branch 3 times, most recently from 1a11caf to 1bf67ae Compare May 3, 2019 10:40
@schabdo schabdo modified the milestones: 0.3.0M3, 0.3.0M4 May 21, 2019
@smy4kor smy4kor force-pushed the externalRef_for_actions branch 3 times, most recently from e4d392a to 4427214 Compare May 27, 2019 09:37
@laverman laverman modified the milestones: 0.3.0M4, 0.3.0M5 Jun 7, 2019
@smy4kor smy4kor force-pushed the externalRef_for_actions branch from 4de15b8 to fb2a60a Compare June 11, 2019 15:09
@laverman
Copy link
Contributor

It would be great if you could add the missing database migration scripts. Thanks 👍

@eclipse-hawkbit eclipse-hawkbit deleted a comment from smy4kor Jul 1, 2019
@eclipse-hawkbit eclipse-hawkbit deleted a comment from smy4kor Jul 1, 2019
@eclipse-hawkbit eclipse-hawkbit deleted a comment from smy4kor Jul 1, 2019
@eclipse-hawkbit eclipse-hawkbit deleted a comment from smy4kor Jul 1, 2019
@eclipse-hawkbit eclipse-hawkbit deleted a comment from smy4kor Jul 1, 2019
@eclipse-hawkbit eclipse-hawkbit deleted a comment from smy4kor Jul 1, 2019
@eclipse-hawkbit eclipse-hawkbit deleted a comment from smy4kor Jul 1, 2019
@Nkyn Nkyn force-pushed the externalRef_for_actions branch from ee7adfc to 275b03d Compare July 8, 2019 13:49
@schabdo
Copy link
Contributor

schabdo commented Jul 9, 2019

@hawkbit-bot verify please

@smy4kor
Copy link
Contributor Author

smy4kor commented Jul 9, 2019

@hawkbit-bot and @schabdo we have no done any changes from our previous discussion yet.

@schabdo
Copy link
Contributor

schabdo commented Jul 9, 2019

No worries! I just consulted @hawkbit-bot for an update on this PR

@SandeepAsSmy4kor
increate length limit for externalRef and add it to index
3fb342c 
externalref will be much longer than 256 chars if controllerId is as
long as 256 chars

Signed-off-by: Ravindranath Sandeep (INST-IOT/ESW-Imb) <[email protected]>
@SandeepAsSmy4kor
adding tests for verifying externalRef in controllerManagement
bead501 
Signed-off-by: Ravindranath Sandeep (INST-IOT/ESW-Imb) <[email protected]>
@SandeepAsSmy4kor
improve test to consider multiple externalRefs
3e15988 
Signed-off-by: Ravindranath Sandeep (INST-IOT/ESW-Imb) <[email protected]>
@smy4kor smy4kor force-pushed the externalRef_for_actions branch from 229e259 to 3e15988 Compare July 17, 2019 08:26
@SandeepAsSmy4kor
fix issue in migration script for mssql server
91e352e 
Signed-off-by: Ravindranath Sandeep (INST-IOT/ESW-Imb) <[email protected]>
smy4kor
smy4kor commented Jul 18, 2019
View reviewed changes
Copy link
Contributor Author

@smy4kor smy4kor left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@schabdo and @stefbehl

  • I have verified the migration scripts in all databases. There was an issue for sql-server which is now fixed.
  • Have now added the index for externalRef.
  • Added few tests in ControllerManagementTest
  • Increased the size of externalRef from 256 to 512. This is because the externalRef is controllerId + ruleId-of-iot-manager. There will be cases which string is very long.

...-repository-jpa/src/main/resources/db/migration/H2/V1_12_13__add_action_external_id___H2.sql Outdated Show resolved Hide resolved
...itory-jpa/src/main/resources/db/migration/MYSQL/V1_12_13__add_action_external_id___MYSQL.sql Outdated Show resolved Hide resolved
...src/main/resources/db/migration/SQL_SERVER/V1_12_13__add_action_external_id___SQL_SERVER.sql Outdated Show resolved Hide resolved
...awkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java Show resolved Hide resolved
...awkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java Show resolved Hide resolved
hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java Show resolved Hide resolved
@stefbehl stefbehl self-requested a review July 23, 2019 13:12
stefbehl
stefbehl approved these changes Jul 23, 2019
View reviewed changes
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. Thanks Sandeep!

@SandeepAsSmy4kor
fix documentation
78080e0 
Signed-off-by: Ravindranath Sandeep (INST-IOT/ESW-Imb) <[email protected]>
@stefbehl stefbehl merged commit fba6cf9 into eclipse-hawkbit:master Jul 25, 2019
@stefbehl stefbehl deleted the externalRef_for_actions branch July 25, 2019 07:45
@schabdo schabdo changed the title External ref for actions and status-update events External reference for actions and status-update events Jul 29, 2019
AmmarBikic pushed a commit to bosch-io/hawkbit that referenced this pull request Oct 2, 2020
@smy4kor @stefbehl
External ref for actions and status-update events (eclipse-hawkbit#830)
e3bbee5 
* Add feature to listen to rollout status update
* With this feature, extensions can update back the status of a given rollout using an event(containing distributionSetId and targetId).
* In future, AmqpMessageHandlerService can make use of this feature and de-couple its own implementation from performing status update of an action.
* Implement ActionStatusUpdateHandlerService using actionId
* Extend actions to support externalRef
* Update the action status using externalRef.
* Update securityContext to support running a callable under specific authorities.
* Fixing the review comments
* Increase length of externalRef to 128 chars
* Remove actionStatusUpdateEvent and the handler service
* Use 256 chars for externalRef
* Increment the version for migration script
* Another feature had use v1_12_12 in a recent PR. So incrementing the version.
* Create length limit for externalRef and add it to index
* Externalref will be much longer than 256 chars if controllerId is as long as 256 chars
* Adding tests for verifying externalRef in controllerManagement
* Improve test to consider multiple externalRefs
* Fix issue in migration script for mssql server
* Fix documentation

Signed-off-by: Ravindranath Sandeep (INST-IOT/ESW-Imb) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a-sayyed a-sayyed a-sayyed left review comments

@stefbehl stefbehl stefbehl approved these changes

@schabdo schabdo schabdo approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
0.3.0M5
Development

Successfully merging this pull request may close these issues.
6 participants
@smy4kor @laverman @schabdo @stefbehl @a-sayyed @SandeepAsSmy4kor