GeoServer

Make the contract of the recently added Catalog.getLayerGroupByName(workspace, name) method more consistent

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.2-beta1
  • Fix Version/s: 2.2-beta2
  • Component/s: Configuration
  • Labels:
  • Number of attachments :
    1

Description

We recently improved the contract of the recently added Catalog.getStyleByName(WorkspaceInfo ws, String name) method.

It's the turn of the Catalog.getLayerGroupByName family of methods. Problem is pretty much explained on the following comment in CatalogImplTest.testGetLayerGroupByNameWithWorkspace test case: "will randomly return one... we should probably return null with multiple matches".

So far the only real world user is the REST config API, which already parsed the requested workspace/layer group name out of the request URL, so it's worth fixing the method contract now that we're early in the game, before the issue catches up on us with other callers having to deal with the non deterministic behavior.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Gabriel Roldan added a comment -

The attached patch clarifies and enforces the contract of the getLayerGroupByName methods and keeps the build happy

Show
Gabriel Roldan added a comment - The attached patch clarifies and enforces the contract of the getLayerGroupByName methods and keeps the build happy
Hide
Permalink
Justin Deoliveira added a comment -

Looks good, one minor thing.

public LayerGroupInfo getLayerGroupByName(String prefixedName);

The argument might be prefixed and it might not correct? I would leave it as name just to be consistent.

Show
Justin Deoliveira added a comment - Looks good, one minor thing. public LayerGroupInfo getLayerGroupByName( String prefixedName); The argument might be prefixed and it might not correct? I would leave it as name just to be consistent.
Hide
Permalink
Justin Deoliveira added a comment -

Looks good, one minor thing.

public LayerGroupInfo getLayerGroupByName(String prefixedName);

The argument might be prefixed and it might not correct? I would leave it as name just to be consistent.

Show
Justin Deoliveira added a comment - Looks good, one minor thing. public LayerGroupInfo getLayerGroupByName( String prefixedName); The argument might be prefixed and it might not correct? I would leave it as name just to be consistent.
Hide
Permalink
Gabriel Roldan added a comment -

granted. For some reason it seemed "prefixedName" would more naturally describe the intent. We need to specify it may or may not be a prefixed name on the javadocs in any case though, so no strong opinion, name does just as fine to me. Thanks Justin.

Show
Gabriel Roldan added a comment - granted. For some reason it seemed "prefixedName" would more naturally describe the intent. We need to specify it may or may not be a prefixed name on the javadocs in any case though, so no strong opinion, name does just as fine to me. Thanks Justin.
Hide
Permalink
Gabriel Roldan added a comment -

Marking as resolved at r17001

Will wait until Hudson returns from death and confirms the build is happy for him too before closing

Show
Gabriel Roldan added a comment - Marking as resolved at r17001 Will wait until Hudson returns from death and confirms the build is happy for him too before closing
Hide
Permalink
Andrea Aime added a comment -

Switching all issues that have been in "resolved" state for more than one month without further comments to "closed" status

Show
Andrea Aime added a comment - Switching all issues that have been in "resolved" state for more than one month without further comments to "closed" status

People

  • Assignee:
    Gabriel Roldan
    Reporter:
    Gabriel Roldan
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: