-
Notifications
You must be signed in to change notification settings - Fork 26
SLICE-128 making SliceLookUpTag type attribute value String #109
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
base: master
Are you sure you want to change the base?
Changes from 5 commits
48f58b4
ee07da0
314679a
4b910c0
46e22c6
572e333
0c74dd9
391e3d0
c676d8b
3b39c6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,7 @@ public class SliceLookupTag extends SimpleTagSupport { | |
|
|
||
| private String appName; // auto-detected when null | ||
|
|
||
| private Class<?> type; | ||
| private String type; | ||
|
|
||
| private void clean() { | ||
| type = null; | ||
|
|
@@ -49,14 +49,17 @@ public void doTag() throws JspException { | |
| } | ||
|
|
||
| final PageContext pageContext = (PageContext) getJspContext(); | ||
| final Object model = SliceTagUtils.getFromCurrentPath(pageContext, type, appName); | ||
| final Class typeClass = SliceTagUtils.getClassFromType(pageContext, type); | ||
| final Object model = SliceTagUtils.getFromCurrentPath(pageContext, typeClass, appName); | ||
| pageContext.setAttribute(var, model, PageContext.PAGE_SCOPE); | ||
| } catch (ClassNotFoundException e) { | ||
| throw new JspTagException("Could not get class for " + type); | ||
| } finally { | ||
| clean(); | ||
| } | ||
| } | ||
|
|
||
| public void setType(Class<?> type) { | ||
| public void setType(String type) { | ||
|
||
| this.type = type; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ | |
| import com.cognifide.slice.api.injector.InjectorWithContext; | ||
| import com.cognifide.slice.api.injector.InjectorsRepository; | ||
| import com.cognifide.slice.api.provider.ModelProvider; | ||
| import org.apache.sling.commons.classloader.DynamicClassLoaderManager; | ||
|
|
||
| public final class SliceTagUtils { | ||
|
|
||
|
|
@@ -73,6 +74,22 @@ public static <T> T getFromCurrentPath(final PageContext pageContext, final Clas | |
| return getFromCurrentPath(request, injectorsRepository, requestContextProvider, type, appName); | ||
| } | ||
|
|
||
| /** | ||
| * A helper method that returns the {@link Class} object, resolving it via it's Canonical Name. | ||
| * | ||
| * @param pageContext allows to access request context | ||
| * @param type canonical name of the modal object, whose {@link Class} object needs to be returned | ||
| * @return {@link Class} object pertaining to {@code type} as it's canonical name | ||
| * @throws ClassNotFoundException if the class was not found | ||
| */ | ||
| public static Class<?> getClassFromType(final PageContext pageContext, final String type) throws ClassNotFoundException { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mmajchrzak I've also refactored this method to use ModelProvider class to get it's Class object.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm.. By introducing of this method you're fetching the model twice during tag execution. Refactor it please, so that modelProvider#get(String, Resource) is used but model is read only once. |
||
| final SlingScriptHelper scriptHelper = getSlingScriptHelper(pageContext); | ||
| final DynamicClassLoaderManager dynamicClassLoaderManager = scriptHelper | ||
| .getService(DynamicClassLoaderManager.class); | ||
| final ClassLoader classLoader = dynamicClassLoaderManager.getDynamicClassLoader(); | ||
| return classLoader.loadClass(type); | ||
| } | ||
|
|
||
| /** | ||
| * A helper method that returns a model of the Sling resource related to given request | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /*- | ||
| * #%L | ||
| * Slice - Core Tests | ||
| * %% | ||
| * Copyright (C) 2012 Cognifide Limited | ||
| * %% | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * #L% | ||
| */ | ||
|
|
||
| package com.cognifide.slice.api.tag | ||
|
|
||
| import com.cognifide.slice.test.setup.BaseSetup | ||
| import org.apache.sling.api.scripting.SlingBindings | ||
| import org.apache.sling.api.scripting.SlingScriptHelper | ||
| import org.apache.sling.commons.classloader.DynamicClassLoaderManager | ||
| import org.junit.Assert | ||
|
|
||
| import javax.servlet.ServletRequest | ||
| import javax.servlet.jsp.PageContext | ||
| import java.lang.reflect.Method | ||
|
|
||
| /** | ||
| * @author Shashi Bhushan | ||
| * Date: 23.08.16 | ||
| */ | ||
| class SliceTagUtilsTest extends BaseSetup { | ||
|
|
||
| def "Get Class object, given the String type"() { | ||
| given: | ||
| final PageContext pageContext = Mock(PageContext){ | ||
| getRequest()>> Mock(ServletRequest){ | ||
| getAttribute(_ as String) >> Mock(SlingBindings) { | ||
| getSling() >> Mock(SlingScriptHelper) { | ||
| getService(_ as Class) >> Mock(DynamicClassLoaderManager) { | ||
| getDynamicClassLoader() >> Mock(ClassLoader) { | ||
| loadClass(_ as String) >> SliceTagUtils.class | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| when: | ||
| Class aClass = SliceTagUtils.getClassFromType(pageContext, "com.cognifide.slice.api.tag.SliceTagUtils"); | ||
|
|
||
| then: "aClass object should not be null and should be equal to expected Class" | ||
| Assert.assertNotNull(aClass) | ||
| Assert.assertEquals(SliceTagUtils.class, aClass); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ModelProvider already has the capability to create a model from type provided as String. We should refactor the code this way so we can use ModelProvider#get(String className, Resource resource)