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

ISPN-7244 Creating new cache should only show valid cache types #177

Closed
wants to merge 2 commits into from
Closed

Conversation

vblagoje
Copy link

@vblagoje vblagoje commented Jan 17, 2017

Master only. See https://issues.jboss.org/browse/ISPN-7244 Please verify local mode and domain mode to test this functionality @ryanemerson

@ryanemerson
Copy link
Collaborator

ryanemerson commented Jan 18, 2017

@vblagoje We should define the behaviour of default cache types for standalone/domain all in the CacheConfigService so that the logic is centralised. I believe the following actions are required:

  1. Replace CACHE_TYPES in CacheConfigService with a DOMAIN_CACHE_TYPES and STANDALONE_CACHE_TYPES arrays.

  2. As the CacheConfigService already injects the launch service, create the following methods in CacheConfigService:

    • getAllCacheTypes(); // return appropriate array based upon launch type
    • getDefaultCacheType(); // return the first index of the array returned by ^ method
    • filterTemplates(template[]); // filter passed templates based upon getAllCacheTypes

The advantage of ^ approach is that all of the logic for default/valid template/cache types is dictated by the two arrays. This will also reduce the size of the resolve logic for CacheTemplates and Caches.

WDYT?

@vblagoje
Copy link
Author

Great ideas and insights @ryanemerson Will follow your suggestions!

@vblagoje
Copy link
Author

Thanks for these awesome insights @ryanemerson Onto id generator now!

@ryanemerson
Copy link
Collaborator

@vblagoje Integrated c592d7f, thanks

@vblagoje vblagoje deleted the t_7244 branch January 17, 2018 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants