karaf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guillaume Nodet <gno...@apache.org>
Subject Re: git commit: KARAF-2762 Refactored CommandExporter and implemented removal on stop
Date Mon, 17 Feb 2014 13:18:38 GMT
-1
I think it's a really bad idea to have the user expose services in OSGI
which are not supposed to be used and not thread safe by design.
Please revert and let's continue the discussion on the dev mailing list.


2014-02-17 13:54 GMT+01:00 <cschneider@apache.org>:

> Repository: karaf
> Updated Branches:
>   refs/heads/master e0eda8a36 -> 8a4c9124a
>
>
> KARAF-2762 Refactored CommandExporter and implemented removal on stop
>
>
> Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
> Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/8a4c9124
> Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/8a4c9124
> Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/8a4c9124
>
> Branch: refs/heads/master
> Commit: 8a4c9124a5336208f05a4b239bea48c917ed1baa
> Parents: e0eda8a
> Author: Christian Schneider <chris@die-schneider.net>
> Authored: Mon Feb 17 13:54:19 2014 +0100
> Committer: Christian Schneider <chris@die-schneider.net>
> Committed: Mon Feb 17 13:54:19 2014 +0100
>
> ----------------------------------------------------------------------
>  .../karaf/shell/exporter/ActionCommand.java     | 132 +++++++++++++++++++
>  .../karaf/shell/exporter/ActionTracker.java     |  75 +++++++++++
>  .../apache/karaf/shell/exporter/Activator.java  |  32 +----
>  .../karaf/shell/exporter/CommandExporter.java   | 131 ------------------
>  4 files changed, 210 insertions(+), 160 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/karaf/blob/8a4c9124/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/ActionCommand.java
> ----------------------------------------------------------------------
> diff --git
> a/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/ActionCommand.java
> b/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/ActionCommand.java
> new file mode 100644
> index 0000000..946d120
> --- /dev/null
> +++
> b/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/ActionCommand.java
> @@ -0,0 +1,132 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you 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.
> + */
> +package org.apache.karaf.shell.exporter;
> +
> +import java.lang.reflect.Field;
> +import java.util.ArrayList;
> +import java.util.Hashtable;
> +import java.util.List;
> +import java.util.Map;
> +
> +import org.apache.felix.gogo.commands.Action;
> +import org.apache.felix.gogo.commands.Command;
> +import org.apache.felix.gogo.commands.CommandWithAction;
> +import org.apache.felix.gogo.commands.basic.AbstractCommand;
> +import org.apache.felix.service.command.CommandProcessor;
> +import org.apache.felix.service.command.Function;
> +import org.apache.karaf.shell.console.CompletableFunction;
> +import org.apache.karaf.shell.console.Completer;
> +import org.osgi.framework.BundleContext;
> +import org.osgi.framework.ServiceRegistration;
> +import org.slf4j.Logger;
> +import org.slf4j.LoggerFactory;
> +
> +/**
> + * Wraps an Action as a command using a template for the action injects
> + */
> +@SuppressWarnings("deprecation")
> +public class ActionCommand extends AbstractCommand implements
> CompletableFunction {
> +    private static Logger logger =
> LoggerFactory.getLogger(ActionCommand.class);
> +
> +    private Action actionTemplate;
> +    private List<Completer> completers = new ArrayList<Completer>();
> +
> +    public ActionCommand(Action actionTemplate) {
> +        this.actionTemplate = actionTemplate;
> +        addCompleters();
> +    }
> +
> +    public ServiceRegistration<?> registerService(BundleContext context) {
> +        Class<? extends Action> actionClass = actionTemplate.getClass();
> +        Command cmd = actionClass.getAnnotation(Command.class);
> +        if (cmd == null) {
> +            throw new IllegalArgumentException("Action class " +
> actionClass
> +                                               + " is not annotated with
> @Command");
> +        }
> +        String[] interfaces = new String[] {
> +            Function.class.getName(),
> +            CommandWithAction.class.getName(),
> +            AbstractCommand.class.getName()
> +        };
> +        Hashtable<String, String> props = new Hashtable<String, String>();
> +        props.put(CommandProcessor.COMMAND_SCOPE, cmd.scope());
> +        props.put(CommandProcessor.COMMAND_FUNCTION, cmd.name());
> +        logger.info("Registering command " + cmd.scope() + ":" + cmd.name()
> + " in the name of bundle " + context.getBundle().getBundleId());
> +        return context.registerService(interfaces, this, props);
> +    }
> +
> +    @Override
> +    public Class<? extends Action> getActionClass() {
> +        return actionTemplate.getClass();
> +    }
> +
> +    @Override
> +    public Action createNewAction() {
> +        try {
> +            Action newAction = actionTemplate.getClass().newInstance();
> +            copyFields(newAction);
> +            return newAction;
> +        } catch (InstantiationException e) {
> +            throw new RuntimeException(e);
> +        } catch (IllegalAccessException e) {
> +            throw new RuntimeException(e);
> +        }
> +    }
> +
> +    @Override
> +    public List<Completer> getCompleters() {
> +        return completers;
> +    }
> +
> +    @Override
> +    public Map<String, Completer> getOptionalCompleters() {
> +        return null;
> +    }
> +
> +    private void copyFields(Action newAction) {
> +        Field[] fields = actionTemplate.getClass().getDeclaredFields();
> +        for (Field field : fields) {
> +            try {
> +                if (!field.isAccessible()) {
> +                    field.setAccessible(true);
> +                }
> +                Object value = field.get(actionTemplate);
> +                field.set(newAction, value);
> +            } catch (Exception e) {
> +                throw new RuntimeException(e.getMessage(), e);
> +            }
> +        }
> +    }
> +
> +    private void addCompleters() {
> +        for (Field field : actionTemplate.getClass().getDeclaredFields())
> {
> +            if (Completer.class.isAssignableFrom(field.getType())) {
> +                try {
> +                    if (!field.isAccessible()) {
> +                        field.setAccessible(true);
> +                    }
> +
>  this.completers.add((Completer)field.get(actionTemplate));
> +                } catch (Exception e) {
> +                    logger.warn("Error setting completer from field " +
> field.getName());
> +                }
> +            }
> +        }
> +    }
> +
> +}
>
>
> http://git-wip-us.apache.org/repos/asf/karaf/blob/8a4c9124/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/ActionTracker.java
> ----------------------------------------------------------------------
> diff --git
> a/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/ActionTracker.java
> b/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/ActionTracker.java
> new file mode 100644
> index 0000000..55cd06e
> --- /dev/null
> +++
> b/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/ActionTracker.java
> @@ -0,0 +1,75 @@
> +package org.apache.karaf.shell.exporter;
> +
> +import java.util.Map;
> +import java.util.concurrent.ConcurrentHashMap;
> +
> +import org.apache.felix.gogo.commands.Action;
> +import org.osgi.framework.Bundle;
> +import org.osgi.framework.BundleContext;
> +import org.osgi.framework.ServiceReference;
> +import org.osgi.framework.ServiceRegistration;
> +import org.osgi.util.tracker.ServiceTracker;
> +import org.osgi.util.tracker.ServiceTrackerCustomizer;
> +import org.slf4j.Logger;
> +import org.slf4j.LoggerFactory;
> +
> +/**
> + * Tracks services that implement {@link
> org.apache.felix.gogo.commands.Action},
> + * wraps each into an ActionCommand
> + * and exports the command as a service in the name of the bundle
> exporting the Action
> + */
> +@SuppressWarnings("deprecation")
> +final class ActionTracker extends ServiceTracker<Action, Action> {
> +    private Logger logger = LoggerFactory.getLogger(this.getClass());
> +
> +    @SuppressWarnings("rawtypes")
> +    private Map<ServiceReference, ServiceRegistration> registrations =
> +        new ConcurrentHashMap<ServiceReference, ServiceRegistration>();
> +
> +    ActionTracker(BundleContext context, Class<Action> clazz,
> +                         ServiceTrackerCustomizer<Action, Action>
> customizer) {
> +        super(context, clazz, customizer);
> +    }
> +
> +    @Override
> +    public Action addingService(ServiceReference<Action> reference) {
> +        if (!registrations.containsKey(reference)) {
> +            Bundle userBundle = reference.getBundle();
> +            try {
> +                BundleContext context = userBundle.getBundleContext();
> +                ActionCommand command = new
> ActionCommand(context.getService(reference));
> +                registrations.put(reference,
> command.registerService(context));
> +            } catch (Exception e) {
> +                logger.warn("Error exporting action as command from
> service of bundle "
> +                    + userBundle.getSymbolicName()
> +                    + "[" + userBundle.getBundleId() + "]", e);
> +            }
> +        }
> +        return super.addingService(reference);
> +    }
> +
> +    @Override
> +    public void removedService(ServiceReference<Action> reference, Action
> service) {
> +        if (registrations.containsKey(reference)) {
> +            try {
> +                registrations.remove(reference).unregister();
> +            } catch (Exception e) {
> +                // Ignore .. might already be unregistered if exporting
> bundle stopped
> +            }
> +        }
> +        super.removedService(reference, service);
> +    }
> +
> +    @Override
> +    public void close() {
> +        for (ServiceRegistration<?> reg : registrations.values()) {
> +            try {
> +                reg.unregister();
> +            } catch (Exception e) {
> +                // Ignore .. might already be unregistered if exporting
> bundle stopped
> +            }
> +        }
> +        registrations.clear();
> +        super.close();
> +    }
> +}
> \ No newline at end of file
>
>
> http://git-wip-us.apache.org/repos/asf/karaf/blob/8a4c9124/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/Activator.java
> ----------------------------------------------------------------------
> diff --git
> a/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/Activator.java
> b/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/Activator.java
> index 8ce28f4..34eb761 100644
> ---
> a/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/Activator.java
> +++
> b/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/Activator.java
> @@ -14,49 +14,23 @@
>  package org.apache.karaf.shell.exporter;
>
>  import org.apache.felix.gogo.commands.Action;
> -import org.osgi.framework.Bundle;
>  import org.osgi.framework.BundleActivator;
>  import org.osgi.framework.BundleContext;
> -import org.osgi.framework.ServiceReference;
>  import org.osgi.util.tracker.ServiceTracker;
> -import org.slf4j.Logger;
> -import org.slf4j.LoggerFactory;
>
>  @SuppressWarnings("deprecation")
>  public class Activator implements BundleActivator {
> -    Logger logger = LoggerFactory.getLogger(this.getClass());
> +    private ServiceTracker<Action, Action> tracker;
>
>      @Override
>      public void start(BundleContext context) throws Exception {
> -        ServiceTracker<Action, Action> tracker = new
> ServiceTracker<Action, Action>(context, Action.class, null) {
> -
> -            @Override
> -            public Action addingService(ServiceReference<Action>
> reference) {
> -                Bundle userBundle = reference.getBundle();
> -                try {
> -                    Action action = context.getService(reference);
> -                    CommandExporter.export(userBundle.getBundleContext(),
> action);
> -                    logger.info("Service registered");
> -                } catch (Exception e) {
> -                    logger.warn("Error exporting action from service of
> bundle "
> -                        + userBundle.getSymbolicName()
> -                        + "[" + userBundle.getBundleId() + "]", e);
> -                }
> -                return super.addingService(reference);
> -            }
> -
> -            @Override
> -            public void removedService(ServiceReference<Action>
> reference, Action service) {
> -                // TODO implement removing of commands
> -                super.removedService(reference, service);
> -            }
> -
> -        };
> +        tracker = new ActionTracker(context, Action.class, null);
>          tracker.open();
>      }
>
>      @Override
>      public void stop(BundleContext context) throws Exception {
> +        tracker.close();
>      }
>
>  }
>
>
> http://git-wip-us.apache.org/repos/asf/karaf/blob/8a4c9124/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/CommandExporter.java
> ----------------------------------------------------------------------
> diff --git
> a/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/CommandExporter.java
> b/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/CommandExporter.java
> deleted file mode 100644
> index a9c75c3..0000000
> ---
> a/shell/command-exporter/src/main/java/org/apache/karaf/shell/exporter/CommandExporter.java
> +++ /dev/null
> @@ -1,131 +0,0 @@
> -/*
> - * Licensed to the Apache Software Foundation (ASF) under one
> - * or more contributor license agreements.  See the NOTICE file
> - * distributed with this work for additional information
> - * regarding copyright ownership.  The ASF licenses this file
> - * to you 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.
> - */
> -package org.apache.karaf.shell.exporter;
> -
> -import java.lang.reflect.Field;
> -import java.util.ArrayList;
> -import java.util.Hashtable;
> -import java.util.List;
> -import java.util.Map;
> -
> -import org.apache.felix.gogo.commands.Action;
> -import org.apache.felix.gogo.commands.Command;
> -import org.apache.felix.gogo.commands.CommandWithAction;
> -import org.apache.felix.gogo.commands.basic.AbstractCommand;
> -import org.apache.felix.service.command.CommandProcessor;
> -import org.apache.felix.service.command.Function;
> -import org.apache.karaf.shell.console.CompletableFunction;
> -import org.apache.karaf.shell.console.Completer;
> -import org.osgi.framework.BundleContext;
> -import org.osgi.framework.ServiceRegistration;
> -import org.slf4j.Logger;
> -import org.slf4j.LoggerFactory;
> -
> -/**
> - * Exports an Action as a command using a template for the action injects
> - */
> -@SuppressWarnings("deprecation")
> -public class CommandExporter extends AbstractCommand implements
> CompletableFunction {
> -    private static Logger logger =
> LoggerFactory.getLogger(CommandExporter.class);
> -
> -    private Action actionTemplate;
> -    private List<Completer> completers = new ArrayList<Completer>();
> -
> -    public CommandExporter(Action actionTemplate)
> -    {
> -        this.actionTemplate = actionTemplate;
> -        addCompleters();
> -    }
> -
> -    public Class<? extends Action> getActionClass()
> -    {
> -        return actionTemplate.getClass();
> -    }
> -
> -    public Action createNewAction() {
> -        try {
> -            Action newAction = actionTemplate.getClass().newInstance();
> -            copyFields(newAction);
> -            return newAction;
> -        } catch (InstantiationException e) {
> -            throw new RuntimeException(e);
> -        } catch (IllegalAccessException e) {
> -            throw new RuntimeException(e);
> -        }
> -    }
> -
> -    private void copyFields(Action newAction) {
> -        Field[] fields = actionTemplate.getClass().getDeclaredFields();
> -        for (Field field : fields) {
> -            try {
> -                field.setAccessible(true);
> -                Object value = field.get(actionTemplate);
> -                field.set(newAction, value);
> -            } catch (Exception e) {
> -                throw new RuntimeException(e.getMessage(), e);
> -            }
> -        }
> -    }
> -
> -    @SuppressWarnings("rawtypes")
> -    public static ServiceRegistration export(BundleContext context,
> Action actionTemplate)
> -    {
> -        Class<? extends Action> actionClass = actionTemplate.getClass();
> -        logger.info(actionClass.getName());
> -        Command cmd = actionClass.getAnnotation(Command.class);
> -        if (cmd == null)
> -        {
> -            throw new IllegalArgumentException("Action class is not
> annotated with @Command");
> -        }
> -        Hashtable<String, String> props = new Hashtable<String, String>();
> -        props.put(CommandProcessor.COMMAND_SCOPE, cmd.scope());
> -        props.put(CommandProcessor.COMMAND_FUNCTION, cmd.name());
> -        CommandExporter command = new CommandExporter(actionTemplate);
> -        return context.registerService(new String[]{
> -
>  Function.class.getName(),
> -
>  CommandWithAction.class.getName(),
> -
>  AbstractCommand.class.getName()
> -                                                    },
> -                                       command, props);
> -    }
> -
> -    private void addCompleters() {
> -        for (Field field : actionTemplate.getClass().getDeclaredFields())
> {
> -            field.setAccessible(true);
> -            if (Completer.class.isAssignableFrom(field.getType())) {
> -                try {
> -
>  this.completers.add((Completer)field.get(actionTemplate));
> -                } catch (Exception e) {
> -                    logger.warn("Error setting completer from field " +
> field.getName());
> -                }
> -            }
> -        }
> -    }
> -
> -    @Override
> -    public List<Completer> getCompleters() {
> -        return completers;
> -    }
> -
> -    @Override
> -    public Map<String, Completer> getOptionalCompleters() {
> -        return null;
> -    }
> -
> -}
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message