refactror: Move osgi from flow #2
Conversation
|
Some hamcrest dependency is missing failing the validation |
Indeed. |
denis-anisimov
force-pushed
the
move-osgi
branch
from
d769495 to
6863344
Compare
|
Well, this PR needs Flow SNAPSHOT anyway. |
| Bundle osgiBundle = org.osgi.framework.FrameworkUtil | ||
| .getBundle(Bundle.class); | ||
| return osgiBundle.getVersion().toString(); | ||
| } catch (Throwable throwable) { |
There was a problem hiding this comment.
| ((ClassLoaderAwareServletContainerInitializer) initializer) | ||
| .process(filterClasses(handleTypes.orElse(null)), context); | ||
| } catch (ServletException e) { | ||
| throw new RuntimeException( |
There was a problem hiding this comment.
|
|
||
| Collection<Class<?>> bundleClasses = new ArrayList<>(); | ||
|
|
||
| for (String clazz : classes) { |
There was a problem hiding this comment.
| } | ||
| } | ||
|
|
||
| public static class ResourceService { |
There was a problem hiding this comment.
| HttpSessionListener, ServletContextListener { | ||
|
|
||
| @Reference | ||
| private ServletContainerInitializerClasses initializerClasses; |
There was a problem hiding this comment.
|
|
||
| String contextName = generateUniqueContextName(contextPath); | ||
|
|
||
| Dictionary<String, String> contextProps = new Hashtable<String, String>(); |
There was a problem hiding this comment.
|
|
||
| private static final VaadinServletMarker MARKER_INSTANCE = new VaadinServletMarker(); | ||
|
|
||
| private static final class VaadinServletMarker { |
There was a problem hiding this comment.
| } | ||
|
|
||
| private void registerPushResources(String contextName) { | ||
| Dictionary<String, String> pushProps = new Hashtable<String, String>(); |
There was a problem hiding this comment.
| } | ||
|
|
||
| private void registerClientResources(String contextName) { | ||
| Dictionary<String, String> clientProps = new Hashtable<String, String>(); |
There was a problem hiding this comment.
| } | ||
| } | ||
| return contextNames; | ||
| } catch (InvalidSyntaxException exception) { |
There was a problem hiding this comment.
|
|
||
| VaadinServletContext context = new VaadinServletContext(servletContext); | ||
| // ensure the context is set into the context | ||
| context.getAttribute(Lookup.class, () -> new OsgiLookupImpl()); |
There was a problem hiding this comment.
|
SonarQube analysis reported 16 issues
Watch the comments in this conversation to review them. 1 extra issueNote: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:
|
There was a problem hiding this comment.
Just one question - what is the rule for when the (public) classes should have the Osgi.. prefix in the names ? Is it when it is an osgi specific version of something that is already in flow-server or ? Just wondering what is the practice we have so we can keep it consistent in the future.
No any rule. |
Well, since then since everything is in an osgi package and inside the osgi artifact and repository, there probably would not be any need to use the prefix. But like you pointed out, not visible to users so there is no concerns regarding API design, it is just about how we want to do it. I don't have any opinion either on this, just wondered if there was some practice. Just let it be |
OK. |
Fixes vaadin/flow#9219