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

Improve LocalizedTextUtil defend NPE #35

Closed
wants to merge 1 commit into from
Closed

Improve LocalizedTextUtil defend NPE #35

wants to merge 1 commit into from

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Feb 15, 2015

defend NPE if not ValueStack presented, for example non-struts request or backend thread.

depend NPE if not ValueStack presented, for example non-struts request or backend thread.
@quaff quaff changed the title Improve LocalizedTextUtil depend NPE Improve LocalizedTextUtil defend NPE Feb 15, 2015
@asfbot
Copy link

asfbot commented Feb 15, 2015

Struts-JDK7-pull-request #24 SUCCESS
This pull request looks good

@wolpi
Copy link

wolpi commented Feb 16, 2015

could you add a unit test, please?

@lukaszlenart
Copy link
Member

I'm not sure if this is a good solution, can you provide more details?

@quaff
Copy link
Contributor Author

quaff commented Mar 9, 2015

@lukaszlenart , It doesn't add new thing just null-check added, It's safe if existing unit tests passed.

@lukaszlenart
Copy link
Member

What I meant is that if ValueStack is missing in context something wrong happened, so solution to null-check is error-prone :)

@quaff
Copy link
Contributor Author

quaff commented Mar 10, 2015

ValueStack will be always not null with struts2 request, so It wouldn't change the behavior of struts2, this pull request is intend to use in non-struts2 context.

@lukaszlenart
Copy link
Member

That's the case, can you explain what you mean by non-struts2 context?

@quaff
Copy link
Contributor Author

quaff commented Mar 11, 2015

simplest case

import java.util.Locale;
public class Main {
    public static void main(String[] args) {
        System.out.println(LocalizedTextUtil.findText(Main.class, "key",Locale.getDefault(), "defaultMessage", null, null));
    }
}

Main.properties

key=this is a key

it will print "this is a key"

@lukaszlenart
Copy link
Member

So you can use a StubValueContext like this one

public class StubValueStack implements ValueStack {
    Map<String, Object> ctx = new HashMap<String, Object>();
    CompoundRoot root = new CompoundRoot();

    public Map<String, Object> getContext() {
        return ctx;
    }

    public void setDefaultType(Class defaultType) {
    }

    public void setExprOverrides(Map<Object, Object> overrides) {
    }

    public Map<Object, Object> getExprOverrides() {
        return null;
    }

    public CompoundRoot getRoot() {
        return root;
    }

    public void setValue(String expr, Object value) {
        ctx.put(expr, value);
    }

    public void setParameter(String expr, Object value) {
        throw new UnsupportedOperationException("not implemented");
    }

    public void setValue(String expr, Object value, boolean throwExceptionOnFailure) {
        ctx.put(expr, value);
    }

    public String findString(String expr) {
        return (String) ctx.get(expr);
    }

    public String findString(String expr, boolean throwExceptionOnFailure) {
        return findString(expr, false);
    }

    public Object findValue(String expr) {
        return findValue(expr, false);
    }

    public Object findValue(String expr, boolean throwExceptionOnFailure) {
        return ctx.get(expr);
    }

    public Object findValue(String expr, Class asType) {
        return findValue(expr, asType, false);
    }

    public Object findValue(String expr, Class asType, boolean throwExceptionOnFailure) {
        return ctx.get(expr);
    }

    public Object peek() {
        return root.peek();
    }

    public Object pop() {
        return root.pop();
    }

    public void push(Object o) {
        root.push(o);
    }

    public void set(String key, Object o) {
        ctx.put(key, o);
    }

    public int size() {
        return root.size();
    }
}

@quaff
Copy link
Contributor Author

quaff commented Mar 12, 2015

It's just a static util class, It should be light and simple, your suggestion is opposite.

@lukaszlenart
Copy link
Member

But you cannot pass null and expect that it will work properly, if you want such behaviour add method with signature that doesn't expect ValueStack

@quaff
Copy link
Contributor Author

quaff commented Mar 12, 2015

I hope so, please add a method drop last two arguments, thanks.

@asfgit asfgit closed this May 21, 2015
@lukaszlenart
Copy link
Member

Please re-open this PR against master branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants