Skip to content

adding getById support to resource collections that can support it#849

Merged
anuchandy merged 29 commits into
Azure:masterfrom
martinsawicki:martin-internals
Jun 18, 2016
Merged

adding getById support to resource collections that can support it#849
anuchandy merged 29 commits into
Azure:masterfrom
martinsawicki:martin-internals

Conversation

@martinsawicki

@martinsawicki martinsawicki commented Jun 17, 2016

Copy link
Copy Markdown
  • adding an abstract ResourcesImpl base class to have all those collections inherit from
  • ResourcesImpl requires the presence of getByGroup in the implementation
  • Resourcesimpl provides getById automatically, implemented in terms of the individual getByGroup impl
  • adding getById test to TestTemplate
  • enforcing the presence of consistent createFluentModel across collections deriving from ResourcesImpl
  • pushing converter logic and other shared functionlaity into base class
  • fixing up GenericResources a bit

@azuresdkci

Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

* @param <ImplT> the individual resource implementation
*/
public abstract class ResourcesImpl<T, ImplT>
implements

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ResourcesImpl or GroupableResourcesImpl ?

The items in this collection are groupable they extends from "GroupableResourceImpl"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

great point, though now in the process of addressing it I discovered that Deployments are weird - they're Groupable but they do not have a region, so they cannot inherit from GroupableResource right now. I'm looking into what should be one about that...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Deployment is always seems a special case. just a thought, may be we can have 'DeploymentsImpl' not to inherit from 'ResourcesImpl' and have it implement getById() directly then we can impose bounding

  GrouapbleResourcesImpl<T extends GroupableResource, ImplT extends GroupableResourceImpl>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yep, that's basically what I;m doing now.
I think Resource/GroupableResource may need to be rethought a little, later

unknown added 4 commits June 17, 2016 15:33
adding a requirement for the other createFluentWrapper via an abstract method on the base class
unknown added 3 commits June 17, 2016 17:35
- GenericResources in Azure do not expose list across groups, and passing null fails, so removing .list() support
- adding getByGroup()
- adding getById()
- enabling GenericResources to inherit from the same base class as other GroupableResources
…e class to remove the significant code repetition
@anuchandy
anuchandy merged commit 2848dc6 into Azure:master Jun 18, 2016
jianghaolu pushed a commit to jianghaolu/azure-sdk-for-java that referenced this pull request Feb 27, 2017
adding getById support to resource collections that can support it
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.

4 participants