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

refactor: Rework SDK to use Interfaces and factory methods #741

Merged

Conversation

lenny-goodell
Copy link
Member

@lenny-goodell lenny-goodell commented Mar 16, 2021

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Code needed to be refactored to use interfaces and factory methods
Issue Number: #573

What is the new behavior?

Added mocks for interfaces for use is custom service unit tests.
Refactored to make better use of the dependency injection container (dic) to propagate the common dependencies through out the layers
General file clean up for items flag by IDE as non-standard code.
Renamed OutputData to ResponseData

These ASC draft PR shows the impact of these changes
edgexfoundry/app-service-configurable#214
as well as the changes made to the template app service in this repo

Does this PR introduce a breaking change?

  • Yes
  • No

BREAKING CHANGE: App Services will require refactoring to use new interfaces and factory methods

Are there any new imports or modules? If so, what are they used for and why?

no

Are there any specific instructions or things that should be known prior to reviewing?

Start your review with these files, then the changes in the other files will make more sense.

  • pkg/interfaces/service.go
  • pkg/interfaces/context.go
  • pkg/factory.go
  • pkg/factory_test.go
  • internal/app/service.go
  • internal/app/service_test.go
  • internal/appfunction/context.go
  • internal/appfunction/context_test.go
  • app-service-template/main.go
  • app-service-template/main_test.go
  • app-service-template/functions/sample.go
  • app-service-template/functions/sample_test.go
  • pkg/transforms/*
  • Then all the remaining files

Other information

lenny added 2 commits March 16, 2021 10:26
Added mocks for interfaces for use is custom service unit tests.
Refactored to make better use of the dependency injection container (dic) to propigate the common dependecies throught the layers
General file clean up for items flag by IDE as non-standard code.

closes #573

Signed-off-by: lenny <[email protected]>
Added mocks for interfaces for use is custom service unit tests.
Refactored to make better use of the dependency injection container (dic) to propagate the common dependencies through out the layers
General file clean up for items flag by IDE as non-standard code.

closes #573

BREAKING CHANGE: App Services will require refactoring to use new interfaces  and factory methods

Signed-off-by: lenny <[email protected]>
Copy link
Contributor

@jim-wang-intel jim-wang-intel left a comment

Choose a reason for hiding this comment

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

just reviewed some of them, not totally done yet.

internal/runtime/runtime.go Outdated Show resolved Hide resolved
internal/app/triggerfactory.go Outdated Show resolved Hide resolved
internal/app/triggerfactory.go Outdated Show resolved Hide resolved
internal/app/triggerfactory.go Outdated Show resolved Hide resolved
pkg/transforms/batch.go Outdated Show resolved Hide resolved
AlexCuse
AlexCuse previously approved these changes Mar 16, 2021
Copy link
Contributor

@jim-wang-intel jim-wang-intel left a comment

Choose a reason for hiding this comment

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

Done with PR reviews

app-service-template/Attribution.txt Show resolved Hide resolved
app-service-template/functions/sample.go Outdated Show resolved Hide resolved
internal/app/configurable.go Outdated Show resolved Hide resolved
internal/app/configurable.go Outdated Show resolved Hide resolved
internal/app/configurable.go Outdated Show resolved Hide resolved
pkg/transforms/compression.go Outdated Show resolved Hide resolved
pkg/transforms/compression.go Outdated Show resolved Hide resolved
pkg/transforms/compression.go Outdated Show resolved Hide resolved
pkg/transforms/http.go Outdated Show resolved Hide resolved
pkg/transforms/tags.go Outdated Show resolved Hide resolved
@lenny-goodell lenny-goodell marked this pull request as ready for review March 18, 2021 17:34
Copy link
Contributor

@jim-wang-intel jim-wang-intel left a comment

Choose a reason for hiding this comment

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

LGTM

@lenny-goodell lenny-goodell merged commit 3a57661 into edgexfoundry:master Mar 18, 2021
@lenny-goodell lenny-goodell deleted the refactor-to-interfaces branch March 18, 2021 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor SDK: Add interfaces for AppSdk and AppContext so services can mock them for unit testing.
3 participants